Repository navigation
fix: include serial tests in coverage.xml and upload it from CI - #373
Closed
runpod-Henrik wants to merge 16 commits into
Closed
runpod-Henrik wants to merge 16 commits into
runpod-Henrik wants to merge 16 commits into
Conversation
runpod-Henrik
force-pushed
the
Henrik/ci-coverage-artifact
branch
from
August 25, 2026 18:20
77bbea2 to
8dd2f16
Compare
deanq
reviewed
Aug 26, 2026
deanq
left a comment
deanq
left a comment
Member
There was a problem hiding this comment.
A few robustness notes on the coverage plumbing. The core fix (re-emitting coverage.xml after the serial --cov-append pass) is right; these are edge cases around failing/aborted runs.
The quality-gates matrix declared 3.10/3.11/3.12/3.13 but every leg ran
Python 3.11. `make dev` runs a bare `uv sync`, and uv resolves its
interpreter from `.python-version` (pinned to 3.11) rather than from the
one actions/setup-python just installed. Confirmed across four CI runs on
three different branches, the oldest from 2026-08-10: every leg reports
`Using CPython 3.11.x` and builds `.venv/lib/python3.11`.
UV_PYTHON takes precedence over the pin file. Set at job level rather
than on the install step because `make ci-quality-github` runs
`uv run pytest`, which resolves the interpreter again.
uv.lock is re-locked in the same commit because it was stale in two ways
and 3.13 could not resolve without it:
* `requires-python = ">=3.10, <3.13"` while pyproject.toml says
`>=3.10,<3.14`, so the lock never covered 3.13 at all
* `tomlkit>=0.13.0` is declared in pyproject.toml but was missing from
the runpod-flash dependency and requires-dist lists
`uv lock --check` fails against the previous lock and passes against this
one, for each of 3.10/3.11/3.12/3.13. CI did not catch the drift because
pre-check uses `--frozen` (which does not verify freshness) and `make dev`
uses plain `uv sync`, which silently re-resolves on the runner.
Verified: the full suite on a real 3.13 interpreter gives 2680 passed,
with no 3.13-specific failures.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
test_deploy_all_background was the source of a flaky failure that hit an
unrelated test, most often
FAILED tests/unit/test_regressions.py::TestREG008NonInteractiveEnvDelete
::test_undeploy_resource_force_remove_no_tty
_pickle.PicklingError: args[0] from __newobj__ args has the wrong class
deploy_all_background() starts a daemon thread and returns immediately,
and the test never joined it. The `patch.object(..., "get_or_deploy_resource")`
context was therefore lifted while the thread was still starting, so the
thread ran against the unpatched manager and registered the fixture's
MagicMock(spec=ServerlessResource) objects into the ResourceManager
singleton -- during whichever unrelated test happened to be running at
that moment.
ResourceManager._save_resources() cloudpickles its entire state on every
registration, and a MagicMock cannot be pickled: `obj.__class__` is the
spec'd class while `type(obj)` is MagicMock, which is exactly the
mismatch pickle.save_reduce rejects. The mock arrived as a dict *key*,
via _migrate_to_name_based_keys() calling `resource.get_resource_key()`
on it, so the offending key rendered as
`<MagicMock name='mock.get_resource_key()'>`.
Because it depended on thread scheduling and on xdist's dynamic worker
assignment, the victim, the worker and the matrix leg all varied per run:
the same failure appears on three unrelated branches, on legs 3.10, 3.12
and 3.13 and on workers gw0, gw2 and gw3. Forcing everything into one
process reproduced it every time; `-n 4` reproduced it in 0 of 6 runs.
threading.Thread is now stubbed, which keeps what this test actually
asserts -- the call is non-blocking and spawns a daemon thread -- and lets
nothing escape. The test previously asserted nothing at all; its own
comment said "not much we can test here without waiting for thread".
Verified: full suite single-process, 2680 passed, 0 PicklingError (the
same run reproduced the failure before this change). The two remaining
local failures are TestVersionFlag, which is colour-dependent and passes
both in CI and locally under NO_COLOR=1.
Two deeper faults are left alone here, as they are wider changes:
ResourceManager._resources / _resource_configs are class variables that
instance mutation writes through, and conftest's worker_flash_dir is
scope="session", so every test in a worker shares one resources.pkl.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Making the version matrix real (previous commit) exposed three failures on
Python 3.10 that had been masked for months. All three are test artifacts,
not product bugs, and all three reproduce on a real 3.10 interpreter
independently of this branch.
tests/unit/cli/commands/test_run_server_helpers.py
call_with_body -> _map_body_to_params calls inspect.signature(func), and
signature() of a Mock is version-dependent: on 3.10 it raises
`TypeError: 'Mock' object is not subscriptable` (Mock auto-creates a
__signature__ child which inspect then tries to use), while on 3.11+ it
reports (*args, **kwargs). Passing spec= does not help -- verified; the
failure is in signature() itself.
call_with_body catches Exception and converts it into a 500 JSONResponse,
so the visible failure was the uninformative
`assert <JSONResponse object> == {'ok': True}`, with the real TypeError
readable only inside the response body.
The mock is now wrapped in a real `async def (*args, **kwargs)` -- the
same signature 3.11+ inferred from the Mock, so both branches of
_map_body_to_params behave exactly as before -- and delegating to the mock
keeps `assert_called_once_with()`.
tests/unit/core/resources/test_resource_manager_extended.py
test_loads_legacy_dict_format cloudpickle.dump()s a MagicMock, which 3.10
rejects with "Could not pickle object as excessively deep recursion
required". Replaced with a module-level _LegacyResource: only .config_hash
is read (via _refresh_config_hashes), and the *absence* of
get_resource_key is what keeps the legacy key un-migrated, which is what
this test asserts. Module-level rather than defined in the test body,
since cloudpickle serialises a function-local class by value.
Verified on 3.10, 3.11 and 3.13: 40 passed in both affected files on each,
where 3.10 previously failed all three tests. ruff format and check clean.
Worth following up separately: the blanket `except Exception ->
JSONResponse(500)` in call_with_body turns genuine errors into silent 500s,
which is what made this take three steps to diagnose.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The coverage run is split in two: parallel tests with --cov-report=xml, then serial tests with --cov-append but only --cov-report=term-missing. The second run appends to .coverage without re-emitting the XML, so coverage.xml was left holding the parallel run alone. Measured locally: 85.1% (8085/9503) before the append is re-emitted vs 85.6% (8132/9503) after — 47 covered lines from the 19 serial-marked tests were being dropped from the report. Also upload coverage.xml so the weekly coverage report can read the number from the newest successful run on main instead of it being collected by hand. `if-no-files-found: error` so a silently-missing report fails the step rather than publishing an empty artifact. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The coverage report was only readable by downloading the artifact. Write a Test Results + Coverage Summary table to $GITHUB_STEP_SUMMARY so the numbers show up on the run page, matching what the ai-api component workflow does. Reads both pytest-results-parallel.xml and pytest-results-serial.xml, so the counts cover the whole suite rather than one pass. Stdlib only, so there is no extra install step, and `if: always()` means the summary still renders when tests fail — which is when it is most useful. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Addresses all four review comments on this PR.
Summary step (same three defects, all reachable because the step runs under
`if: always()`):
* the coverage-XML parse was unguarded while the junit parse beside it
was. A report truncated by a timeout, OOM or crashed xdist worker made
ET.parse raise, so the step exited non-zero and stacked a spurious
failure on top of the real one. Verified: the old script exits 1 on a
truncated report, the new one exits 0 and reports it in the summary.
* attributes were parsed with bare int(), so a malformed value raised
rather than degrading.
* the status cell treated `0 failures` as passing even when no tests ran.
Upload now uses `if-no-files-found: warn`. With `error`, a run that never
produced coverage.xml — pytest erroring at collection before pytest-cov
writes anything — failed the upload too, masking the root cause. This
upload hangs off the PR-gating job, so it should not fail a PR on its own.
Makefile: documented the two asymmetries flagged in review.
* the serial line, not the parallel one, is the real coverage gate: the
parallel pass passes --cov-fail-under=0 while the serial pass inherits
65 from pyproject addopts. Aligning the two flags would quietly
disable the gate, so that is now written down.
* make stops at the first failing recipe line, so a failing parallel pass
means the serial re-emit never runs and coverage.xml holds the parallel
subset. Left as-is deliberately rather than guarding with `-`/`|| true`:
that would let a red parallel suite exit 0, which is worse than an
undercount on a run that already failed. The weekly report only reads
runs whose status is success, so nothing consumes that undercount.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Branch coverage was never collected, so the job summary could only ever show line coverage. Adds --cov-branch. Purely additive: line coverage is unchanged (verified per repo), so the weekly trend, which reads line coverage, is unaffected. The Cobertura report now carries branches-valid/covered, which the summary renders as its own row. Note --cov-fail-under gates on coverage.py total, which now blends lines and branches, so that number drops even though line coverage does not. Measured before committing: this repo stays comfortably above its gate. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Replaces the inline summary script with runpod/internal-workflows/.github/actions/coverage-summary, which now owns both the summary rendering and the artifact upload. The same ~90-line script had been copy-pasted into six repos, and review found the same bugs in every copy — four rounds of a single class where "passed" was the default state and each new way of losing a report had to be patched out of it separately. The shared version computes it from positive evidence instead, and has 59 unit tests plus a smoke job behind it. Pinned by commit rather than tag, so a change to the action cannot reach this repo until someone bumps the SHA. Behaviour was proven on runpod/github-image-builder first: green run, artifact uploaded, and the weekly collector parsed it back at the expected number before the remaining repos followed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A public repository cannot consume an action from a private one, and
runpod/internal-workflows is private. Every matrix leg failed at "Set up
job" with:
Unable to resolve action `runpod/internal-workflows`, not found
This reverts the migration to the shared action for this repo. The inline
script is restored, including all the hardening from review.
The shared action still applies to the private repos — github-image-builder
is migrated and green, main-ui and RunPod follow. Sharing it here needs
internal-workflows to be public, or the action published somewhere public.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
runpod-Henrik
force-pushed
the
Henrik/ci-coverage-artifact
branch
from
August 27, 2026 00:08
3ad315f to
eb12ff8
Compare
Replaces the ~90-line inline heredoc summary with the shared action, now that
there is a host this repo can actually consume.
The first attempt at this pointed at runpod/internal-workflows, which is
private, and failed at `Set up job` on every matrix leg:
Unable to resolve action `runpod/internal-workflows`, not found
No tests ran at all, so it was reverted. A public repository cannot resolve
an action from a private one, and this repo is public. The action now lives
in runpod/coverage-summary-action, which is public, and the same commit is
already green in three private consumers.
Two things differ from the reverted attempt:
* pinned to the public repo's commit rather than internal-workflows'
* the "Upload test results" step is kept. The earlier attempt dropped it,
which would have silently stopped publishing test-results-<version>
artifacts. The action uploads one artifact; coverage and test results are
two, so that step stays separate.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…tifact # Conflicts: # tests/unit/test_deployment.py
…tifact # Conflicts: # uv.lock
|
One ordering note (non-blocking): this branch doesn't include #374's |
deanq
reviewed
Oct 15, 2026
deanq
left a comment
deanq
left a comment
Member
There was a problem hiding this comment.
#374 merged on 2026-09-26, so the UV_PYTHON block in ci.yml and the test-wrapper changes in test_run_server_helpers.py / test_resource_manager_extended.py are already on main. They only show in this diff because the branch predates that merge. A rebase onto main would cut this PR down to its own changes (Makefile + coverage summary step) and settles the merge-order note above.
| # `-`/`|| true` so this always ran would let a red parallel suite exit 0. | ||
| # The weekly report only reads runs whose status is success, so an | ||
| # undercount on an already-failing run is not consumed by anything. | ||
| uv run pytest tests/ --junitxml=pytest-results-serial.xml -v -m "serial" --cov=runpod_flash --cov-branch --cov-append --cov-report=term-missing --cov-report=xml |
Member
There was a problem hiding this comment.
--cov-branch here changes what --cov-fail-under=65 measures: the total becomes (lines + branches covered) / (lines + branches), which is usually lower than line-only. pyproject.toml still says [tool.coverage.run] branch = false, so the config and the CLI now disagree.
It's also only added to test-coverage and ci-quality-github. test-coverage-serial and ci-quality-github-serial stay line-only, so the same 65% gate can pass locally via the serial target and fail in CI (or the reverse) on identical code.
Suggest setting branch = true once in [tool.coverage.run] and dropping the CLI flags, so every target measures the same thing. Also worth confirming the suite still clears 65% with branch coverage on.
There was a problem hiding this comment.
Confirmed: at head, --cov-branch appears at Makefile:75,78,129,146, but pyproject.toml:122 still says branch = false, and test-coverage-serial (:81) and ci-quality-github-serial (:157) are line-only. On the gate question, CI already answers it: the 3.11 leg reports 9557 statements + 3172 branches and "Total coverage: 85.30%", so 65% holds. Setting branch = true in [tool.coverage.run] and dropping the four flags fixes the drift in one place.
There was a problem hiding this comment.
Verified fixed at b920a4e: pyproject.toml:122 now sets branch = true, and none of the four --cov-branch flags remain (Makefile:77,78,143,146). test-coverage-serial (:81) and ci-quality-github-serial (:157) read the same config, so every target measures line + branch against the 65% gate. Leaving this thread for @deanq to resolve.
| @echo "::endgroup::" | ||
| @echo "::group::Test suite with coverage (serial pass)" | ||
| uv run pytest tests/ --junitxml=pytest-results-serial.xml -v -m "serial" --cov=runpod_flash --cov-append --cov-report=term-missing | ||
| # Re-emit the XML after appending the serial tests, or coverage.xml is left |
Member
There was a problem hiding this comment.
These # lines are tab-indented inside the recipe, so make treats them as recipe lines: it echoes each one and hands it to the shell. Every make quality-check / CI leg prints this 14-line block into the "serial pass" log group. Moving the block above the target (unindented), or prefixing lines with @, keeps it out of the logs.
There was a problem hiding this comment.
Confirmed: the Quality Gates (3.11) log echoes them, for example # Re-emit the XML after appending the serial tests, ... and # This line, not the parallel one above, ... in the serial-pass group. test-coverage at Makefile:76-77 has the same two-line echo. Moving both blocks above their targets, unindented, keeps them as documentation and out of the logs.
There was a problem hiding this comment.
Verified fixed at b920a4e: both comment blocks now sit unindented above their targets (Makefile:74-75 and :121-134), so make no longer echoes them. The Quality Gates (3.11) log at this head has no echoed # lines in either pass. Leaving this thread for @deanq to resolve.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
justinwlin
approved these changes
Oct 26, 2026
justinwlin
left a comment
justinwlin
left a comment
Contributor
There was a problem hiding this comment.
Rebased onto main (via update-branch), CI green on all matrix legs, coverage artifacts published per Python version. Local make ci-quality-github: 85.33% with branch coverage, coverage.xml now includes the serial pass.
OneAsterisk
reviewed
Oct 26, 2026
OneAsterisk
left a comment
OneAsterisk
left a comment
There was a problem hiding this comment.
Claude review, round 1
Verdict: Ready after fixes. The core fix is correct and CI proves it, but the PR description no longer matches the code.
Approval gate: 0 Critical, 1 High, 2 Medium+Low → not approved: the PR body makes claims that are false at head.
Existing feedback
- deanq on
Makefile:146(--cov-branchvspyproject.tomlbranch = false): Confirmed (Medium), reply in thread. #373 (comment) - deanq on
Makefile:132(tab-indented#lines echoed into logs): Confirmed (Low), the 3.11 CI log shows them, reply in thread. #373 (comment) - deanq's four 2026-08-26 threads (parse guard,
warnvserror, abort-on-parallel-failure, gate asymmetry): resolved, and the head code matches the replies (if-no-files-found: warn, Makefile comments at lines 135-145). - deanq review on rebasing after #374, and jebenexer on merge order: Already addressed. The diff at
1d6eddcis onlyMakefileandci.yml.UV_PYTHONis onmain(ci.yml:242-243is not in the diff).
PR description vs code
| Claim | Status | Where |
|---|---|---|
Serial pass re-emits coverage.xml in test-coverage and ci-quality-github |
✅ | Makefile:78, Makefile:146 |
ci.yml uploads coverage.xml as coverage-${{ matrix.python-version }} |
✅ (via runpod/coverage-summary-action, not upload-artifact directly) |
ci.yml:278-286; 3.11 log: "Artifact coverage-3.11 has been successfully uploaded" |
if-no-files-found: error so a missing report fails the step |
❌ now warn |
ci.yml:286 |
| Numbers: 85.6% / 8132 of 9503 lines, CI gate 85.47% | --cov-branch; CI 3.11 reports 85.30% over 9557 statements + 3172 branches |
Makefile:129,146 |
| Branch coverage | ❌ not mentioned | Makefile:75,78,129,146 |
| No production code touched | ✅ | diff is 2 files |
Findings
High
- PR body is stale against head (
.github/workflows/ci.yml:286, inline). The "Changes" section describes a directupload-artifactstep withif-no-files-found: error. Head uses the shared action withwarn. The body never mentions--cov-branch, which changes what the published number means. The stated purpose is a weekly report that stopped false "regressions". The first branch-coverage week will read about 0.3 points lower than the line-only figures in this description. Please update "Changes" and "Verification", and note the metric change for the report owner.
Medium
- Branch coverage configured in two places that disagree (
Makefile:146, deanq's thread). Confirmed, details in the thread.
Low
- Recipe comments are echoed into CI logs (
Makefile:132, deanq's thread). Confirmed, andMakefile:76-77intest-coveragehas the same issue.
Nits
Makefile:138uses an em dash; the rest of the Makefile and ci.yml use--.
Simplification opportunities
- Setting
branch = truein[tool.coverage.run]removes four--cov-branchflags (Makefile:75,78,129,146) and fixes the Medium. Fits this PR.
Verified clean
- Pinned action
runpod/coverage-summary-action@65b35a3is public. It accepts every input passed here (format,coverage-file,results,artifact-name,if-no-files-found). It passes inputs via env, not script interpolation. - The action uploads with
if: always()andoverwrite: true, so re-runs do not collide on artifact names. - Per-matrix artifact names are unique (
coverage-3.10tocoverage-3.14). - The serial pass still inherits
--cov-fail-under=65frompyproject.tomladdopts. CI 3.11: "Required test coverage of 65% reached. Total coverage: 85.30%". Branch coverage clears the gate. - Re-emit works in CI: the parallel pass reports 84.83%, and the appended serial pass reports 85.30%. Each writes
coverage.xml. - All 10 checks pass at
1d6eddc.
Questions for the author
- Does the weekly report compare against historical line-only numbers? If so, should it start a new baseline at this merge?
Reviewed by Claude (claude-opus-5-5) via the pr-review skill. Tokens: 12 in / ~5,000 out / 277,359 cache read / 51,556 cache write = ~334,000. Estimated cost: ~$1.35 at the rate table in the skill's sum_usage.py (5-minute cache-write TTL), as of posting. Round 1 of this PR. This is automated assistance; a human owns the final call.
…ts out of logs - Set branch = true in [tool.coverage.run] and drop the four --cov-branch flags, so every target, including the -serial ones, measures the same thing against the 65% gate. - Move the coverage comment blocks above test-coverage and ci-quality-github. Tab-indented, make echoed all of them into every CI log.
OneAsterisk
approved these changes
Oct 26, 2026
OneAsterisk
left a comment
OneAsterisk
left a comment
There was a problem hiding this comment.
Claude review, round 2
Verdict: Approved. Round 2 fixes every round 1 finding, and the new commit adds no new problems.
Approval gate: 0 Critical, 0 High, 0 Medium+Low → approved.
Status of prior findings
| # | Finding | Severity | Status | Evidence |
|---|---|---|---|---|
| R1-1 | PR body stale against head (upload-artifact/error, no branch coverage) |
High | Fixed | Body now describes runpod/coverage-summary-action with warn, branch = true, and the metric change |
| R1-2 | Branch coverage set in two places that disagree (deanq) | Medium | Fixed | pyproject.toml:122 branch = true; no --cov-branch left in Makefile, b920a4e |
| R1-3 | Recipe comments echoed into CI logs (deanq) | Low | Fixed | Makefile:74-75, :121-134 unindented above targets; 3.11 log at head has no echoed lines, b920a4e |
| R1-4 | Em dash in Makefile comment | Nit | Fixed | Makefile:127 uses -- |
| R1-Q | Does the weekly report need a new baseline? | Question | Answered | Body: "Anything comparing against older numbers should expect that step." |
Existing feedback
- deanq on
Makefile(--cov-branchvsbranch = false): Verified fixed at b920a4e, reply in thread. #373 (comment) - deanq on
Makefile(echoed#lines): Verified fixed at b920a4e, reply in thread. #373 (comment) - deanq's four 2026-08-26 threads: still resolved; the moved comment block at
Makefile:124-134keeps the gate-asymmetry and abort-on-failure notes verbatim in meaning. - justinwlin approval at 1d6eddc: predates b920a4e (their own follow-up commit). No conflict with this round.
PR description vs code
| Claim | Status | Where |
|---|---|---|
Serial pass re-emits coverage.xml in test-coverage and ci-quality-github |
✅ | Makefile:78, :146 |
| Comments sit above each target, not echoed | ✅ | Makefile:74-75, :121-134; 3.11 log |
branch = true; every target, including -serial ones, uses the same 65% gate |
✅ | pyproject.toml:122, addopts --cov-fail-under=65; Makefile:81, :157 read the same config |
Shared action pinned by commit, if-no-files-found: warn, test-results upload separate |
✅ | ci.yml:278-286 |
| Metric is now line + branch, about 85.3% | ✅ | 3.11 at head: 9557 stmts + 3172 branches, "Total coverage: 85.30%" |
coverage-<version> artifact per Python version |
✅ | 3.11 log: "Artifact coverage-3.11 has been successfully uploaded" |
| No production code touched | ✅ | 3 files: ci.yml, Makefile, pyproject.toml |
Findings
None at Critical, High, Medium, or Low.
Nits
- PR body says "the 19
serial-marked tests", but the serial pass runs 53 tests (CI 3.11: "53 passed, 1 skipped"). 19 is the count ofserialmarkers, some on classes. Consider "the 53 tests inserial-marked groups".
Simplification opportunities
None. The diff is down to 3 files and 41 lines.
Verified clean
branch = truewithparallel = true: the parallel pass starts without--cov-append, so pytest-cov erases old data first. Stale line-only data cannot mix with arc data intest-coverageorci-quality-github.scripts/mcp_code_intel_server.py:147parses theTOTALline with a lazy regex up to the firstNN.NN%. The addedBranch BrPartcolumns are integers, so it still reads the right percentage.e2e.ymlruns with--no-cov, so it is unaffected by the branch switch.- The 18c83aa merge of
main(#393 deps bump) does not appear in the PR diff. - CI at b920a4e: all 10 checks pass, including the 5 Quality Gates legs and Nix flake.
Questions for the author
None.
Reviewed by Claude (claude-opus-5-5) via the pr-review skill. Tokens: 42 in / ~9,500 out / ~1,236,000 cache read / ~62,000 cache write = ~1,308,000. Estimated cost: ~$2.00 at the rate table in the skill's sum_usage.py (5-minute cache-write TTL), as of posting. Round 2 of this PR. This is automated assistance; a human owns the final call.
Contributor
|
Superseded by #395 (same diff, squashed onto current main). |
justinwlin
added a commit
that referenced
this pull request
Oct 27, 2026
* fix: include serial tests in coverage.xml and upload it from CI Re-emit coverage.xml on the --cov-append pass so the serial tests are counted, collect branch coverage, and publish coverage.xml per matrix leg through runpod/coverage-summary-action. Supersedes #373. * chore: trim coverage comments in ci.yml and Makefile Drop the internal-workflows history from the ci.yml comment and the reference to the external weekly report from the Makefile comment.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug
Our coverage run is split in two passes:
The second pass appends to
.coveragebut only re-renders the terminal report — it never re-emits the XML. Socoverage.xmlon disk holds the parallel run alone, and the 19serial-marked tests contribute nothing to it.Measured locally:
47 covered lines were being dropped from the report. The terminal output was always correct; only the XML was wrong, which is why this went unnoticed — nothing consumed the XML until now.
Changes
Makefile: add--cov-report=xmlto the--cov-appendpass in bothtest-coverageandci-quality-github, socoverage.xmlincludes the serial tests. The explanatory comments sit above each target, somakeno longer echoes them into the logs.pyproject.toml:branch = truein[tool.coverage.run]. Every target now measures line + branch coverage against the same 65% gate, includingtest-coverage-serialandci-quality-github-serial..github/workflows/ci.yml: publishcoverage.xmlper matrix leg ascoverage-${{ matrix.python-version }}through the sharedrunpod/coverage-summary-action, pinned by commit, withif-no-files-found: warn. The test-results upload stays a separate step.The published metric changed. With branch coverage on, the total is (lines + branches covered) / (lines + branches): 85.33% locally, against 85.6% for line coverage alone. Anything comparing against older numbers should expect that step.
Why upload it
We're automating the weekly Test Coverage Progress report, which currently relies on hand-collected numbers. That has produced real errors — figures reported as improvements when they were regressions, and at least one number that matched no CI run at all. With this artifact published, the report reads the number straight from the newest successful run on
main.Verification
Ran the full CI gate (
make ci-quality-github) on the branch merged with currentmain:coverage.xmlreports 9557 lines (86.83% covered) and 3172 branches (80.83% covered). The log contains no echoed Makefile comment lines. CI on GitHub publishes acoverage-<version>artifact for each Python version.No production code touched — CI config and Makefile flags only.
🤖 Generated with Claude Code