diff --git a/.anvil.lock b/.anvil.lock index 94777ac04..79fc1184a 100644 --- a/.anvil.lock +++ b/.anvil.lock @@ -1,7 +1,7 @@ version = 1 tool = "anvil" tool_version = "0.6.0" -catalog_checksum = "sha256:1211a67cb575cd26d52862ac4b5405129659ecf63a0e668131502c59e912ce1d" +catalog_checksum = "sha256:b4f90b5873db3987b212783d2e0ed9cc6642e00446875e8b57749d5d67bbb506" [[file]] path = ".anvil/container/Dockerfile.dockerignore" @@ -17,7 +17,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" @@ -29,7 +29,7 @@ checksum = "sha256:ea44d5e1a2cb1471cf2cef05eceab846d8afa45cf691be800f21fec1218da [[file]] path = ".github/workflows/anvil-pr-impl.yml" -checksum = "sha256:4481b76085cda73c49d061ec493d467d50815667b81700e5724119170427aec8" +checksum = "sha256:70c77226f9efe7252047d9ea4be93a1ac328708e72f4665214cb0c13e06b5c80" [[file]] path = ".github/workflows/anvil-pr.yml" @@ -37,7 +37,7 @@ checksum = "sha256:0c2530d9a38e6a74e0a7fd4f999b4a1790f97de30b58b68c6c2344600da19 [[file]] path = ".github/workflows/anvil-scheduled-impl.yml" -checksum = "sha256:ac70061acf594c8c212c45ed97c3b653e7b8de68f4e1dcc9695a2628e4e2596d" +checksum = "sha256:ee1261dc018f6bdf3252890b57f0bc9510810c69505a64d3aef6d26315b824a3" [[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 ce3c159fe..e60a6f22c 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 9d696cc4a..d7738e376 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. `always()` + # keeps completed reports publishable after the gate or a later recipe + # fails. The separate hashFiles predicates require both feature + # configurations; one multi-pattern call would accept a partial pair. + 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 e4a25b4d0..fae9fd00e 100644 --- a/.github/workflows/anvil-scheduled-impl.yml +++ b/.github/workflows/anvil-scheduled-impl.yml @@ -65,11 +65,14 @@ jobs: free-disk-space: true - name: Upload coverage to Codecov # Upload from every leg except windows-11-arm (see the matching - # comment in pr-impl-workflow.yml for the rationale). + # comment in pr-impl-workflow.yml for the platform rationale). + # `always()` preserves completed reports after a later failure, while + # separate hashFiles predicates require both feature configurations; + # a multi-pattern call would accept either file by itself. # 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 8f4b8f56a..eabe382d6 100644 --- a/crates/cargo-anvil/docs/design/github.md +++ b/crates/cargo-anvil/docs/design/github.md @@ -569,6 +569,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 @@ -578,10 +579,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: @@ -628,9 +625,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: @@ -639,10 +637,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 @@ -838,7 +838,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. The check → bucket mapping is in @@ -990,8 +990,12 @@ If that returns `false`, or the release is missing, pin the commit SHA instead. ## 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 the all-features +(`lcov-all-features.info`) and no-default-features (`lcov-no-default.info`) +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 = ...)`) @@ -1004,10 +1008,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 @@ -1020,9 +1024,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 cc2145400..1515934f2 100644 --- a/crates/cargo-anvil/docs/implementation.md +++ b/crates/cargo-anvil/docs/implementation.md @@ -196,14 +196,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 on 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 6eaa33e45..b95437dc4 100644 --- a/crates/cargo-anvil/src/anvil/artifacts/github.rs +++ b/crates/cargo-anvil/src/anvil/artifacts/github.rs @@ -207,8 +207,28 @@ 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("uses: ./.github/actions/anvil-report-status")); + 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" + ); + let reporter = RUN_GROUP_ACTION + .split_once("- name: Publish supplemental Anvil commit status") + .expect("run-group action should contain the supplemental reporter step") + .1; + assert!( + reporter.contains("if: always()"), + "reporting must run after the authoritative recipe step fails" + ); + assert!(reporter.contains("uses: ./.github/actions/anvil-report-status")); + assert!( + reporter.contains("exit_code: ${{ steps.run.outputs.exit_code }}"), + "the reporter must consume the exit code recorded before failure propagation" + ); + assert!( + reporter.contains("failed_recipe: ${{ steps.run.outputs.failed_recipe }}"), + "the reporter must consume the recipe recorded before failure propagation" + ); // Impact reaches scoped checks through the downloaded impact cache // (read via `_anvil-impact-include`), not threaded --package env vars; // the executor only fixes the mode. @@ -303,7 +323,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")); @@ -315,10 +335,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")); } @@ -329,10 +346,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")); } @@ -342,10 +356,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")); } @@ -450,7 +461,14 @@ 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') != ''" + ), + "`always()` preserves completed reports after failure, and separate predicates require the complete pair" + ); assert!(PR_IMPL_WORKFLOW.contains("flags: ${{ matrix.os }}")); assert_eq!( PR_IMPL_WORKFLOW.matches("permissions:").count(), @@ -479,9 +497,17 @@ 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') != ''" + ), + "`always()` preserves completed reports after failure, and separate predicates require the complete pair" + ); 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")); + assert!(SCHEDULED_IMPL_WORKFLOW.contains("actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3")); assert!(SCHEDULED_IMPL_WORKFLOW.contains("github.rest.search.issuesAndPullRequests")); assert!(SCHEDULED_IMPL_WORKFLOW.contains("github.rest.issues.createComment")); assert!(SCHEDULED_IMPL_WORKFLOW.contains("github.rest.issues.create")); diff --git a/crates/cargo-anvil/templates/github/pr-impl-workflow.yml b/crates/cargo-anvil/templates/github/pr-impl-workflow.yml index 9d696cc4a..d7738e376 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. `always()` + # keeps completed reports publishable after the gate or a later recipe + # fails. The separate hashFiles predicates require both feature + # configurations; one multi-pattern call would accept a partial pair. + 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 ce3c159fe..e60a6f22c 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 982dc1ae6..fae9fd00e 100644 --- a/crates/cargo-anvil/templates/github/scheduled-impl-workflow.yml +++ b/crates/cargo-anvil/templates/github/scheduled-impl-workflow.yml @@ -65,11 +65,14 @@ jobs: free-disk-space: true - name: Upload coverage to Codecov # Upload from every leg except windows-11-arm (see the matching - # comment in pr-impl-workflow.yml for the rationale). + # comment in pr-impl-workflow.yml for the platform rationale). + # `always()` preserves completed reports after a later failure, while + # separate hashFiles predicates require both feature configurations; + # a multi-pattern call would accept either file by itself. # 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 @@ -155,7 +158,7 @@ jobs: issues: write steps: - name: Create or update failure issue - uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8.0.0 + uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 env: ANVIL_JOB_RESULTS: ${{ toJSON(needs) }} with: diff --git a/crates/cargo-anvil/tests/snapshots/snapshots__github_backend.snap b/crates/cargo-anvil/tests/snapshots/snapshots__github_backend.snap index 9b9836841..b9a3237a9 100644 --- a/crates/cargo-anvil/tests/snapshots/snapshots__github_backend.snap +++ b/crates/cargo-anvil/tests/snapshots/snapshots__github_backend.snap @@ -471,6 +471,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. @@ -484,11 +485,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. @@ -907,10 +903,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. `always()` + # keeps completed reports publishable after the gate or a later recipe + # fails. The separate hashFiles predicates require both feature + # configurations; one multi-pattern call would accept a partial pair. + 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 @@ -1136,11 +1133,14 @@ jobs: free-disk-space: true - name: Upload coverage to Codecov # Upload from every leg except windows-11-arm (see the matching - # comment in pr-impl-workflow.yml for the rationale). + # comment in pr-impl-workflow.yml for the platform rationale). + # `always()` preserves completed reports after a later failure, while + # separate hashFiles predicates require both feature configurations; + # a multi-pattern call would accept either file by itself. # 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 @@ -1226,7 +1226,7 @@ jobs: issues: write steps: - name: Create or update failure issue - uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8.0.0 + uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 env: ANVIL_JOB_RESULTS: ${{ toJSON(needs) }} with: diff --git a/crates/cargo-coverage-gate/README.md b/crates/cargo-coverage-gate/README.md index 293cba5e0..32f0981e2 100644 --- a/crates/cargo-coverage-gate/README.md +++ b/crates/cargo-coverage-gate/README.md @@ -20,7 +20,13 @@ 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 actionable details without relying on a later +coverage-service upload. A coverable line is a distinct LCOV `DA:` record. +Numeric failures show exact covered/coverable counts and uncovered ranges; +`expect-no-coverable-lines` failures show the unexpected coverable ranges; +and `NO DATA` explains that no records were attributed. Location lists are +bounded, with an exact count of omitted locations. ### Threshold resolution @@ -105,7 +111,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]: ggGmYW0CYXZlMC43LjNhdIQbFhzZ8rzWNNYbuRaDSGWynFgbH4PMdoT7GNcbVwNPtPjAhvFhYvRhcoQbSLvuLqYvzncbvkJE0ZCfihkbvLj44R1A234bYHxJKfzepgZhZIGDc2NhcmdvLWNvdmVyYWdlLWdhdGVlMC4zLjBzY2FyZ29fY292ZXJhZ2VfZ2F0ZQ [__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 7d6b1d140..d203789ec 100644 --- a/crates/cargo-coverage-gate/docs/design/README.md +++ b/crates/cargo-coverage-gate/docs/design/README.md @@ -255,6 +255,42 @@ 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**. +A **coverable line** is a distinct LCOV `DA:` record: a source line for which +LLVM emitted line-coverage instrumentation, whether or not a test executed it. +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. One hundred locations is large +enough to show several failure clusters while keeping a pathological package +to a few kilobytes of diagnostic output; it should be reevaluated if typical +failures routinely omit the first useful cluster or materially inflate CI +summaries. + +The [implementation guide](../implementation.md) records the parser, +attribution, status-selection, ordering, and rendering pipeline behind this +contract. + +```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 +331,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/docs/implementation.md b/crates/cargo-coverage-gate/docs/implementation.md new file mode 100644 index 000000000..5cd2b203c --- /dev/null +++ b/crates/cargo-coverage-gate/docs/implementation.md @@ -0,0 +1,46 @@ +# cargo-coverage-gate implementation + +This guide describes how the crate turns LCOV records into deterministic +failure diagnostics. The user-visible behavior is defined by the +[design](design/README.md). + +## Diagnostic pipeline + +1. The LCOV parser merges reports by source path and line number. A line is + coverable when it has a distinct `DA:` record. Hit counts determine the + covered subset, while sorted coverable and uncovered line numbers remain + available for diagnostics. +2. Attribution maps each source file to the most specific workspace member + whose manifest directory contains it. Aggregation computes exact package + counters from the attributed files. +3. Verdict evaluation selects diagnostic locations by status. Numeric failures + select uncovered lines, unexpected-coverable-lines failures select all + coverable lines, and passing or no-data outcomes select none. Paths are made + relative to the package manifest directory when possible, then diagnostics + are ordered by package, path, and line. +4. The terminal and Markdown renderers share the same detail and range + formatting. Each renderer emits at most 100 locations per package and + computes the omitted count from the complete diagnostic set, so truncation + does not alter aggregate counts or conceal how much output was omitted. + +## Invariants + +- Coverable line numbers are unique because the parser merges records by line + number, and ascending because `FileReport` construction explicitly sorts + them. +- Uncovered lines are a subset of coverable lines. +- Package counters describe the complete attributed input, independent of the + rendered location limit. +- Both renderers consume the same status-specific diagnostics and preserve + deterministic ordering. +- A no-data outcome is explanatory rather than location-bearing because there + are no attributed LCOV records to name. + +## Display bound + +The 100-location limit keeps a package's failure detail to a few kilobytes +while retaining enough context to show multiple clusters of missed code. The +limit is a presentation bound rather than a coverage-data bound: exact totals +and the omitted count still describe the full report. Reevaluate it when real +failure reports show that useful first clusters are routinely omitted or that +the resulting CI summaries are still too large. diff --git a/crates/cargo-coverage-gate/src/aggregate.rs b/crates/cargo-coverage-gate/src/aggregate.rs index 1a9419f38..e2b02acb8 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 37efc121d..181129318 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 8ef9f33a7..8fda7aca3 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,22 +124,36 @@ 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); } } + sort_line_numbers(&mut coverable_lines); + sort_line_numbers(&mut uncovered_lines); 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 } } } +fn sort_line_numbers(lines: &mut [u32]) { + lines.sort_unstable(); +} + #[cfg(test)] #[cfg_attr(coverage_nightly, coverage(off))] mod tests { @@ -161,6 +179,15 @@ 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] + fn line_numbers_are_sorted_explicitly() { + let mut lines = [8, 2, 5, 3]; + sort_line_numbers(&mut lines); + assert_eq!(lines, [2, 3, 5, 8]); } #[test] @@ -232,6 +259,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 3325e701a..e5fff1ef1 100644 --- a/crates/cargo-coverage-gate/src/lib.rs +++ b/crates/cargo-coverage-gate/src/lib.rs @@ -11,7 +11,13 @@ //! `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 actionable details without relying on a later +//! coverage-service upload. A coverable line is a distinct LCOV `DA:` record. +//! Numeric failures show exact covered/coverable counts and uncovered ranges; +//! `expect-no-coverable-lines` failures show the unexpected coverable ranges; +//! and `NO DATA` explains that no records were attributed. Location lists are +//! bounded, with an exact count of omitted locations. //! //! ## Threshold resolution //! diff --git a/crates/cargo-coverage-gate/src/render/diagnostics.rs b/crates/cargo-coverage-gate/src/render/diagnostics.rs new file mode 100644 index 000000000..26de17375 --- /dev/null +++ b/crates/cargo-coverage-gate/src/render/diagnostics.rs @@ -0,0 +1,70 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +//! Shared failure-detail formatting for terminal and Markdown output. + +use crate::verdict::{PackageOutcome, Status}; + +/// Maximum source locations shown for one package. +pub(crate) const MAX_DIAGNOSTIC_LINES: usize = 100; + +pub(crate) 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 {}.", + super::plural(outcome.totals.count as usize, "line", "lines") + )), + Status::Ok | Status::NoCoverableLines => None, + } +} + +pub(crate) fn diagnostic_line_count(outcome: &PackageOutcome) -> usize { + outcome.diagnostics.iter().map(|diagnostic| diagnostic.lines.len()).sum() +} + +pub(crate) 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 { + use super::*; + + #[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"); + } +} diff --git a/crates/cargo-coverage-gate/src/render/markdown.rs b/crates/cargo-coverage-gate/src/render/markdown.rs index 824131f30..46b1b706d 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 00bfdd5d7..91bd04dfb 100644 --- a/crates/cargo-coverage-gate/src/render/mod.rs +++ b/crates/cargo-coverage-gate/src/render/mod.rs @@ -14,9 +14,12 @@ //! //! [`Report`]: crate::verdict::Report +mod diagnostics; pub(crate) mod markdown; pub(crate) mod text; +pub(crate) use diagnostics::{MAX_DIAGNOSTIC_LINES, diagnostic_line_count, failure_detail, format_line_ranges}; + use crate::threshold::ThresholdSource; use crate::verdict::{PackageOutcome, Status}; @@ -171,6 +174,7 @@ mod tests { }, totals: LineTotals { count, covered }, status: Status::Ok, + diagnostics: Vec::new(), } } @@ -183,6 +187,7 @@ mod tests { }, totals: LineTotals { count, covered }, status, + diagnostics: Vec::new(), } } diff --git a/crates/cargo-coverage-gate/src/render/text.rs b/crates/cargo-coverage-gate/src/render/text.rs index 7c8af320f..ffa7bee77 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 ff23d9e5e..8c7e55468 100644 --- a/crates/cargo-coverage-gate/src/verdict.rs +++ b/crates/cargo-coverage-gate/src/verdict.rs @@ -14,12 +14,13 @@ //! [`threshold`]: crate::threshold use std::collections::HashSet; +use std::path::PathBuf; use crate::Verdict; use crate::aggregate::{LineTotals, aggregate}; use crate::attribute::{AttributionOutcome, attribute}; use crate::error::{CoverageGateError, UnknownPackageSelectorError}; -use crate::lcov_cov::CoverageReport; +use crate::lcov_cov::{CoverageReport, FileReport}; use crate::threshold::{Threshold, ThresholdSource}; use crate::workspace::{Member, Workspace}; @@ -55,6 +56,8 @@ 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, } impl PackageOutcome { @@ -65,6 +68,16 @@ impl PackageOutcome { } } +/// 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, +} + /// Full verdict report — one row per gated package. #[derive(Debug, Clone)] pub(crate) struct Report { @@ -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: &[&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,16 +319,15 @@ 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; fn make_file(path: &str, count: u32, covered: u32) -> FileReport { FileReport { filename: PathBuf::from(path), lines_total: count, lines_covered: covered, + coverable_lines: (1..=count).collect(), + uncovered_lines: ((covered + 1)..=count).collect(), } } @@ -356,6 +396,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 1800cb264..0e0ad3794 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]