diff --git a/.anvil.lock b/.anvil.lock index 87055558..e88cb935 100644 --- a/.anvil.lock +++ b/.anvil.lock @@ -1,7 +1,7 @@ version = 1 tool = "anvil" tool_version = "0.5.0" -catalog_checksum = "sha256:b9e76e5dab6d2cd8cd2eaea04bd1b4457ea8839a688795981dc4cc9a415f8092" +catalog_checksum = "sha256:b2da3c025ce50918fae38da58f80625205984a4804a3c770c0cc3e66d005ebf0" [[file]] path = ".anvil/container/Containerfile" @@ -45,7 +45,7 @@ checksum = "sha256:9940d1947482150ac08fcb9b4150da99f5ae60642f4caeea137577ce0e709 [[file]] path = ".github/actions/anvil-run-group/action.yml" -checksum = "sha256:ff8def6c0786b6e146c4b633dfe38cb9b8ede398345516cca32bbcd5586087af" +checksum = "sha256:d564a0ce424cda58c8f5b2476cf0c8b36e759afbdf8227608bd1f3fc74913fdf" [[file]] path = ".github/actions/anvil-setup/action.yml" @@ -57,7 +57,7 @@ checksum = "sha256:ea44d5e1a2cb1471cf2cef05eceab846d8afa45cf691be800f21fec1218da [[file]] path = ".github/workflows/anvil-pr-impl.yml" -checksum = "sha256:518d267b6def1d2549800df52fbe32c5f0848b2b2d9c00d9480b86e07b54ee3a" +checksum = "sha256:de66fea0b7340f2dc6b2f7bd70d5230e0c0c67187b3cd6870214b1f4139a1616" [[file]] path = ".github/workflows/anvil-pr.yml" @@ -65,7 +65,7 @@ checksum = "sha256:0c2530d9a38e6a74e0a7fd4f999b4a1790f97de30b58b68c6c2344600da19 [[file]] path = ".github/workflows/anvil-scheduled-impl.yml" -checksum = "sha256:ac70061acf594c8c212c45ed97c3b653e7b8de68f4e1dcc9695a2628e4e2596d" +checksum = "sha256:da730340e961a0d7520856ce8e2a947e767d88cc8dbe3f534cf9a79452320244" [[file]] path = ".github/workflows/anvil-scheduled.yml" diff --git a/.github/actions/anvil-run-group/action.yml b/.github/actions/anvil-run-group/action.yml index ce3c159f..e60a6f22 100644 --- a/.github/actions/anvil-run-group/action.yml +++ b/.github/actions/anvil-run-group/action.yml @@ -72,6 +72,7 @@ runs: failed_recipe="$(sed -n 's/^error: recipe `\([^`]*\)` failed\( on line [0-9][0-9]*\)\{0,1\} with exit code [0-9][0-9]*$/\1/p' "$log" | tail -n 1)" echo "failed_recipe=${failed_recipe:-anvil-$ANVIL_GROUP}" >> "$GITHUB_OUTPUT" echo "exit_code=$status" >> "$GITHUB_OUTPUT" + exit "$status" # Reporting is supplemental: run after success or failure, but never let # an API outage determine the authoritative workflow-job result. @@ -84,8 +85,3 @@ runs: setup_outcome: ${{ steps.setup.outcome }} exit_code: ${{ steps.run.outputs.exit_code }} failed_recipe: ${{ steps.run.outputs.failed_recipe }} - - - name: "Failed Just recipe: ${{ steps.run.outputs.failed_recipe }}" - if: always() && steps.run.outputs.exit_code != '' && steps.run.outputs.exit_code != '0' - shell: bash - run: exit 1 diff --git a/.github/workflows/anvil-pr-impl.yml b/.github/workflows/anvil-pr-impl.yml index 116a15ce..75d2a000 100644 --- a/.github/workflows/anvil-pr-impl.yml +++ b/.github/workflows/anvil-pr-impl.yml @@ -211,10 +211,11 @@ jobs: # Codecov UI without changing the union total. # lcov-*.info are produced by the anvil-llvm-cov recipe inside # anvil-pr-test (one per feature config); if the affected set was - # empty the recipe no-ops and there is no file to upload, so we gate - # on the files existing (impact scoping lives in the downloaded cache - # now, not a job output). Codecov coalesces the two per-config files. - if: matrix.os != 'windows-arm' && hashFiles('target/coverage/lcov-all-features.info', 'target/coverage/lcov-no-default.info') != '' + # empty the recipe no-ops and there are no files to upload, so we gate + # on both files existing (impact scoping lives in the downloaded cache + # now, not a job output). Requiring both avoids publishing a partial + # result if the second feature configuration fails. + if: always() && matrix.os != 'windows-arm' && hashFiles('target/coverage/lcov-all-features.info') != '' && hashFiles('target/coverage/lcov-no-default.info') != '' uses: codecov/codecov-action@v7.0.0 # immutable release, the tag cannot be moved with: files: target/coverage/lcov-all-features.info,target/coverage/lcov-no-default.info diff --git a/.github/workflows/anvil-scheduled-impl.yml b/.github/workflows/anvil-scheduled-impl.yml index 982dc1ae..881b10e3 100644 --- a/.github/workflows/anvil-scheduled-impl.yml +++ b/.github/workflows/anvil-scheduled-impl.yml @@ -69,7 +69,7 @@ jobs: # Multi-flag tag combines the OS with a "scheduled" marker so # the Codecov UI can distinguish PR-tier uploads from scheduled # uploads while still tracking each platform separately. - if: matrix.os != 'windows-arm' + if: always() && matrix.os != 'windows-arm' && hashFiles('target/coverage/lcov-all-features.info') != '' && hashFiles('target/coverage/lcov-no-default.info') != '' uses: codecov/codecov-action@v7.0.0 # immutable release, the tag cannot be moved with: files: target/coverage/lcov-all-features.info,target/coverage/lcov-no-default.info diff --git a/crates/cargo-anvil/docs/design/github.md b/crates/cargo-anvil/docs/design/github.md index ec9fe696..20ac5eb1 100644 --- a/crates/cargo-anvil/docs/design/github.md +++ b/crates/cargo-anvil/docs/design/github.md @@ -534,6 +534,7 @@ runs: failed_recipe="$(sed -n 's/^error: recipe `\([^`]*\)` failed\( on line [0-9][0-9]*\)\{0,1\} with exit code [0-9][0-9]*$/\1/p' "$log" | tail -n 1)" echo "failed_recipe=${failed_recipe:-anvil-$ANVIL_GROUP}" >> "$GITHUB_OUTPUT" echo "exit_code=$status" >> "$GITHUB_OUTPUT" + exit "$status" - name: Publish supplemental Anvil commit status if: always() && inputs.publish_commit_statuses == 'true' && github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name == github.repository continue-on-error: true @@ -543,10 +544,6 @@ runs: setup_outcome: ${{ steps.setup.outcome }} exit_code: ${{ steps.run.outputs.exit_code }} failed_recipe: ${{ steps.run.outputs.failed_recipe }} - - name: "Failed Just recipe: ${{ steps.run.outputs.failed_recipe }}" - if: always() && steps.run.outputs.exit_code != '' && steps.run.outputs.exit_code != '0' - shell: bash - run: exit 1 ``` Input set on the shared group action: @@ -593,9 +590,10 @@ the following mechanisms, all driven by Just's existing terminal diagnostic: 1. The problem matcher registered by `anvil-setup` promotes ``error: recipe `anvil-license-headers` failed with exit code 1`` to a GitHub annotation. -2. The group composite ends with a failing step named - `Failed Just recipe: anvil-license-headers`, putting the recipe name in the - job's step list. +2. The `Run Anvil group` step itself returns Just's exit status after recording + the recipe name and exit code for supplemental reporting. The failed step is + therefore the step containing the complete, live recipe output; no + synthetic failure step can displace or truncate the underlying diagnostic. 3. On eligible pull requests, `anvil-report-status` publishes a commit status whose reserved context namespace names the failed recipe and runner: @@ -604,10 +602,12 @@ the following mechanisms, all driven by Just's existing terminal diagnostic: ``` The group action streams normal Just output, captures the terminal failed -recipe, reports supplemental presentation on a best-effort basis, and then -propagates Just's result to the authoritative workflow job. The reporter -neither invokes checks nor contains group membership. Internal capture, -parsing, status reconciliation, and test-harness details are documented in the +recipe, writes its outputs, and returns Just's status from that same step. +Subsequent reporting uses `always()` and is supplemental and best-effort, so it +still runs after a recipe failure without replacing the authoritative failed +step. The reporter neither invokes checks nor contains group membership. +Internal capture, parsing, status reconciliation, and test-harness details are +documented in the [implementation guide](../implementation.md#github-group-execution-and-status-reporting). When `publish_commit_statuses` is enabled, the shared reporter manages statuses @@ -801,7 +801,7 @@ The wiring never gates jobs on the impact result — every job runs regardless o status. This is intentional: unscoped checks (`deny`, `audit`, `aprz`, `pr-title`, `mutants-full`) must run on every PR even when every tier reports `--skip`. Steps that need a per-tier side decision read the downloaded cache file directly (e.g. the Codecov -upload is gated on the coverage files existing via `hashFiles(...)`), never on a job +upload is gated on both coverage files existing via `hashFiles(...)`), never on a job output. @@ -904,8 +904,11 @@ Recommended root workflow shape: ## 10. Coverage upload After `pr-test` (and `scheduled-test`) runs the `anvil-llvm-cov` recipe, the reusable -workflow uploads the resulting `target/coverage/lcov.info` to Codecov from every leg of -the matrix except `windows-11-arm`. The windows-arm leg is excluded because its +workflow uploads the resulting coverage files to Codecov from every leg of the matrix +except `windows-11-arm`. The upload condition uses `always()` plus a file-existence +guard: completed coverage reports are retained even when the coverage gate or a later +group recipe fails, while failures before both feature configurations complete do not +trigger an empty or partial upload. The windows-arm leg is excluded because its LLVM-coverage instrumentation produces `malformed instrumentation profile data: symbol name is empty` errors that make the profile unusable. Coverage from every other leg is necessary because OS/arch-gated code (`cfg(target_os = ...)`, `cfg(target_arch = ...)`) @@ -918,10 +921,10 @@ The upload step: ```yaml - name: Upload coverage to Codecov - if: matrix.os != 'windows-arm' && hashFiles('target/coverage/lcov-all-features.info', 'target/coverage/lcov-no-default.info') != '' + if: always() && matrix.os != 'windows-arm' && hashFiles('target/coverage/lcov-all-features.info') != '' && hashFiles('target/coverage/lcov-no-default.info') != '' uses: codecov/codecov-action@v7.0.0 # immutable release, the tag cannot be moved with: - files: target/coverage/lcov.info + files: target/coverage/lcov-all-features.info,target/coverage/lcov-no-default.info flags: ${{ matrix.os }} token: ${{ secrets.CODECOV_TOKEN }} fail_ci_if_error: false @@ -934,9 +937,10 @@ all; private repos set `CODECOV_TOKEN` at the repo level. `fail_ci_if_error: fal keeps the build green when Codecov is unreachable (typical for internal repos that can't reach `codecov.io`). -On the scheduled upload the step additionally combines the OS flag with a `scheduled` -marker (`flags: scheduled,${{ matrix.os }}`) so PR vs scheduled streams stay -distinguishable in the Codecov UI while still being queryable per-OS. +The scheduled upload has the same `always()` and file-existence semantics. It +additionally combines the OS flag with a `scheduled` marker +(`flags: scheduled,${{ matrix.os }}`) so PR vs scheduled streams stay distinguishable +in the Codecov UI while still being queryable per-OS. anvil does not gate the PR on coverage. The lcov upload is informational; Codecov's own status check is the gating layer when the adopter wants one (configured in Codecov, diff --git a/crates/cargo-anvil/docs/implementation.md b/crates/cargo-anvil/docs/implementation.md index 03bb2bcb..213f15ed 100644 --- a/crates/cargo-anvil/docs/implementation.md +++ b/crates/cargo-anvil/docs/implementation.md @@ -120,14 +120,22 @@ tests in `tests/impact.rs` exercise the real recipe rather than a CI-only path. ## GitHub group execution and status reporting -The generated `anvil-run-group` composite action owns the capture-before-failure -protocol. Its inline Bash step invokes Just through `tee`, temporarily disables -immediate exit, and reads `PIPESTATUS[0]` so the saved result belongs to Just -rather than `tee`. It selects the final standard Just failed-recipe diagnostic, -including the optional line-number form, and falls back to the group recipe -when a tool exits without that diagnostic. The step writes the recipe and exit -code as outputs without failing so the reporter can consume them. After -best-effort reporting, a final guarded step propagates the captured failure. +The generated `anvil-run-group` composite action owns the +capture-before-propagation protocol. Its inline Bash step invokes Just through +`tee`, temporarily disables immediate exit, and reads `PIPESTATUS[0]` so the +saved result belongs to Just rather than `tee`. It selects the final standard +Just failed-recipe diagnostic, including the optional line-number form, and +falls back to the group recipe when a tool exits without that diagnostic. The +step writes the recipe and exit code as outputs, then returns the captured +status itself. This is a correctness constraint for diagnostics: the GitHub +step marked failed must be the step containing the complete recipe output. +Moving propagation to a later synthetic step would make GitHub focus that +empty step and hide the useful output behind a successful predecessor. + +The reporter uses `always()`, so GitHub runs it after the group step fails and +the outputs written before propagation remain available to it. Its +`continue-on-error` remains necessary because supplemental API reporting must +not replace or obscure the authoritative recipe result. The status reporter is an inline `actions/github-script` body. It validates the pull-request head SHA, reads same-commit status history newest-first, and keeps diff --git a/crates/cargo-anvil/src/anvil/artifacts/github.rs b/crates/cargo-anvil/src/anvil/artifacts/github.rs index e4977457..8272b55e 100644 --- a/crates/cargo-anvil/src/anvil/artifacts/github.rs +++ b/crates/cargo-anvil/src/anvil/artifacts/github.rs @@ -189,7 +189,11 @@ mod tests { assert!(RUN_GROUP_ACTION.contains("group: ${{ inputs.group }}")); assert!(RUN_GROUP_ACTION.contains("free-disk-space: ${{ inputs.free-disk-space }}")); assert!(RUN_GROUP_ACTION.contains("status=${PIPESTATUS[0]}")); - assert!(RUN_GROUP_ACTION.contains("Failed Just recipe: ${{ steps.run.outputs.failed_recipe }}")); + assert!(RUN_GROUP_ACTION.contains("exit \"$status\"")); + assert!( + !RUN_GROUP_ACTION.contains("Failed Just recipe:"), + "failure propagation must stay in the step containing the recipe output" + ); assert!(RUN_GROUP_ACTION.contains("uses: ./.github/actions/anvil-report-status")); // Impact reaches scoped checks through the downloaded impact cache // (read via `_anvil-impact-include`), not threaded --package env vars; @@ -285,7 +289,7 @@ export -f just assert!( status.success(), - "the capture script must defer group failure to the named action step" + "a successful group must return success after exporting its result" ); assert!(outputs.contains("failed_recipe=anvil-pr-fast")); assert!(outputs.contains("exit_code=0")); @@ -297,10 +301,7 @@ export -f just let diagnostic = "error: recipe `anvil-license-headers` failed with exit code 17"; let (status, outputs) = run_group_step(diagnostic, 17); - assert!( - status.success(), - "the capture script must defer group failure to the named action step" - ); + assert_eq!(status.code(), Some(17), "the recipe-running step must return Just's status"); assert!(outputs.contains("failed_recipe=anvil-license-headers")); assert!(outputs.contains("exit_code=17")); } @@ -311,10 +312,7 @@ export -f just let diagnostic = "error: recipe `anvil-license-headers` failed on line 42 with exit code 17"; let (status, outputs) = run_group_step(diagnostic, 17); - assert!( - status.success(), - "the capture script must defer group failure to the named action step" - ); + assert_eq!(status.code(), Some(17), "the recipe-running step must return Just's status"); assert!(outputs.contains("failed_recipe=anvil-license-headers")); assert!(outputs.contains("exit_code=17")); } @@ -324,10 +322,7 @@ export -f just fn run_group_step_falls_back_to_group_without_terminal_diagnostic() { let (status, outputs) = run_group_step("unexpected tool failure", 9); - assert!( - status.success(), - "the capture script must defer group failure to the named action step" - ); + assert_eq!(status.code(), Some(9), "the recipe-running step must return Just's status"); assert!(outputs.contains("failed_recipe=anvil-pr-fast")); assert!(outputs.contains("exit_code=9")); } @@ -427,7 +422,11 @@ export -f just 1, "Codecov upload step should be declared exactly once (gated per-leg via `if:`)" ); - assert!(PR_IMPL_WORKFLOW.contains("matrix.os != 'windows-arm'")); + assert!(PR_IMPL_WORKFLOW.contains( + "if: always() && matrix.os != 'windows-arm' && \ + hashFiles('target/coverage/lcov-all-features.info') != '' && \ + hashFiles('target/coverage/lcov-no-default.info') != ''" + )); assert!(PR_IMPL_WORKFLOW.contains("flags: ${{ matrix.os }}")); assert_eq!( PR_IMPL_WORKFLOW.matches("permissions:").count(), @@ -456,6 +455,11 @@ export -f just } assert!(SCHEDULED_IMPL_WORKFLOW.contains("publish-failure:")); assert!(SCHEDULED_IMPL_WORKFLOW.contains("codecov/codecov-action")); + assert!(SCHEDULED_IMPL_WORKFLOW.contains( + "if: always() && matrix.os != 'windows-arm' && \ + hashFiles('target/coverage/lcov-all-features.info') != '' && \ + hashFiles('target/coverage/lcov-no-default.info') != ''" + )); assert!(SCHEDULED_IMPL_WORKFLOW.contains("vars.ANVIL_PUBLISH_FAILURE_ISSUE != 'false'")); assert!(SCHEDULED_IMPL_WORKFLOW.contains("contains(needs.*.result, 'failure')")); assert!(SCHEDULED_IMPL_WORKFLOW.contains("actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd")); diff --git a/crates/cargo-anvil/templates/github/pr-impl-workflow.yml b/crates/cargo-anvil/templates/github/pr-impl-workflow.yml index 116a15ce..75d2a000 100644 --- a/crates/cargo-anvil/templates/github/pr-impl-workflow.yml +++ b/crates/cargo-anvil/templates/github/pr-impl-workflow.yml @@ -211,10 +211,11 @@ jobs: # Codecov UI without changing the union total. # lcov-*.info are produced by the anvil-llvm-cov recipe inside # anvil-pr-test (one per feature config); if the affected set was - # empty the recipe no-ops and there is no file to upload, so we gate - # on the files existing (impact scoping lives in the downloaded cache - # now, not a job output). Codecov coalesces the two per-config files. - if: matrix.os != 'windows-arm' && hashFiles('target/coverage/lcov-all-features.info', 'target/coverage/lcov-no-default.info') != '' + # empty the recipe no-ops and there are no files to upload, so we gate + # on both files existing (impact scoping lives in the downloaded cache + # now, not a job output). Requiring both avoids publishing a partial + # result if the second feature configuration fails. + if: always() && matrix.os != 'windows-arm' && hashFiles('target/coverage/lcov-all-features.info') != '' && hashFiles('target/coverage/lcov-no-default.info') != '' uses: codecov/codecov-action@v7.0.0 # immutable release, the tag cannot be moved with: files: target/coverage/lcov-all-features.info,target/coverage/lcov-no-default.info diff --git a/crates/cargo-anvil/templates/github/run-group-action.yml b/crates/cargo-anvil/templates/github/run-group-action.yml index ce3c159f..e60a6f22 100644 --- a/crates/cargo-anvil/templates/github/run-group-action.yml +++ b/crates/cargo-anvil/templates/github/run-group-action.yml @@ -72,6 +72,7 @@ runs: failed_recipe="$(sed -n 's/^error: recipe `\([^`]*\)` failed\( on line [0-9][0-9]*\)\{0,1\} with exit code [0-9][0-9]*$/\1/p' "$log" | tail -n 1)" echo "failed_recipe=${failed_recipe:-anvil-$ANVIL_GROUP}" >> "$GITHUB_OUTPUT" echo "exit_code=$status" >> "$GITHUB_OUTPUT" + exit "$status" # Reporting is supplemental: run after success or failure, but never let # an API outage determine the authoritative workflow-job result. @@ -84,8 +85,3 @@ runs: setup_outcome: ${{ steps.setup.outcome }} exit_code: ${{ steps.run.outputs.exit_code }} failed_recipe: ${{ steps.run.outputs.failed_recipe }} - - - name: "Failed Just recipe: ${{ steps.run.outputs.failed_recipe }}" - if: always() && steps.run.outputs.exit_code != '' && steps.run.outputs.exit_code != '0' - shell: bash - run: exit 1 diff --git a/crates/cargo-anvil/templates/github/scheduled-impl-workflow.yml b/crates/cargo-anvil/templates/github/scheduled-impl-workflow.yml index 982dc1ae..881b10e3 100644 --- a/crates/cargo-anvil/templates/github/scheduled-impl-workflow.yml +++ b/crates/cargo-anvil/templates/github/scheduled-impl-workflow.yml @@ -69,7 +69,7 @@ jobs: # Multi-flag tag combines the OS with a "scheduled" marker so # the Codecov UI can distinguish PR-tier uploads from scheduled # uploads while still tracking each platform separately. - if: matrix.os != 'windows-arm' + if: always() && matrix.os != 'windows-arm' && hashFiles('target/coverage/lcov-all-features.info') != '' && hashFiles('target/coverage/lcov-no-default.info') != '' uses: codecov/codecov-action@v7.0.0 # immutable release, the tag cannot be moved with: files: target/coverage/lcov-all-features.info,target/coverage/lcov-no-default.info diff --git a/crates/cargo-anvil/tests/snapshots/snapshots__github_backend.snap b/crates/cargo-anvil/tests/snapshots/snapshots__github_backend.snap index 2bc8163f..4572a2ab 100644 --- a/crates/cargo-anvil/tests/snapshots/snapshots__github_backend.snap +++ b/crates/cargo-anvil/tests/snapshots/snapshots__github_backend.snap @@ -1508,6 +1508,7 @@ runs: failed_recipe="$(sed -n 's/^error: recipe `\([^`]*\)` failed\( on line [0-9][0-9]*\)\{0,1\} with exit code [0-9][0-9]*$/\1/p' "$log" | tail -n 1)" echo "failed_recipe=${failed_recipe:-anvil-$ANVIL_GROUP}" >> "$GITHUB_OUTPUT" echo "exit_code=$status" >> "$GITHUB_OUTPUT" + exit "$status" # Reporting is supplemental: run after success or failure, but never let # an API outage determine the authoritative workflow-job result. @@ -1521,11 +1522,6 @@ runs: exit_code: ${{ steps.run.outputs.exit_code }} failed_recipe: ${{ steps.run.outputs.failed_recipe }} - - name: "Failed Just recipe: ${{ steps.run.outputs.failed_recipe }}" - if: always() && steps.run.outputs.exit_code != '' && steps.run.outputs.exit_code != '0' - shell: bash - run: exit 1 - === .github/actions/anvil-setup/action.yml === # Copyright (c) Microsoft Corporation. # Licensed under the MIT License. @@ -1982,10 +1978,11 @@ jobs: # Codecov UI without changing the union total. # lcov-*.info are produced by the anvil-llvm-cov recipe inside # anvil-pr-test (one per feature config); if the affected set was - # empty the recipe no-ops and there is no file to upload, so we gate - # on the files existing (impact scoping lives in the downloaded cache - # now, not a job output). Codecov coalesces the two per-config files. - if: matrix.os != 'windows-arm' && hashFiles('target/coverage/lcov-all-features.info', 'target/coverage/lcov-no-default.info') != '' + # empty the recipe no-ops and there are no files to upload, so we gate + # on both files existing (impact scoping lives in the downloaded cache + # now, not a job output). Requiring both avoids publishing a partial + # result if the second feature configuration fails. + if: always() && matrix.os != 'windows-arm' && hashFiles('target/coverage/lcov-all-features.info') != '' && hashFiles('target/coverage/lcov-no-default.info') != '' uses: codecov/codecov-action@v7.0.0 # immutable release, the tag cannot be moved with: files: target/coverage/lcov-all-features.info,target/coverage/lcov-no-default.info @@ -2188,7 +2185,7 @@ jobs: # Multi-flag tag combines the OS with a "scheduled" marker so # the Codecov UI can distinguish PR-tier uploads from scheduled # uploads while still tracking each platform separately. - if: matrix.os != 'windows-arm' + if: always() && matrix.os != 'windows-arm' && hashFiles('target/coverage/lcov-all-features.info') != '' && hashFiles('target/coverage/lcov-no-default.info') != '' uses: codecov/codecov-action@v7.0.0 # immutable release, the tag cannot be moved with: files: target/coverage/lcov-all-features.info,target/coverage/lcov-no-default.info diff --git a/crates/cargo-coverage-gate/README.md b/crates/cargo-coverage-gate/README.md index 293cba5e..1d43b0bf 100644 --- a/crates/cargo-coverage-gate/README.md +++ b/crates/cargo-coverage-gate/README.md @@ -20,7 +20,10 @@ by [`cargo-llvm-cov`][__link0] against per-package thresholds carried in `Cargo.toml`. The accompanying `cargo-coverage-gate` binary reads the coverage lcov tracefile, resolves each package’s threshold from a small three-layer lookup, and emits a verdict table to stdout (and, -optionally, to a Markdown summary file for CI step summaries). +optionally, to a Markdown summary file for CI step summaries). A failing +verdict includes exact covered/coverable counts and the uncovered source +line ranges, so the output identifies what must be covered without relying +on a later coverage-service upload. ### Threshold resolution @@ -105,7 +108,7 @@ plus the appropriate exit code. This crate was developed as part of The Oxidizer Project. Browse this crate's source code. - [__cargo_doc2readme_dependencies_info]: ggGmYW0CYXZlMC43LjNhdIQbFhzZ8rzWNNYbuRaDSGWynFgbH4PMdoT7GNcbVwNPtPjAhvFhYvRhcoQbDzRwf0qddWQbQiTzhu0-bE0bX-rutkvfDuYbITgXvtMXiRVhZIGDc2NhcmdvLWNvdmVyYWdlLWdhdGVlMC4zLjBzY2FyZ29fY292ZXJhZ2VfZ2F0ZQ + [__cargo_doc2readme_dependencies_info]: ggGmYW0CYXZlMC43LjNhdIQbFhzZ8rzWNNYbuRaDSGWynFgbH4PMdoT7GNcbVwNPtPjAhvFhYvRhcoQbUyBJhTlThKMb2KAsMNOODUYbjhYEhuJ2xScbr_AgFmRNpuZhZIGDc2NhcmdvLWNvdmVyYWdlLWdhdGVlMC4zLjBzY2FyZ29fY292ZXJhZ2VfZ2F0ZQ [__link0]: https://github.com/taiki-e/cargo-llvm-cov [__link1]: https://docs.rs/cargo-coverage-gate/0.3.0/cargo_coverage_gate/fn.evaluate.html [__link2]: https://docs.rs/cargo-coverage-gate/0.3.0/cargo_coverage_gate/struct.EvaluatedReport.html diff --git a/crates/cargo-coverage-gate/docs/design/README.md b/crates/cargo-coverage-gate/docs/design/README.md index 7d6b1d14..89e78a8d 100644 --- a/crates/cargo-coverage-gate/docs/design/README.md +++ b/crates/cargo-coverage-gate/docs/design/README.md @@ -255,6 +255,32 @@ columns render `(no lines)` and `—`, since there is no percentage floor. Markdown variant uses the same columns and a leading `### coverage-gate` header so it renders cleanly in GitHub job summaries and ADO build summaries. +When the verdict is not a pass, both renderers append **failure details**. +For a package below its numeric threshold, the details show exact +covered/coverable line counts and the uncovered package-relative source +locations. For a package that unexpectedly contains coverable lines, they +show those locations instead. A `NO DATA` package gets an explicit statement +that no coverage records were attributed to it. + +Locations are ordered by package, file, and line, and contiguous lines are +rendered as ranges. To keep CI logs and summaries bounded, at most the first +100 relevant line locations are shown per package; the renderer reports how +many additional locations were omitted. The aggregate counts always describe +the complete input, not the displayed subset. + +```text +Failure details: + beta: 60/100 lines covered; 40 uncovered. + src/lib.rs: 61-100 + gamma: 91/100 lines covered; 9 uncovered. + src/lib.rs: 12, 24-27, 83-86 +``` + +The details are part of the gate output rather than delegated to an external +coverage service or a transient CI artifact. A failed local command and a +failed CI stage therefore retain enough information to identify what must be +covered even when a later upload step is skipped. + ### 5.5 Local invocation ```sh @@ -295,6 +321,8 @@ For each `SF:` section the tool counts: - `lines_total` — number of distinct `DA:` records (executable lines the instrumentation knows about). - `lines_covered` — number of those with a non-zero hit count. +- The line numbers of all `DA:` records and of the zero-count subset, + retained for failure diagnostics. Records other than `SF:` / `DA:` / `LF:` / `LH:` (function `FN:`, branch `BRDA:`, etc.) are accepted by the parser but not used; the diff --git a/crates/cargo-coverage-gate/src/aggregate.rs b/crates/cargo-coverage-gate/src/aggregate.rs index 1a9419f3..e2b02acb 100644 --- a/crates/cargo-coverage-gate/src/aggregate.rs +++ b/crates/cargo-coverage-gate/src/aggregate.rs @@ -60,6 +60,8 @@ mod tests { filename: PathBuf::from(path), lines_total: count, lines_covered: covered, + coverable_lines: (1..=count).collect(), + uncovered_lines: ((covered + 1)..=count).collect(), } } diff --git a/crates/cargo-coverage-gate/src/attribute.rs b/crates/cargo-coverage-gate/src/attribute.rs index 37efc121..18112931 100644 --- a/crates/cargo-coverage-gate/src/attribute.rs +++ b/crates/cargo-coverage-gate/src/attribute.rs @@ -61,6 +61,8 @@ mod tests { filename: PathBuf::from(path), lines_total: 10, lines_covered: 5, + coverable_lines: (1..=10).collect(), + uncovered_lines: (6..=10).collect(), } } diff --git a/crates/cargo-coverage-gate/src/lcov_cov.rs b/crates/cargo-coverage-gate/src/lcov_cov.rs index 8ef9f33a..8adacbc0 100644 --- a/crates/cargo-coverage-gate/src/lcov_cov.rs +++ b/crates/cargo-coverage-gate/src/lcov_cov.rs @@ -51,6 +51,10 @@ pub(crate) struct FileReport { pub(crate) lines_total: u32, /// Number of those lines hit at least once across the run. pub(crate) lines_covered: u32, + /// Instrumented source line numbers in ascending order. + pub(crate) coverable_lines: Vec, + /// Instrumented source line numbers with no hits, in ascending order. + pub(crate) uncovered_lines: Vec, } impl CoverageReport { @@ -120,18 +124,26 @@ impl CoverageReport { for (key, section) in report.sections { let mut total: u32 = 0; let mut covered: u32 = 0; - for data in section.lines.values() { + let mut coverable_lines = Vec::with_capacity(section.lines.len()); + let mut uncovered_lines = Vec::new(); + for (key, data) in §ion.lines { total = total.saturating_add(1); + coverable_lines.push(key.line); if data.count > 0 { covered = covered.saturating_add(1); + } else { + uncovered_lines.push(key.line); } } files.push(FileReport { filename: key.source_file, lines_total: total, lines_covered: covered, + coverable_lines, + uncovered_lines, }); } + files.sort_by(|a, b| a.filename.cmp(&b.filename)); Self { files } } } @@ -161,6 +173,8 @@ mod tests { // Fixture has 4 instrumented lines, 3 hit. assert_eq!(f.lines_total, 4); assert_eq!(f.lines_covered, 3); + assert_eq!(f.coverable_lines, vec![1, 2, 3, 4]); + assert_eq!(f.uncovered_lines, vec![3]); } #[test] @@ -232,6 +246,8 @@ end_of_record let f = &report.files[0]; assert_eq!(f.lines_total, 4, "line set is the union across configs"); assert_eq!(f.lines_covered, 3, "covered if hit in either config"); + assert_eq!(f.coverable_lines, vec![1, 2, 3, 4]); + assert_eq!(f.uncovered_lines, vec![4]); } #[test] diff --git a/crates/cargo-coverage-gate/src/lib.rs b/crates/cargo-coverage-gate/src/lib.rs index 3325e701..7737aaad 100644 --- a/crates/cargo-coverage-gate/src/lib.rs +++ b/crates/cargo-coverage-gate/src/lib.rs @@ -11,7 +11,10 @@ //! `Cargo.toml`. The accompanying `cargo-coverage-gate` binary reads the //! coverage lcov tracefile, resolves each package's threshold from a small //! three-layer lookup, and emits a verdict table to stdout (and, -//! optionally, to a Markdown summary file for CI step summaries). +//! optionally, to a Markdown summary file for CI step summaries). A failing +//! verdict includes exact covered/coverable counts and the uncovered source +//! line ranges, so the output identifies what must be covered without relying +//! on a later coverage-service upload. //! //! ## Threshold resolution //! diff --git a/crates/cargo-coverage-gate/src/render/markdown.rs b/crates/cargo-coverage-gate/src/render/markdown.rs index 824131f3..46b1b706 100644 --- a/crates/cargo-coverage-gate/src/render/markdown.rs +++ b/crates/cargo-coverage-gate/src/render/markdown.rs @@ -10,7 +10,10 @@ use std::io; -use crate::render::{files, format_delta, format_lines, format_source, format_status_markdown, format_threshold, result_summary}; +use crate::render::{ + MAX_DIAGNOSTIC_LINES, diagnostic_line_count, failure_detail, files, format_delta, format_line_ranges, format_lines, format_source, + format_status_markdown, format_threshold, result_summary, +}; use crate::verdict::Report; /// Render `report` as a GFM table to `out`. @@ -34,6 +37,7 @@ pub(crate) fn render(out: &mut dyn io::Write, report: &Report) -> io::Result<()> writeln!(out)?; writeln!(out, "**Result:** {}", result_summary(&report.outcomes))?; + write_failure_details(out, report)?; if report.unattributed > 0 { writeln!( out, @@ -44,13 +48,49 @@ pub(crate) fn render(out: &mut dyn io::Write, report: &Report) -> io::Result<()> Ok(()) } +fn write_failure_details(out: &mut dyn io::Write, report: &Report) -> io::Result<()> { + let failures: Vec<_> = report + .outcomes + .iter() + .filter_map(|outcome| failure_detail(outcome).map(|detail| (outcome, detail))) + .collect(); + if failures.is_empty() { + return Ok(()); + } + + writeln!(out)?; + writeln!(out, "#### Failure details")?; + for (outcome, detail) in failures { + writeln!(out, "- **{}:** {}", outcome.name, detail)?; + let mut remaining = MAX_DIAGNOSTIC_LINES; + for diagnostic in &outcome.diagnostics { + if remaining == 0 { + break; + } + let displayed = diagnostic.lines.len().min(remaining); + writeln!( + out, + " - `{}`: {}", + diagnostic.path.display(), + format_line_ranges(&diagnostic.lines[..displayed]) + )?; + remaining -= displayed; + } + let omitted = diagnostic_line_count(outcome).saturating_sub(MAX_DIAGNOSTIC_LINES); + if omitted > 0 { + writeln!(out, " - ... {omitted} more line locations omitted")?; + } + } + Ok(()) +} + #[cfg(test)] #[cfg_attr(coverage_nightly, coverage(off))] mod tests { use super::*; use crate::aggregate::LineTotals; use crate::threshold::{Threshold, ThresholdSource}; - use crate::verdict::{PackageOutcome, Status}; + use crate::verdict::{LineDiagnostic, PackageOutcome, Status}; fn outcome(name: &str, count: u32, covered: u32, threshold: f64, source: ThresholdSource, status: Status) -> PackageOutcome { PackageOutcome { @@ -61,6 +101,7 @@ mod tests { }, totals: LineTotals { count, covered }, status, + diagnostics: Vec::new(), } } @@ -84,17 +125,65 @@ mod tests { #[test] fn uses_check_emoji_for_pass_and_cross_for_fail() { + let mut beta = outcome("beta", 100, 50, 80.0, ThresholdSource::Workspace, Status::Fail); + beta.diagnostics.push(LineDiagnostic { + path: "src/lib.rs".into(), + lines: vec![51, 52, 60], + }); let report = Report { - outcomes: vec![ - outcome("alpha", 100, 95, 80.0, ThresholdSource::Package, Status::Ok), - outcome("beta", 100, 50, 80.0, ThresholdSource::Workspace, Status::Fail), - ], + outcomes: vec![outcome("alpha", 100, 95, 80.0, ThresholdSource::Package, Status::Ok), beta], unattributed: 0, }; let s = render_to_string(&report); assert!(s.contains("| ✅ |")); assert!(s.contains("| ❌ |")); assert!(s.contains("1 package below threshold")); + assert!(s.contains("#### Failure details")); + assert!(s.contains("**beta:** 50/100 lines covered; 50 uncovered.")); + assert!(s.contains("`src/lib.rs`: 51-52, 60")); + } + + #[test] + fn failure_detail_limit_spans_diagnostic_files() { + let mut failed = outcome("alpha", 120, 0, 80.0, ThresholdSource::Package, Status::Fail); + failed.diagnostics.push(LineDiagnostic { + path: "src/first.rs".into(), + lines: (1..=60).collect(), + }); + failed.diagnostics.push(LineDiagnostic { + path: "src/second.rs".into(), + lines: (101..=160).collect(), + }); + failed.diagnostics.push(LineDiagnostic { + path: "src/third.rs".into(), + lines: vec![200], + }); + let report = Report { + outcomes: vec![failed], + unattributed: 0, + }; + let s = render_to_string(&report); + assert!(s.contains("`src/first.rs`: 1-60"), "got:\n{s}"); + assert!(s.contains("`src/second.rs`: 101-140"), "got:\n{s}"); + assert!(!s.contains("`src/second.rs`: 101-160"), "got:\n{s}"); + assert!(!s.contains("src/third.rs"), "got:\n{s}"); + assert!(s.contains("21 more line locations omitted"), "got:\n{s}"); + } + + #[test] + fn exact_failure_detail_limit_has_no_omission_notice() { + let mut failed = outcome("alpha", 100, 0, 80.0, ThresholdSource::Package, Status::Fail); + failed.diagnostics.push(LineDiagnostic { + path: "src/lib.rs".into(), + lines: (1..=100).collect(), + }); + let report = Report { + outcomes: vec![failed], + unattributed: 0, + }; + let s = render_to_string(&report); + assert!(s.contains("`src/lib.rs`: 1-100"), "got:\n{s}"); + assert!(!s.contains("more line locations omitted"), "got:\n{s}"); } #[test] @@ -213,6 +302,23 @@ mod tests { assert!(render(&mut w, &report).is_err()); } + #[test] + fn propagates_error_from_failure_diagnostic_write() { + let mut failed = outcome("alpha", 1, 0, 80.0, ThresholdSource::Package, Status::Fail); + failed.diagnostics.push(LineDiagnostic { + path: "src/DIAGNOSTIC_WRITE_FAIL.rs".into(), + lines: vec![1], + }); + let report = Report { + outcomes: vec![failed], + unattributed: 0, + }; + let mut w = FailOnNeedle { + needle: b"DIAGNOSTIC_WRITE_FAIL", + }; + assert!(render(&mut w, &report).is_err()); + } + #[test] fn propagates_error_from_unattributed_note_write() { // "had paths" appears only in the unattributed-note `writeln!`, diff --git a/crates/cargo-coverage-gate/src/render/mod.rs b/crates/cargo-coverage-gate/src/render/mod.rs index 00bfdd5d..6e7aecfa 100644 --- a/crates/cargo-coverage-gate/src/render/mod.rs +++ b/crates/cargo-coverage-gate/src/render/mod.rs @@ -20,6 +20,8 @@ pub(crate) mod text; use crate::threshold::ThresholdSource; use crate::verdict::{PackageOutcome, Status}; +pub(crate) const MAX_DIAGNOSTIC_LINES: usize = 100; + /// Human-readable text for the `Lines` column. fn format_lines(outcome: &PackageOutcome) -> String { match outcome.status { @@ -155,6 +157,54 @@ fn files(n: usize) -> String { plural(n, "file", "files") } +fn failure_detail(outcome: &PackageOutcome) -> Option { + match outcome.status { + Status::Fail => { + let uncovered = outcome.totals.count.saturating_sub(outcome.totals.covered); + Some(format!( + "{}/{} lines covered; {} uncovered.", + outcome.totals.covered, outcome.totals.count, uncovered + )) + } + Status::NoData => Some("no coverage records were attributed to this package.".to_owned()), + Status::UnexpectedCoverableLines => Some(format!( + "expected no coverable lines; found {}.", + plural(outcome.totals.count as usize, "line", "lines") + )), + Status::Ok | Status::NoCoverableLines => None, + } +} + +fn diagnostic_line_count(outcome: &PackageOutcome) -> usize { + outcome.diagnostics.iter().map(|diagnostic| diagnostic.lines.len()).sum() +} + +fn format_line_ranges(lines: &[u32]) -> String { + let mut ranges = Vec::new(); + let Some((&first, rest)) = lines.split_first() else { + return String::new(); + }; + let mut start = first; + let mut end = first; + for &line in rest { + if line != end.saturating_add(1) { + push_line_range(&mut ranges, start, end); + start = line; + } + end = line; + } + push_line_range(&mut ranges, start, end); + ranges.join(", ") +} + +fn push_line_range(ranges: &mut Vec, start: u32, end: u32) { + if start == end { + ranges.push(start.to_string()); + } else { + ranges.push(format!("{start}-{end}")); + } +} + #[cfg(test)] #[cfg_attr(coverage_nightly, coverage(off))] mod tests { @@ -171,6 +221,7 @@ mod tests { }, totals: LineTotals { count, covered }, status: Status::Ok, + diagnostics: Vec::new(), } } @@ -183,6 +234,7 @@ mod tests { }, totals: LineTotals { count, covered }, status, + diagnostics: Vec::new(), } } @@ -232,6 +284,13 @@ mod tests { assert_eq!(format_delta(&o), "—"); } + #[test] + fn line_ranges_compress_only_adjacent_lines() { + assert_eq!(format_line_ranges(&[]), ""); + assert_eq!(format_line_ranges(&[7]), "7"); + assert_eq!(format_line_ranges(&[1, 2, 3, 5, 7, 8]), "1-3, 5, 7-8"); + } + #[test] fn plural_helpers_pick_singular_only_for_one() { assert_eq!(packages(0), "0 packages"); diff --git a/crates/cargo-coverage-gate/src/render/text.rs b/crates/cargo-coverage-gate/src/render/text.rs index 7c8af320..ffa7bee7 100644 --- a/crates/cargo-coverage-gate/src/render/text.rs +++ b/crates/cargo-coverage-gate/src/render/text.rs @@ -5,7 +5,10 @@ use std::io; -use crate::render::{files, format_delta, format_lines, format_source, format_status_text, format_threshold, result_summary}; +use crate::render::{ + MAX_DIAGNOSTIC_LINES, diagnostic_line_count, failure_detail, files, format_delta, format_line_ranges, format_lines, format_source, + format_status_text, format_threshold, result_summary, +}; use crate::verdict::Report; const HEADERS: [&str; 6] = ["Package", "Lines", "Threshold", "Δ vs threshold", "Status", "Source"]; @@ -48,6 +51,7 @@ pub(crate) fn render(out: &mut dyn io::Write, report: &Report) -> io::Result<()> write_separator(out, &widths)?; writeln!(out, "Result: {}", result_summary(&report.outcomes))?; + write_failure_details(out, report)?; if report.unattributed > 0 { writeln!( out, @@ -58,6 +62,42 @@ pub(crate) fn render(out: &mut dyn io::Write, report: &Report) -> io::Result<()> Ok(()) } +fn write_failure_details(out: &mut dyn io::Write, report: &Report) -> io::Result<()> { + let failures: Vec<_> = report + .outcomes + .iter() + .filter_map(|outcome| failure_detail(outcome).map(|detail| (outcome, detail))) + .collect(); + if failures.is_empty() { + return Ok(()); + } + + writeln!(out)?; + writeln!(out, "Failure details:")?; + for (outcome, detail) in failures { + writeln!(out, " {}: {}", outcome.name, detail)?; + let mut remaining = MAX_DIAGNOSTIC_LINES; + for diagnostic in &outcome.diagnostics { + if remaining == 0 { + break; + } + let displayed = diagnostic.lines.len().min(remaining); + writeln!( + out, + " {}: {}", + diagnostic.path.display(), + format_line_ranges(&diagnostic.lines[..displayed]) + )?; + remaining -= displayed; + } + let omitted = diagnostic_line_count(outcome).saturating_sub(MAX_DIAGNOSTIC_LINES); + if omitted > 0 { + writeln!(out, " ... {omitted} more line locations omitted")?; + } + } + Ok(()) +} + fn write_row(out: &mut dyn io::Write, row: &[String; 6], widths: &[usize; 6]) -> io::Result<()> { writeln!( out, @@ -92,7 +132,7 @@ mod tests { use super::*; use crate::aggregate::LineTotals; use crate::threshold::{Threshold, ThresholdSource}; - use crate::verdict::{PackageOutcome, Status}; + use crate::verdict::{LineDiagnostic, PackageOutcome, Status}; fn outcome(name: &str, count: u32, covered: u32, threshold: f64, source: ThresholdSource, status: Status) -> PackageOutcome { PackageOutcome { @@ -103,6 +143,7 @@ mod tests { }, totals: LineTotals { count, covered }, status, + diagnostics: Vec::new(), } } @@ -144,11 +185,13 @@ mod tests { #[test] fn renders_fail_with_negative_delta() { + let mut beta = outcome("beta", 100, 60, 80.0, ThresholdSource::Workspace, Status::Fail); + beta.diagnostics.push(LineDiagnostic { + path: "src/lib.rs".into(), + lines: (61..=100).collect(), + }); let report = Report { - outcomes: vec![ - outcome("alpha", 100, 95, 80.0, ThresholdSource::Package, Status::Ok), - outcome("beta", 100, 60, 80.0, ThresholdSource::Workspace, Status::Fail), - ], + outcomes: vec![outcome("alpha", 100, 95, 80.0, ThresholdSource::Package, Status::Ok), beta], unattributed: 0, }; let s = render_to_string(&report); @@ -157,6 +200,68 @@ mod tests { assert!(s.contains("FAIL")); assert!(s.contains("workspace")); assert!(s.contains("1 package below threshold")); + assert!(s.contains("beta: 60/100 lines covered; 40 uncovered.")); + assert!(s.contains("src/lib.rs: 61-100")); + } + + #[test] + fn failure_details_are_bounded() { + let mut failed = outcome("alpha", 200, 0, 80.0, ThresholdSource::Package, Status::Fail); + failed.diagnostics.push(LineDiagnostic { + path: "src/lib.rs".into(), + lines: (1..=200).collect(), + }); + let report = Report { + outcomes: vec![failed], + unattributed: 0, + }; + let s = render_to_string(&report); + assert!(s.contains("src/lib.rs: 1-100"), "got:\n{s}"); + assert!(s.contains("100 more line locations omitted"), "got:\n{s}"); + assert!(!s.contains("1-200"), "got:\n{s}"); + } + + #[test] + fn failure_detail_limit_spans_diagnostic_files() { + let mut failed = outcome("alpha", 120, 0, 80.0, ThresholdSource::Package, Status::Fail); + failed.diagnostics.push(LineDiagnostic { + path: "src/first.rs".into(), + lines: (1..=60).collect(), + }); + failed.diagnostics.push(LineDiagnostic { + path: "src/second.rs".into(), + lines: (101..=160).collect(), + }); + failed.diagnostics.push(LineDiagnostic { + path: "src/third.rs".into(), + lines: vec![200], + }); + let report = Report { + outcomes: vec![failed], + unattributed: 0, + }; + let s = render_to_string(&report); + assert!(s.contains("src/first.rs: 1-60"), "got:\n{s}"); + assert!(s.contains("src/second.rs: 101-140"), "got:\n{s}"); + assert!(!s.contains("src/second.rs: 101-160"), "got:\n{s}"); + assert!(!s.contains("src/third.rs"), "got:\n{s}"); + assert!(s.contains("21 more line locations omitted"), "got:\n{s}"); + } + + #[test] + fn exact_failure_detail_limit_has_no_omission_notice() { + let mut failed = outcome("alpha", 100, 0, 80.0, ThresholdSource::Package, Status::Fail); + failed.diagnostics.push(LineDiagnostic { + path: "src/lib.rs".into(), + lines: (1..=100).collect(), + }); + let report = Report { + outcomes: vec![failed], + unattributed: 0, + }; + let s = render_to_string(&report); + assert!(s.contains("src/lib.rs: 1-100"), "got:\n{s}"); + assert!(!s.contains("more line locations omitted"), "got:\n{s}"); } #[test] @@ -171,6 +276,7 @@ mod tests { assert!(s.contains("default")); assert!(s.contains("—")); assert!(s.contains("no attributed coverage data")); + assert!(s.contains("gamma: no coverage records were attributed to this package.")); } #[test] @@ -202,21 +308,21 @@ mod tests { #[test] fn renders_unexpected_coverable_lines_row_and_summary() { + let mut alpha = outcome("alpha", 7, 0, 0.0, ThresholdSource::Package, Status::UnexpectedCoverableLines); + alpha.diagnostics.push(LineDiagnostic { + path: "src/lib.rs".into(), + lines: vec![2, 3, 9], + }); let report = Report { - outcomes: vec![outcome( - "alpha", - 7, - 0, - 0.0, - ThresholdSource::Package, - Status::UnexpectedCoverableLines, - )], + outcomes: vec![alpha], unattributed: 0, }; let s = render_to_string(&report); assert!(s.contains("NOT EMPTY")); assert!(s.contains("7 lines")); assert!(s.contains("1 package with unexpected coverable lines")); + assert!(s.contains("alpha: expected no coverable lines; found 7 lines.")); + assert!(s.contains("src/lib.rs: 2-3, 9")); } #[test] @@ -284,4 +390,21 @@ mod tests { let mut w = FailOnNeedle { needle: b"had paths" }; assert!(render(&mut w, &report).is_err()); } + + #[test] + fn propagates_error_from_failure_diagnostic_write() { + let mut failed = outcome("alpha", 1, 0, 80.0, ThresholdSource::Package, Status::Fail); + failed.diagnostics.push(LineDiagnostic { + path: "src/DIAGNOSTIC_WRITE_FAIL.rs".into(), + lines: vec![1], + }); + let report = Report { + outcomes: vec![failed], + unattributed: 0, + }; + let mut w = FailOnNeedle { + needle: b"DIAGNOSTIC_WRITE_FAIL", + }; + assert!(render(&mut w, &report).is_err()); + } } diff --git a/crates/cargo-coverage-gate/src/verdict.rs b/crates/cargo-coverage-gate/src/verdict.rs index ff23d9e5..4d494c8c 100644 --- a/crates/cargo-coverage-gate/src/verdict.rs +++ b/crates/cargo-coverage-gate/src/verdict.rs @@ -14,6 +14,7 @@ //! [`threshold`]: crate::threshold use std::collections::HashSet; +use std::path::PathBuf; use crate::Verdict; use crate::aggregate::{LineTotals, aggregate}; @@ -55,6 +56,18 @@ pub(crate) struct PackageOutcome { pub(crate) totals: LineTotals, /// Outcome of the comparison. pub(crate) status: Status, + /// Source locations relevant to a failing outcome. + pub(crate) diagnostics: Vec, +} + +/// Relevant source lines from one file in a failing package. +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct LineDiagnostic { + /// Path relative to the package manifest directory when possible. + pub(crate) path: PathBuf, + /// Uncovered lines for a numeric failure, or all coverable lines for + /// an `expect-no-coverable-lines` failure. + pub(crate) lines: Vec, } impl PackageOutcome { @@ -129,11 +142,13 @@ pub(crate) fn evaluate(report: &CoverageReport, workspace: &Workspace, gated_pac let status = classify(totals, threshold); (threshold, status) }; + let diagnostics = diagnostics(attrib, m, status); PackageOutcome { name: m.name.clone(), threshold, totals, status, + diagnostics, } }) .collect(); @@ -145,6 +160,32 @@ pub(crate) fn evaluate(report: &CoverageReport, workspace: &Workspace, gated_pac }) } +fn diagnostics(files: &[&crate::lcov_cov::FileReport], member: &Member, status: Status) -> Vec { + let mut diagnostics: Vec = files + .iter() + .filter_map(|file| { + let lines = match status { + Status::Fail => &file.uncovered_lines, + Status::UnexpectedCoverableLines => &file.coverable_lines, + Status::Ok | Status::NoData | Status::NoCoverableLines => return None, + }; + if lines.is_empty() { + return None; + } + Some(LineDiagnostic { + path: file + .filename + .strip_prefix(&member.manifest_dir) + .unwrap_or(&file.filename) + .to_path_buf(), + lines: lines.clone(), + }) + }) + .collect(); + diagnostics.sort_by(|a, b| a.path.cmp(&b.path)); + diagnostics +} + /// Resolve `packages` (each a cargo-style selector) against the /// workspace. /// @@ -278,8 +319,6 @@ fn classify_no_coverable_lines(totals: LineTotals) -> Status { #[cfg(test)] #[cfg_attr(coverage_nightly, coverage(off))] mod tests { - use std::path::PathBuf; - use super::*; use crate::lcov_cov::FileReport; @@ -288,6 +327,8 @@ mod tests { filename: PathBuf::from(path), lines_total: count, lines_covered: covered, + coverable_lines: (1..=count).collect(), + uncovered_lines: ((covered + 1)..=count).collect(), } } @@ -356,6 +397,26 @@ mod tests { let beta = r.outcomes.iter().find(|o| o.name == "beta").unwrap(); assert_eq!(beta.status, Status::Fail); assert!((beta.percent().unwrap() - 60.0).abs() < f64::EPSILON); + assert_eq!(beta.diagnostics[0].path, PathBuf::from("src/lib.rs")); + assert_eq!(beta.diagnostics[0].lines, (61..=100).collect::>()); + } + + #[test] + fn unexpected_coverable_lines_report_every_instrumented_location() { + let report = make_report(vec![make_file("/repo/crates/alpha/src/lib.rs", 4, 2)]); + let ws = make_workspace(vec![make_member_expect_empty("alpha", "/repo/crates/alpha")], None); + let evaluated = evaluate(&report, &ws, &[]).expect("evaluate"); + let alpha = &evaluated.outcomes[0]; + assert_eq!(alpha.status, Status::UnexpectedCoverableLines); + assert_eq!(alpha.diagnostics[0].path, PathBuf::from("src/lib.rs")); + assert_eq!(alpha.diagnostics[0].lines, vec![1, 2, 3, 4]); + } + + #[test] + fn diagnostics_omit_files_without_relevant_lines() { + let file = make_file("/repo/crates/alpha/src/lib.rs", 0, 0); + let member = make_member("alpha", "/repo/crates/alpha", Some(80.0)); + assert!(diagnostics(&[&file], &member, Status::Fail).is_empty()); } #[test] diff --git a/crates/cargo-coverage-gate/tests/cli.rs b/crates/cargo-coverage-gate/tests/cli.rs index 1800cb26..0e0ad379 100644 --- a/crates/cargo-coverage-gate/tests/cli.rs +++ b/crates/cargo-coverage-gate/tests/cli.rs @@ -145,7 +145,9 @@ fn one_crate_below_threshold_exits_1() { .assert() .code(1) .stdout(predicate::str::contains("FAIL")) - .stdout(predicate::str::contains("1 package below threshold")); + .stdout(predicate::str::contains("1 package below threshold")) + .stdout(predicate::str::contains("beta: 60/100 lines covered; 40 uncovered.")) + .stdout(predicate::str::contains("src/lib.rs: 61-100")); } #[test]