Skip to content

chore: upload coverage.xml artifact from CI - #108

Open
runpod-Henrik wants to merge 5 commits into
mainfrom
henrik/ci-coverage-artifact
Open

chore: upload coverage.xml artifact from CI#108
runpod-Henrik wants to merge 5 commits into
mainfrom
henrik/ci-coverage-artifact

Conversation

@runpod-Henrik

Copy link
Copy Markdown
Collaborator

What

The test job already runs make test-coverage on every push, but the result only ever went to the terminal. This emits Cobertura XML and uploads it 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 main instead of anyone retyping it.

Changes

  • Makefile: add --cov-report=xml to test-coverage (keeps the existing term-missing output)
  • .github/workflows/ci.yml: upload coverage.xml as coverage-${{ matrix.python-version }}

The artifact name is per-matrix-version because upload-artifact@v4 requires unique names within a run. if-no-files-found: error so a silently-missing report fails the step rather than publishing an empty artifact.

Verification

Ran make test-coverage in a clean worktree at this commit:

277 passed, 2 warnings in 2.82s
Coverage XML written to file coverage.xml
Required test coverage of 35% reached. Total coverage: 81.21%

make format-check and make lint both clean. The generated coverage.xml parses to 942/1160 lines (81.2%), matching coverage.py's own lines-covered/lines-valid attributes exactly.

No production code touched — CI config and a Makefile flag only.

🤖 Generated with Claude Code

runpod-Henrik and others added 2 commits August 25, 2026 11:05
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>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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-coverage to 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.xml as 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.

Comment thread Makefile Outdated

test-coverage: # Run tests with coverage report (parallel)
uv run pytest tests/ -v -n auto --dist loadscope --cov=handler --cov=remote_execution --cov-report=term-missing
uv run pytest tests/ -v -n auto --dist loadscope --cov=handler --cov=remote_execution --cov-report=term-missing --cov-report=xml --junitxml=pytest-results.xml
Comment thread .github/workflows/ci.yml Outdated
Comment on lines +111 to +117
- name: Upload coverage report
uses: actions/upload-artifact@v4
if: always()
with:
name: coverage-${{ matrix.python-version }}
path: coverage.xml
if-no-files-found: error
runpod-Henrik and others added 3 commits August 26, 2026 08:33
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>
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.

2 participants