Skip to content

Bring the collector to 100% line coverage - #26

Merged
tschm merged 2 commits into
mainfrom
test/collector-full-coverage
Aug 30, 2026
Merged

Bring the collector to 100% line coverage#26
tschm merged 2 commits into
mainfrom
test/collector-full-coverage

Conversation

@tschm

@tschm tschm commented Aug 30, 2026

Copy link
Copy Markdown
Member

Stacked on #25, which adds the coverage tooling this measures against. Merge #25 first and this retargets to main.

75.4% → 100% (637/845 → 845/845), in 153 new tests. No pragma: no cover and no omit rule — every line is genuinely executed.

module before after
github.py 61% 100%
__main__.py 30% 100%
localgit.py 84% 100%
metrics.py 90% 100%
config.py 98% 100%
state.py 96% 100%

The gap wasn't evenly spread, and neither is the value of closing it:

github.py — the HTTP layer had never run. Everywhere else the client is faked at _json, which is right for testing what the collector derives from a payload, but means the layer producing those payloads was untested: status handling, pagination, rate-limit bookkeeping. Those now run through an httpx.MockTransport, so the client, request building and response handling stay real and only the socket is replaced. That reaches the states a mixed fleet actually contains — a protection endpoint that 404s for two different reasons, an artifact GitHub already deleted, a feed longer than the page cap.

__main__.py — the layer that decides whether a failure degrades or takes the board down. Three properties now pinned: a raising refresh is recorded rather than propagated; a failure preserves last_success (zeroing it would say "never refreshed" about a board that refreshed a minute ago); and the port does not open until both halves are seeded. main() is driven by making the signal handler fire on registration — a second test schedules it instead, so the wait loop actually runs.

localgit.py — the branches a healthy checkout never takes, which is most of the error handling: git missing from PATH, a call that times out, a clone with no upstream, a tracked file deleted but not staged, a broken symlink, a hand-mangled template pointer, a repo with no commits at all.

metrics.py — one fixture with every optional field populated rather than twelve near-identical tests. The risk is uniform (a guard whose body never executed) and splitting it would obscure that.

Two things the writing turned up

  • Only three Config fields re-read the environment per instance; the rest bind at import. A test that monkeypatches JQ_GITHUB_INTERVAL and calls Config() silently reads the value from process start — mine did, and failed. The new reload fixture documents that and restores the module afterwards, which the pre-existing hand-rolled reload didn't.
  • runpy warns when re-executing an already-imported __main__ — fairly, since that's exactly what's happening. The module is dropped from sys.modules first rather than the warning suppressed.

Verified stable

Five consecutive runs and three randomized-order runs (pytest-randomly), 237 tests passing at 100% every time. Order-independence matters here because the suite reloads a module and starts threads and timers.

tschm added 2 commits August 30, 2026 21:41
From 75.4% (637/845) to 845/845, in 153 new tests. No `pragma: no cover` and
no `omit` rule - every line is genuinely executed.

The gap was not evenly spread, and neither is the value of closing it:

  github.py 61% -> 100%. Everywhere else the client is faked at `_json`, which
  is right for testing what the collector derives from a payload but means the
  layer producing those payloads never ran: status handling, pagination,
  rate-limit bookkeeping. Those are now driven through an httpx.MockTransport,
  so the client, request building and response handling stay real and only the
  socket is replaced. That covers the states a mixed fleet actually contains -
  a protection endpoint that 404s for two different reasons, an artifact
  GitHub has already deleted, a feed longer than the page cap.

  __main__.py 30% -> 100%. The layer that decides whether a failure degrades
  or takes the board down, and almost none of it ran under test. The three
  properties now pinned: a raising refresh is recorded rather than propagated,
  a failure preserves last_success (zeroing it would say "never refreshed"
  about a board that refreshed a minute ago), and the port does not open until
  both halves are seeded. main() is driven by making the signal handler fire on
  registration; a second test schedules it instead, so the wait loop runs.

  localgit.py 84% -> 100%. The branches a healthy checkout never takes, which
  is most of the error handling: git missing from PATH, a call that times out,
  a clone with no upstream, a tracked file deleted but not staged, a broken
  symlink, a hand-mangled template pointer, a repo with no commits at all.

  metrics.py 90% -> 100%. One fixture with every optional field populated,
  rather than twelve near-identical tests: the risk is uniform - a guard whose
  body never executed - and splitting it would obscure that.

  config.py and state.py to 100%, including the thread-safety the store exists
  for and the fact that most Config fields bind at *import*, not per instance.

Two things the writing turned up:

  * Only three Config fields re-read the environment per instance; the rest
    bind at import. A test that monkeypatches JQ_GITHUB_INTERVAL and calls
    Config() silently reads the value from process start. The reload fixture
    documents that and restores the module afterwards, which the pre-existing
    hand-rolled reload did not.
  * runpy warns when re-executing an already-imported __main__, fairly. The
    module is dropped from sys.modules first rather than the warning ignored.

Verified stable: five consecutive runs and three randomized-order runs, all
237 tests passing at 100% each time. Order-independence matters here because
the suite reloads a module and starts threads and timers.
test_ahead_and_behind_are_read_from_the_upstream clones a fixture repo and
commits in the clone. make_checkout sets user.email/user.name on the repo it
creates, but that is repo-local config and `git clone` does not copy it - so
the commit fell back to the global identity, which a laptop has and a CI
runner does not.

Passed locally and failed on the runner with "empty ident name". Coverage was
100% in that failing run too: the defect was entirely in the test's own setup,
not in what it measured.

Verified against the condition CI actually runs under - HOME pointed at an
empty directory with GIT_CONFIG_GLOBAL and GIT_CONFIG_SYSTEM at /dev/null, so
no identity is discoverable anywhere: 237 passed, 100%.
Base automatically changed from ci/publish-own-coverage to main August 30, 2026 17:46
@tschm
tschm merged commit aa95b51 into main Aug 30, 2026
3 checks passed
@tschm
tschm deleted the test/collector-full-coverage branch August 30, 2026 17:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant