From 2895a18ca92f415ae62f87167ba9c7a672e3b7dc Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Wed, 14 Oct 2026 02:30:44 +0800 Subject: [PATCH] fix(ci): skip portable report merge when shards are skipped (#2943) * fix(ci): skip portable report merge when shards are skipped The portable consumer can still run after an upstream build failure, even though the test-shard matrix is skipped and no Vitest blob artifacts exist. Guard artifact download and both report merge variants on successful shards; retain the required-shard gate so skipped or failed shards remain failures. This applies the report-consumer condition from ee0256c740c9be18baae6c56d90285df8069f5b4 to current pinned main without changing action pins or required gates. * fix(ci): preserve failed shard diagnostics * docs: clarify current revision CI policy --- .github/workflows/ci.yml | 14 +++++++++++--- CONTRIBUTING.md | 3 +-- tests/boundary/filesystem/ciTestReports.test.ts | 17 ++++++++++++++--- 3 files changed, 26 insertions(+), 8 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 69c757b3b..8b47fcf89 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -290,14 +290,19 @@ jobs: - if: needs.changes.outputs.code == 'true' && success() run: npm ci - uses: actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093 # v4.3.0 - if: needs.changes.outputs.code == 'true' && success() + if: >- + !cancelled() && needs.changes.outputs.code == 'true' && + (needs.test-shard.result == 'success' || needs.test-shard.result == 'failure') with: path: .vitest/blob pattern: vitest-report-* merge-multiple: true - name: Merge coverage, JUnit, and timing reports id: merge - if: needs.changes.outputs.code == 'true' && needs.changes.outputs.tier == 'full' && success() + if: >- + !cancelled() && needs.changes.outputs.code == 'true' && + needs.changes.outputs.tier == 'full' && + (needs.test-shard.result == 'success' || needs.test-shard.result == 'failure') run: >- npm run test:ci:merge -- --coverage.reportsDirectory=coverage @@ -312,7 +317,10 @@ jobs: --outputFile.json=test-results.json - name: Merge fast-suite JUnit and timing reports id: merge-fast - if: needs.changes.outputs.code == 'true' && needs.changes.outputs.tier == 'fast' && success() + if: >- + !cancelled() && needs.changes.outputs.code == 'true' && + needs.changes.outputs.tier == 'fast' && + (needs.test-shard.result == 'success' || needs.test-shard.result == 'failure') run: >- npm exec -- vitest --merge-reports --reporter=default diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 96d8010ee..e80d903a3 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -182,8 +182,7 @@ The separate GitHub Pages host uses only the manual ## Development feedback and PR verification For routine merges, wait for the required checks to pass for the current PR -revision against the current `main`. Branch protection requires up-to-date -branches; administrators retain an override for deliberate maintainer recovery. +revision. Administrators retain an override for deliberate maintainer recovery. If `main` is broken, prioritize a focused repair before updating affected PRs. Select checks for the behavior changed. For a source edit: diff --git a/tests/boundary/filesystem/ciTestReports.test.ts b/tests/boundary/filesystem/ciTestReports.test.ts index 06212ad07..3e2aadcd4 100644 --- a/tests/boundary/filesystem/ciTestReports.test.ts +++ b/tests/boundary/filesystem/ciTestReports.test.ts @@ -45,23 +45,34 @@ const workflow = async () => ), ).jobs; -it("collects failed-shard diagnostics before enforcing the shard result", async () => { +it("merges available failed-shard diagnostics and skips unavailable shard reports", async () => { const steps = (await workflow()).test!.steps; + const download = steps.find( + (step) => + step.uses?.includes("actions/download-artifact") && + step.with?.pattern === "vitest-report-*", + ); const merge = steps.findIndex((step) => step.run?.includes("test:ci:merge")); const upload = steps.findIndex((step) => step.with?.name === "test-results"); const gate = steps.findIndex( (step) => step.env?.SHARD_RESULT && step.run?.includes("test "), ); expect(merge).toBeGreaterThan(0); + expect(download?.if).toContain("!cancelled()"); + expect(download?.if).toContain("needs.test-shard.result == 'success'"); + expect(download?.if).toContain("needs.test-shard.result == 'failure'"); expect(upload).toBeGreaterThan(merge); expect(gate).toBeGreaterThan(upload); - expect(steps[merge]?.if).not.toContain("needs.test-shard.result"); const merges = steps.filter((step) => step.run?.includes("--reporter=json")); expect(merges).toHaveLength(2); - for (const step of merges) + for (const step of merges) { expect(step.run).toContain( "--reporter=./scripts/ci/test-timing-reporter.mjs", ); + expect(step.if).toContain("!cancelled()"); + expect(step.if).toContain("needs.test-shard.result == 'success'"); + expect(step.if).toContain("needs.test-shard.result == 'failure'"); + } expect(steps[upload]?.with?.path).toContain("test-timings.json"); expect(steps[upload]?.with).toMatchObject({ "include-hidden-files": true }); expect(steps[upload]?.with?.path).toContain(