From c832e9767ac5ae857f650571bf35c391023f1d06 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Thu, 23 Jul 2026 01:14:23 -0700 Subject: [PATCH] fix(ci): carry the no_tests_matched escape through every coverage/results upload step #8167 taught the verify step that a scoped selection matching zero test files writes no report and is not a failure -- but left every upload step below it unguarded, so the trusted/fork coverage uploads (fail_ci_if_error: true) would still turn that non-event red on the missing lcov, and the junit uploads would noisily fail on a missing xml. It also left codecov-policy.test.ts pinning the old verify condition, which broke that suite on main for every branch. Adds the same steps.coverage.outputs.no_tests_matched guard to all five upload steps and re-pins the policy test to the completed invariant: the escape hatch exists exactly once, and every upload below the verify step carries it. --- .github/workflows/ci.yml | 12 +++++++----- test/unit/codecov-policy.test.ts | 16 +++++++++++----- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0784ab5c69..de3e94d0d5 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1088,7 +1088,7 @@ jobs: # Consumed by validate-tests-merge to re-check the global coverage threshold against all shards # combined -- see this job's own header comment. - name: Upload coverage blob report - if: ${{ success() }} + if: ${{ success() && steps.coverage.outputs.no_tests_matched != 'true' }} uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 with: name: coverage-blob-shard-${{ matrix.shard }} @@ -1097,7 +1097,9 @@ jobs: # Direct upload for trusted contexts (push + same-repo PRs). Multiple shards' uploads for the same # commit/PR are additive in Codecov -- see this job's own header comment. - name: Upload coverage to Codecov - if: ${{ success() && github.event.pull_request.head.repo.fork != true }} + # Same no_tests_matched skip as the verify step above (#8167 follow-up): with zero matched tests + # there is no lcov to upload, and fail_ci_if_error would otherwise turn that non-event red. + if: ${{ success() && steps.coverage.outputs.no_tests_matched != 'true' && github.event.pull_request.head.repo.fork != true }} uses: codecov/codecov-action@fb8b3582c8e4def4969c97caa2f19720cb33a72f # v7.0.0 with: token: ${{ secrets.CODECOV_TOKEN }} @@ -1115,7 +1117,7 @@ jobs: # override, etc). Condition dropped the push-or-backend check since this job's own `if:` already # covers it. - name: Upload coverage to Codecov (fork PR tokenless) - if: ${{ success() && github.event.pull_request.head.repo.fork == true }} + if: ${{ success() && steps.coverage.outputs.no_tests_matched != 'true' && github.event.pull_request.head.repo.fork == true }} uses: codecov/codecov-action@fb8b3582c8e4def4969c97caa2f19720cb33a72f # v7.0.0 with: files: ./coverage/lcov.info @@ -1129,7 +1131,7 @@ jobs: # their upload non-blocking so a JUnit ingestion hiccup does not fail CI # after the tests and hard coverage upload have already passed. - name: Upload Vitest results to Codecov - if: ${{ !cancelled() && github.event.pull_request.head.repo.fork != true }} + if: ${{ !cancelled() && steps.coverage.outputs.no_tests_matched != 'true' && github.event.pull_request.head.repo.fork != true }} uses: codecov/codecov-action@fb8b3582c8e4def4969c97caa2f19720cb33a72f # v7.0.0 with: token: ${{ secrets.CODECOV_TOKEN }} @@ -1142,7 +1144,7 @@ jobs: override_pr: ${{ github.event_name == 'pull_request' && github.event.pull_request.number || '' }} fail_ci_if_error: false - name: Upload Vitest results to Codecov (fork PR tokenless) - if: ${{ !cancelled() && github.event.pull_request.head.repo.fork == true }} + if: ${{ !cancelled() && steps.coverage.outputs.no_tests_matched != 'true' && github.event.pull_request.head.repo.fork == true }} uses: codecov/codecov-action@fb8b3582c8e4def4969c97caa2f19720cb33a72f # v7.0.0 with: files: ./reports/junit/vitest.xml diff --git a/test/unit/codecov-policy.test.ts b/test/unit/codecov-policy.test.ts index 58108cd4a1..88d715cb50 100644 --- a/test/unit/codecov-policy.test.ts +++ b/test/unit/codecov-policy.test.ts @@ -56,11 +56,17 @@ describe("Codecov policy", () => { const testResultsUpload = steps[testResultsUploadIndex]!; // Verify must run whenever coverage was generated at all -- the job's own top-level `if:` (push or - // backend==true) already gates the whole matrix, so the step itself only needs `success()`. It - // deliberately does NOT exclude forks, since both the trusted and the tokenless fork upload path - // below it need the report to exist first. - expect(String(verifyStep.if)).toBe("${{ success() }}"); - expect(String(coverageUpload.if)).toContain(String(verifyStep.if).replace(/^\$\{\{\s*|\s*\}\}$/g, "")); + // backend==true) already gates the whole matrix. #8167 added the ONE legitimate escape: a scoped + // selection that matched zero test files writes no report at all (no_tests_matched), so verify — and + // every upload below it — must skip on that output rather than fail closed on a missing lcov. The + // policy stays: absent that explicit escape hatch, a missing report still fails the build, and the + // uploads can never run without the verify guard's own condition. + expect(String(verifyStep.if)).toBe("${{ success() && steps.coverage.outputs.no_tests_matched != 'true' }}"); + // EVERY upload below the verify step must carry the same escape: with zero matched tests there is no + // lcov/junit at all, and the coverage uploads' fail_ci_if_error would turn that non-event red. + expect(String(coverageUpload.if)).toContain("success()"); + expect(String(coverageUpload.if)).toContain("no_tests_matched != 'true'"); + expect(String(testResultsUpload.if)).toContain("no_tests_matched != 'true'"); expect(String(verifyStep.run)).toContain("coverage/lcov.info is missing or empty"); expect(String(verifyStep.run)).toContain("exit 1");