diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index c9b174e9dc3..b04a8a86408 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -391,9 +391,22 @@ jobs: verify: # Preserve the legacy required-check name while the underlying work runs in parallel. + # BLO-20869: verify_serialized_server is folded in here (rather than made + # required on its own) so a cancelled/skipped/never-scheduled serialized + # shard fails this required check instead of being invisible to the merge + # gate. See the issue for the measured latency tradeoff of putting a + # ~90-minute job on the required path this way. name: verify if: ${{ always() }} - needs: [helm_chart, typecheck_release_registry, general_tests, worktree_install, build] + needs: + [ + helm_chart, + typecheck_release_registry, + general_tests, + worktree_install, + build, + verify_serialized_server, + ] runs-on: arc-light timeout-minutes: 5 @@ -425,6 +438,7 @@ jobs: GENERAL_TESTS_RESULT: ${{ needs.general_tests.result }} WORKTREE_INSTALL_RESULT: ${{ needs.worktree_install.result }} BUILD_RESULT: ${{ needs.build.result }} + VERIFY_SERIALIZED_SERVER_RESULT: ${{ needs.verify_serialized_server.result }} run: | declare -A lane_results=( [helm_chart]="$HELM_CHART_RESULT" @@ -432,6 +446,7 @@ jobs: [general_tests]="$GENERAL_TESTS_RESULT" [worktree_install]="$WORKTREE_INSTALL_RESULT" [build]="$BUILD_RESULT" + [verify_serialized_server]="$VERIFY_SERIALIZED_SERVER_RESULT" ) cancelled_lanes=() diff --git a/scripts/__tests__/pr-verify-lane-outcome.test.mjs b/scripts/__tests__/pr-verify-lane-outcome.test.mjs index 1cfd5eef58e..5581229fdc8 100644 --- a/scripts/__tests__/pr-verify-lane-outcome.test.mjs +++ b/scripts/__tests__/pr-verify-lane-outcome.test.mjs @@ -54,6 +54,7 @@ function runVerifyStep(results) { GENERAL_TESTS_RESULT: results.general_tests ?? "success", WORKTREE_INSTALL_RESULT: results.worktree_install ?? "success", BUILD_RESULT: results.build ?? "success", + VERIFY_SERIALIZED_SERVER_RESULT: results.verify_serialized_server ?? "success", }; return spawnSync("bash", ["-c", script], { env, encoding: "utf8" }); } @@ -104,6 +105,31 @@ test("verify step annotates both a real failure and a cancellation when a run ha assert.match(result.stdout, /general_tests/); }); +// BLO-20869: verify_serialized_server must be treated exactly like the other +// required lanes -- cancelled, skipped, or never-scheduled (which also reads +// as "skipped" via `needs`) must all fail this required check instead of +// being invisible to the merge gate. +test("verify step fails when the serialized server suite is cancelled", () => { + const result = runVerifyStep({ verify_serialized_server: "cancelled" }); + assert.notEqual(result.status, 0); + assert.match(result.stdout, /::error title=verify: lane cancelled::/); + assert.match(result.stdout, /verify_serialized_server/); +}); + +test("verify step fails when the serialized server suite is skipped", () => { + const result = runVerifyStep({ verify_serialized_server: "skipped" }); + assert.notEqual(result.status, 0); + assert.match(result.stdout, /::error title=verify: lane skipped::/); + assert.match(result.stdout, /verify_serialized_server/); +}); + +test("verify step fails when the serialized server suite genuinely fails", () => { + const result = runVerifyStep({ verify_serialized_server: "failure" }); + assert.notEqual(result.status, 0); + assert.match(result.stdout, /::error title=verify: lane failure::/); + assert.match(result.stdout, /verify_serialized_server/); +}); + test("verify step annotates a skipped lane as an unmet dependency, not a failure", () => { const result = runVerifyStep({ worktree_install: "skipped" }); assert.notEqual(result.status, 0);