ci: report code coverage on pull requests - #10733
Conversation
There was a problem hiding this comment.
Pull request overview
This PR restores PR-visible code coverage reporting after the Microsoft.Testing.Platform migration by collecting Cobertura coverage artifacts in PR CI runs and aggregating them in a trusted workflow_run reporter which publishes a “Code Coverage” check and job summary.
Changes:
- Add the MTP Code Coverage extension package to Orleans test applications (net10.0 only) and centrally manage its version.
- Collect and upload Cobertura (
*.cobertura.xml) artifacts from the Linux/.NET 10 BVT/SlowBVT/Functional CI matrix on pull requests. - Add a trusted
workflow_runjob which downloads coverage artifacts, summarizes repositorysrc/line coverage, and publishes a “Code Coverage” check.
Show a summary per file
| File | Description |
|---|---|
test/Directory.Build.props |
Adds Microsoft.Testing.Extensions.CodeCoverage to test apps for net10.0. |
Directory.Packages.props |
Introduces a central version pin for the Code Coverage extension. |
.github/workflows/ci.yml |
Enables coverage collection on PR Linux/net10 test runs and uploads suite-scoped coverage artifacts. |
.github/workflows/test-results.yml |
Adds a workflow_run coverage reporter which summarizes artifacts and publishes a check run. |
.github/scripts/summarize-coverage.py |
New script to parse Cobertura XML, aggregate line coverage, and emit JSON/Markdown summaries. |
.github/coverage.config.xml |
Adds MTP code coverage configuration for static instrumentation and exclusions. |
Review details
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- Files reviewed: 6/6 changed files
- Comments generated: 1
- Review effort level: Lite
|
The current Ubuntu/net10 coverage jobs fail because a filtered test module with zero selected tests times out in MTP's TRX process-lifetime handler after coverage is enabled. The module aborts with exit 134 (System.TimeoutException in TrxProcessLifetimeHandler), so the aggregate run reports an infrastructure error despite 0 failed tests. This occurs in BVT, SlowBVT, and Functional coverage jobs. Please adjust the coverage invocation/project selection so zero-selection modules complete cleanly without masking real failures.\n\nExample job: https://github.com/dotnet/orleans/actions/runs/32457710648/job/96698103245 |
There was a problem hiding this comment.
Review details
Suppressed comments (1)
.github/workflows/ci.yml:1100
- This condition skips uploading the
test_output_*artifact for pull_request runs on ubuntu-latest/net10.0. With the new coverage collection happening specifically in that matrix slice, skipping the artifact preventstest-results.ymlfrom reporting those suite outcomes (it only readstest_output_*artifacts). If coverage and test outcomes are both desired, upload test results unconditionally and keep excluding**/*.cobertura.xmlto avoid duplication.
- name: Archive Test Results
if: always() && (github.event_name != 'pull_request' || matrix.os != 'ubuntu-latest' || matrix.framework != 'net10.0')
continue-on-error: true
- Files reviewed: 6/6 changed files
- Comments generated: 1
- Review effort level: Lite
There was a problem hiding this comment.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
.github/workflows/test-results.yml:56
- This step invokes
python, which is not guaranteed to exist as an alias on allubuntu-latestimages (some only providepython3). Usingpython3makes the workflow more robust without changing behavior.
python .github/scripts/summarize-coverage.py coverage \
- Files reviewed: 6/6 changed files
- Comments generated: 1
- Review effort level: Lite
There was a problem hiding this comment.
Review details
Suppressed comments (2)
Previously missed (2) — in code that hasn't changed since the last review.
.github/workflows/ci.yml:1123
- The coverage job builds the repo with a plain
dotnet buildand then runsdotnet test --no-build --framework net10.0. Since test projects targetnet8.0;net10.0by default (seetest/Directory.Build.props:19), this build step will typically build both TFMs even though the job only runs the net10.0 tests, increasing job time. Consider dropping the explicit build step and lettingdotnet test --framework net10.0build only the required net10.0 graph.
- name: Build
run: dotnet build
- name: Collect coverage
run: dotnet test
--no-build
.github/scripts/summarize-coverage.py:36
get_repository_pathaccepts filenames which start with thesource_rootprefix (or/_/src/) but does not reject../.path segments. A crafted Cobertura report could therefore escapesrc/(e.g..../src/../.github/...) and skew the coverage calculation even though the summary claims it only measures files undersrc/. Consider rejecting filenames which contain./..segments after the prefix is stripped.
if normalized.casefold().startswith(source_prefix.casefold()):
return f"src/{normalized[len(source_prefix):]}"
if normalized.startswith(DETERMINISTIC_SOURCE_PREFIX):
return f"src/{normalized[len(DETERMINISTIC_SOURCE_PREFIX):]}"
- Files reviewed: 7/7 changed files
- Comments generated: 0 new
- Review effort level: Lite
4bf3682 to
445b1c4
Compare
445b1c4 to
87b4fdd
Compare
There was a problem hiding this comment.
Review details
Suppressed comments (1)
.github/workflows/test-results.yml:73
- A Python unit test file (
.github/scripts/test_summarize_coverage.py) was added, but this workflow job doesn't run it. Since this job runs the summarizer on trusted default-branch code, executing the unit tests here would help catch XML parsing/path-handling regressions early.
- name: Summarize coverage
id: coverage
run: |
python .github/scripts/summarize-coverage.py coverage \
--source-root "$GITHUB_WORKSPACE/src" \
--markdown-output coverage-summary.md \
--json-output coverage-summary.json
cat coverage-summary.md >> "$GITHUB_STEP_SUMMARY"
echo "line-rate=$(jq -r .line_rate_display coverage-summary.json)" >> "$GITHUB_OUTPUT"
- Files reviewed: 7/7 changed files
- Comments generated: 2
- Review effort level: Lite
98c3154 to
70fa7f5
Compare
Pull requests currently publish test outcomes but do not expose code coverage after the Microsoft.Testing.Platform migration.
This change enables native MTP dynamic coverage in the existing Linux/.NET 10 BVT, SlowBVT, and Functional test matrix cells, so coverage is collected during the original test executions. Those jobs upload compact binary coverage data separately from TRX results, and a post-test job uses Microsoft's
dotnet-coveragetool to merge every module and suite into one deduplicated Cobertura report.The trusted default-branch
workflow_runsummarizes that single report into aCode Coveragecheck, the job summary, and one marker-tagged PR comment which is updated on every successful run. The comment keeps the percentage and covered-line count visible, with source counts, scope, commit, workflow, and artifact links in collapsed details.Coverage is limited to maintained repository source under
src/, excluding test and build-output sources. The split keeps fork pull requests on read-only permissions while the reporter uses the default branch implementation and parses one bounded static XML artifact before writing results.Microsoft Reviewers: Open in CodeFlow