chore: upload coverage.xml artifact from CI - #108
Open
runpod-Henrik wants to merge 6 commits into
Open
Conversation
The test job already runs `make test-coverage` on every push, but the result only ever went to the terminal. Emit Cobertura XML and upload it 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. Adds --junitxml so the run's test counts can be reported alongside coverage; pytest-results.xml is gitignored. 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>
runpod-Henrik
requested review from
KAJdev and
jhcipar
and
a lite review from Copilot
August 25, 2026 23:40
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves CI observability by generating a Cobertura coverage.xml report during the existing make test-coverage run and publishing it as a per-Python-version GitHub Actions artifact, enabling automated downstream reporting from the latest successful main run.
Changes:
- Extend
make test-coverageto emit Cobertura XML (and a JUnit XML test report file). - Add a CI step that renders a coverage/test summary into the GitHub Actions run summary.
- Upload
coverage.xmlas a uniquely named artifact per matrix Python version.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
Makefile |
Adds --cov-report=xml (and --junitxml=pytest-results.xml) to the test-coverage target so CI can publish machine-readable reports. |
.gitignore |
Ignores the newly generated pytest-results.xml file. |
.github/workflows/ci.yml |
Adds a run-summary renderer and uploads coverage.xml as a per-matrix artifact. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Review feedback from #373. Three defects, all reachable because this 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 summary 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 says so in the summary.
* hits/lines 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
at all. A suite dying at import reports errors>0 with tests possibly 0,
so the check is now `no failures AND at least one test`.
Also switch the artifact upload to `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 step too,
red-flagging the job and masking the root cause. This upload hangs off the
PR-gating test job, so it should not be able to fail a PR on its own.
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 ~90-line inline heredoc summary and the separate coverage upload with the shared action, which does both. This repo was previously left on its own copy because the action lived in a private repo in the other org, and consuming it from here would have needed the Access setting widened. That turned out not to be the fix at all: a public repository cannot resolve an action from a private one regardless of Access, which governs sharing within an org or enterprise. The action now lives in runpod/coverage-summary-action, which is public, so cross-org consumption is just a normal third-party action reference -- the same shape as the actions/, docker/ and googleapis/ ones this workflow already uses. The same pinned commit is already green in four other consumers. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
What
The
testjob already runsmake test-coverageon every push, but the result only ever went to the terminal. This emits Cobertura XML, renders the numbers into the run summary, and uploads the report as an artifact.Why
We're automating the weekly Test Coverage Progress report. It currently relies on hand-collected numbers, which 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
maininstead of anyone retyping it.Changes
Makefile:test-coveragegains--cov-report=xml,--cov-branchand--junitxml=pytest-results.xml(keeps the existingterm-missingoutput).--junitxmlis not incidental: the summary step reads it to render the pass/fail table alongside coverage.--cov-branchadds branch coverage, so the report can show more than line coverage..github/workflows/ci.yml: the inline ~90-line summary heredoc is replaced by the sharedrunpod/coverage-summary-action, which renders the summary and uploadscoverage-${{ matrix.python-version }}.The artifact name is per-matrix-version because
upload-artifact@v4requires unique names within a run.if-no-files-found: warn, noterror. The upload runs underif: always(), so on a run where the tests failed before producingcoverage.xml,errorwould stack a second failing step on top of the real one and obscure it. The weekly report only ever reads runs whose status issuccess, so a missing artifact on an already-failing run costs nothing.On the shared action
It lives in its own public repo because a public repository — this one — cannot resolve an action from a private one, and this repo is in a different org besides. An earlier attempt to consume it from a private repo failed at
Set up jobon every matrix leg withUnable to resolve action, not found, so no tests ran at all.Pinned by full commit SHA rather than a tag: a tag can be repointed at new code, and pinning means a change to the action cannot reach us until we bump it deliberately.
Verification
The action produces byte-identical numbers to the inline script it replaces, on this repo's own artifact:
make format-checkandmake lintclean. The same pinned action commit is green in five other consumers across two orgs.No production code touched — CI config and Makefile flags only.
🤖 Generated with Claude Code