feat: add unit test coverage#421
Open
RuibinCheung wants to merge 4 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Adds CI-based unit test coverage measurement for Primus-Turbo by instrumenting existing GPU test workflows with pytest-cov/coverage.py, publishing per-job summaries, and producing a merged HTML coverage report artifact.
Changes:
- Adds
pytest-cov/coverage[toml]and configures pytest + coverage settings inpyproject.toml. - Instruments PyTorch/JAX GPU CI jobs to collect and upload coverage data, plus a new job to merge artifacts and generate a combined HTML report.
- Introduces
tools/ci/coverage_summary.pyto rendercoverage jsonoutput as a compact Markdown table for GitHub Actions summaries.
Reviewed changes
Copilot reviewed 3 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
tools/ci/coverage_summary.py |
New utility to render coverage JSON into a CI-friendly Markdown summary table. |
requirements.txt |
Adds coverage tooling dependencies (pytest-cov, coverage[toml]). |
pyproject.toml |
Adds pytest defaults and coverage configuration (branch/parallel, omit/source/path remapping). |
.gitignore |
Ignores local .coverage* artifacts. |
.github/workflows/ci.yaml |
Instruments GPU test jobs for coverage collection and adds a merge/reporting job with HTML artifact upload. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| run: | | ||
| { | ||
| echo "" | ||
| echo "Full HTML report: download the **visual-coverage** artifact" |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 5 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (1)
.github/workflows/ci.yaml:378
- The run-summary text tells users to download the visual-coverage artifact, but the uploaded artifact is currently named
coverage. This makes the instructions incorrect and confusing.
- name: Upload combined coverage report
id: upload_combined
if: always()
uses: actions/upload-artifact@v4
with:
name: coverage
path: visual-coverage
if-no-files-found: warn
Comment on lines
+219
to
+223
| - name: Combine coverage and generate reports | ||
| if: always() | ||
| run: | | ||
| cp .coverage coverage_data_pytorch || true | ||
| - name: Upload coverage data |
Comment on lines
+317
to
+320
| - name: Combine coverage and generate reports | ||
| if: always() | ||
| run: | | ||
| cp .coverage coverage_data_jax || true |
Comment on lines
+9
to
+11
| Usage: | ||
| coverage_summary.py TITLE REPORT.json [REPORT.json ...] | ||
|
|
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 5 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (3)
tools/ci/coverage_summary.py:11
- The module docstring documents a CLI signature that includes a TITLE argument, but main() only accepts JSON report paths (and ci.yaml invokes it without a title). This mismatch makes the usage text misleading.
Usage:
coverage_summary.py TITLE REPORT.json [REPORT.json ...]
.github/workflows/ci.yaml:222
- This step is named “Combine coverage and generate reports” but it only copies .coverage. With [tool.coverage.run] parallel=true, pytest-cov/subprocess coverage commonly leaves multiple .coverage.* data files that need
coverage combineto produce a single .coverage file. As-is, the artifact upload can be empty/incomplete, breaking the downstream merge.
- name: Combine coverage and generate reports
if: always()
run: |
cp .coverage coverage_data_pytorch || true
.github/workflows/ci.yaml:320
- This step is named “Combine coverage and generate reports” but it only copies .coverage. With [tool.coverage.run] parallel=true, pytest-cov/subprocess coverage commonly leaves multiple .coverage.* data files that need
coverage combineto produce a single .coverage file. As-is, the artifact upload can be empty/incomplete, breaking the downstream merge.
- name: Combine coverage and generate reports
if: always()
run: |
cp .coverage coverage_data_jax || true
Comment on lines
+50
to
+54
| if len(seg) == 2: # primus_turbo/<file>.py | ||
| return seg[1], None | ||
| if seg[1] in DETAILED_GROUPS: | ||
| return seg[1], seg[1] + "/" + seg[2] | ||
| return seg[1], None |
| run: | | ||
| # Enable coverage in test subprocesses (spawned by distributed tests) | ||
| # and clear stale data files left over from retried/previous runs. | ||
| echo "COVERAGE_PROCESS_START=$(pwd)/pyproject.toml" >> $GITHUB_ENV |
| run: | | ||
| # Enable coverage in test subprocesses (spawned by distributed tests) | ||
| # and clear stale data files left over from retried/previous runs. | ||
| echo "COVERAGE_PROCESS_START=$(pwd)/pyproject.toml" >> $GITHUB_ENV |
Comment on lines
192
to
+196
| # use timeout to terminate pytest and its workers on timeout | ||
| command: > | ||
| timeout -k 60s 130m | ||
| bash -lc | ||
| "exec pytest -v tests/pytorch -n 8 --timeout=600 --timeout-method=thread --max-worker-restart=8 --log-cli-level=INFO" | ||
| "exec pytest -v tests/pytorch -n 8 --timeout=600 --timeout-method=thread --max-worker-restart=8 --log-cli-level=INFO --cov=primus_turbo --cov-branch --cov-append" |
RuibinCheung
changed the title
Comment on lines
+221
to
+227
| - name: Combine and stage coverage data | ||
| if: always() | ||
| run: | | ||
| # Merge any leftover parallel/subprocess data files into a single | ||
| # .coverage, then stage it for the combined coverage-report job. | ||
| coverage combine || true | ||
| cp .coverage coverage_data_pytorch || true |
Comment on lines
+379
to
+380
| python tools/ci/coverage_summary.py coverage.json \ | ||
| >> "$GITHUB_STEP_SUMMARY" 2>/dev/null || true |
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.
Description
This PR adds unit-test coverage measurement to the CI pipeline.
It instruments the existing PyTorch and JAX GPU test jobs with
coverage.py(viapytest-cov), collects coverage across single-GPU, deterministic, and distributed test runs, and merges the per-framework data into a single library-wide report.The motivation is to gain visibility into how much of
primus_turbois exercised by the test suite, so gaps can be tracked over time.Coverage is rendered both as a Markdown table in the GitHub Actions run summary and as a downloadable HTML report artifact.
Fixes # (issue)
Type of change
Changes
Please list the changes introduced in this PR:
unittest-pytorch-gfx942andunittest-jax-gfx942CI jobs with--cov=primus_turbo --cov-branch --cov-appendacross single-GPU, deterministic, and distributed test steps.COVERAGE_PROCESS_STARTso subprocess-spawned (distributed) tests are also measured, and clears stale coverage data from retried runs.coverage combine/report/json, render a summary table into$GITHUB_STEP_SUMMARY, and upload the raw coverage data as artifacts.coverage-reportCI job that downloads the PyTorch and JAX coverage artifacts, merges them, and produces a combinedcoverage report, HTML report, and run-summary table, gated on theci:gpulabel for pull requests.tools/ci/coverage_summary.py, which renders coverage.py JSON as a compact Markdown table, merges multiple reports per file by max covered lines, and excludes the JIT-compiledtriton/andflydsl/kernel layers.[tool.pytest.ini_options]and[tool.coverage.*]configuration topyproject.toml(branch/parallel coverage, source/omit paths, path remapping for cross-runner data).pytest-covandcoverage[toml]torequirements.txt..coverage*data files in.gitignore.Checklist: