diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index 3c90e10c0..b3419e574 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -267,9 +267,9 @@ jobs: strategy: fail-fast: false matrix: - # internal/api is by far the slowest package under -race (~10m), so it - # gets a dedicated shard; every other package splits across the rest. - shard: [api, rest-0, rest-1] + # internal/api has its own sharded job below; every other package + # splits across these two. + shard: [rest-0, rest-1] steps: - name: Report documentation-only change @@ -304,15 +304,11 @@ jobs: if: needs.changes.outputs.code == 'true' env: PULSE_DATA_DIR: /tmp/pulse-test-data - # internal/api alone takes ~22m under -race on a fast machine since - # the demo estate scaled to 50 nodes, and CI runners have needed - # roughly double the fast-machine time, hence the large per-binary - # budget. Remaining packages are split deterministically by list - # position. + # Packages other than internal/api are split deterministically by + # list position. run: | set -euo pipefail case "${{ matrix.shard }}" in - api) pkgs="./internal/api" ;; rest-0) pkgs=$(go list ./... | grep -v '/internal/api$' | awk 'NR % 2 == 0') ;; rest-1) pkgs=$(go list ./... | grep -v '/internal/api$' | awk 'NR % 2 == 1') ;; *) echo "unknown shard ${{ matrix.shard }}" >&2; exit 1 ;; @@ -324,6 +320,113 @@ jobs: printf 'Packages in this shard:\n%s\n' "$pkgs" go test -race -timeout 50m $pkgs + # internal/api alone took ~27 of the ~31 minutes of the old single + # `Backend tests (api)` job under -race, which made it the critical path of + # every pull request. Its top-level tests are split across these shards + # as contiguous runs of the package's own test order, and the + # `Backend tests (api)` job below keeps the required check name by passing + # only when every shard passed. + backend-api: + name: Backend tests (api-${{ matrix.index }}) + needs: changes + # Like the rest shards, expand even for documentation-only changes so the + # verdict job sees real shard results rather than skipped jobs. + runs-on: ubuntu-24.04 + timeout-minutes: 45 + strategy: + fail-fast: false + matrix: + # Keep API_SHARD_COUNT equal to the number of indexes listed here. + index: [0, 1, 2, 3] + env: + API_SHARD_COUNT: 4 + + steps: + - name: Report documentation-only change + if: needs.changes.outputs.code != 'true' + run: echo "No backend code changed. Tests are not required for this shard." + + - name: Checkout repository + if: needs.changes.outputs.code == 'true' + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + + - name: Set up Go + if: needs.changes.outputs.code == 'true' + uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0 + with: + go-version-file: go.mod + cache: true + + - name: Provide frontend embed stub + if: needs.changes.outputs.code == 'true' + run: | + mkdir -p internal/api/frontend-modern/dist + [ -f internal/api/frontend-modern/dist/index.html ] || \ + printf 'ci embed stub\n' > internal/api/frontend-modern/dist/index.html + + - name: Go unit tests (internal/api shard ${{ matrix.index }}) + if: needs.changes.outputs.code == 'true' + env: + PULSE_DATA_DIR: /tmp/pulse-test-data + API_SHARD_INDEX: ${{ matrix.index }} + # The test list is computed from the checked-out commit at run time, + # so every shard sees the same list and a test added later lands in + # exactly one shard with no workflow edit. The list keeps go test's + # own order (file name, then declaration), which is also run order, + # and each shard takes one contiguous slice of it. Some internal/api + # tests still lean on package state left by the tests just before + # them (for example admin bypass or session store globals), and an + # interleaved split by name broke dozens of them, while contiguous + # slices keep those neighbours together. Listing with -race builds + # the same test binary the run then reuses from cache. Benchmarks + # are left out because plain go test never runs them. + run: | + set -euo pipefail + listing=$(go test -race -list . ./internal/api) + tests=$(printf '%s\n' "$listing" | grep -E '^(Test|Fuzz|Example)[^[:space:]]*$' || true) + if [ -z "$tests" ]; then + echo "::error::go test -list found no tests in ./internal/api" + exit 1 + fi + total=$(printf '%s\n' "$tests" | wc -l | tr -d ' ') + selected=$(printf '%s\n' "$tests" | awk -v n="$API_SHARD_COUNT" -v i="$API_SHARD_INDEX" -v total="$total" \ + 'NR > int(total * i / n) && NR <= int(total * (i + 1) / n)') + if [ -z "$selected" ]; then + echo "::error::internal/api shard ${API_SHARD_INDEX} of ${API_SHARD_COUNT} resolved to an empty test list" + exit 1 + fi + pattern="^($(printf '%s\n' "$selected" | paste -sd '|' -))\$" + # Linux rejects a single argument over 128 KiB. Fail with a clear + # instruction instead of an opaque exec error. + if [ "${#pattern}" -gt 120000 ]; then + echo "::error::internal/api shard pattern is ${#pattern} bytes; add a shard index and raise API_SHARD_COUNT" + exit 1 + fi + echo "internal/api shard ${API_SHARD_INDEX} of ${API_SHARD_COUNT} runs $(printf '%s\n' "$selected" | wc -l | tr -d ' ') of ${total} tests" + go test -race -timeout 50m -run "$pattern" ./internal/api + + # Required check name. Branch protection matches `Backend tests (api)` + # exactly, so this job must keep that name and must fail unless every + # internal/api shard succeeded (a skipped or cancelled shard fails it too). + backend-api-verdict: + name: Backend tests (api) + needs: backend-api + if: always() + runs-on: ubuntu-24.04 + timeout-minutes: 5 + steps: + - name: Require every internal/api shard + env: + API_SHARDS_RESULT: ${{ needs.backend-api.result }} + run: | + if [ "$API_SHARDS_RESULT" != success ]; then + echo "::error::internal/api shards did not all pass (result: ${API_SHARDS_RESULT})" + exit 1 + fi + echo "Every internal/api shard passed." + scripts-and-build: name: Script smoke tests & backend build needs: changes diff --git a/.github/workflows/test-e2e.yml b/.github/workflows/test-e2e.yml index 0c2945067..ec492a715 100644 --- a/.github/workflows/test-e2e.yml +++ b/.github/workflows/test-e2e.yml @@ -203,9 +203,12 @@ jobs: # Runs even when the stable tier failed (its data still counts toward # promotion), but not when the stable tier was skipped — that means the # environment never came up and every probation spec would fail on it. + # Pull requests skip it. It cannot gate a PR, and the promotion rule in + # e2e-tiering.mjs counts only main runs, so on a PR it only held each + # of the eight shard runners for about eight more minutes. - name: Run probation-tier E2E suite (non-gating) id: probation - if: ${{ !cancelled() && steps.stable.conclusion != 'skipped' }} + if: ${{ !cancelled() && github.event_name != 'pull_request' && steps.stable.conclusion != 'skipped' }} continue-on-error: true # A broken probation surface can otherwise consume the entire 60m job # budget through retries and cancel the job before continue-on-error diff --git a/docs/release-control/v6/internal/subsystems/deployment-installability.md b/docs/release-control/v6/internal/subsystems/deployment-installability.md index 71d1d4f4c..52aa9a4b1 100644 --- a/docs/release-control/v6/internal/subsystems/deployment-installability.md +++ b/docs/release-control/v6/internal/subsystems/deployment-installability.md @@ -24,6 +24,29 @@ Concurrency groups continue to isolate workflows and refs. Cancellation of an obsolete PR run supplies no passing evidence for its replacement, which still needs its own checks. Release publication workflows are unaffected. +### Sharded internal/api backend tests + +Build and Test runs the `internal/api` race tests as four `Backend tests +(api-N)` shards instead of one job, because that one package took about 27 of +the 31 minutes of the old `Backend tests (api)` job and set the critical path +of every pull request. Each shard lists the package's tests with `go test +-race -list` from the commit under test and runs one contiguous quarter of +that list in go test's own order, so the shards cover every test exactly once, +including tests added later, and a shard that resolves no tests fails. The +order is kept on purpose. Some `internal/api` tests depend on package state +left by the tests just before them, and an interleaved split by name broke +dozens of them. The required check name +`Backend tests (api)` belongs to a verdict job that passes only when every +shard succeeded, and `Backend tests (rest-0)` and `Backend tests (rest-1)` keep +their names and package split. All shards still expand for documentation-only +changes so no required check is left pending or reads skipped shards as a +pass. Go test steps are not skipped for frontend-only changes, because Go tests +read frontend sources (the `internal/api` contract tests, the +`internal/unifiedresources` code standards walk of `frontend-modern/src`, and +the `internal/telemetry` repository-wide wording scan). Core E2E runs its +non-gating probation tier only outside pull requests, since promotion counts +only main runs. + ### Docker SDK dependency compatibility The Docker consumers use Moby API v1.56.0 and client v0.6.0 together, without diff --git a/scripts/installtests/build_release_assets_test.go b/scripts/installtests/build_release_assets_test.go index a64578e47..17d3165fe 100644 --- a/scripts/installtests/build_release_assets_test.go +++ b/scripts/installtests/build_release_assets_test.go @@ -3797,6 +3797,84 @@ func TestBenchmarkQualificationRetainsProvenance(t *testing.T) { } } +func TestBackendAPIShardsKeepRequiredCheckExhaustive(t *testing.T) { + content, err := os.ReadFile(repoFile(".github", "workflows", "build-and-test.yml")) + if err != nil { + t.Fatal(err) + } + workflow := string(content) + + // Branch protection requires these exact check names. The rest shards stay + // matrix entries and internal/api keeps its name on the verdict job. + backend := workflowJobBlock(t, workflow, "backend") + for _, required := range []string{ + "name: Backend tests (${{ matrix.shard }})", + "shard: [rest-0, rest-1]", + "grep -v '/internal/api$'", + "go test -race -timeout 50m $pkgs", + } { + if !strings.Contains(backend, required) { + t.Fatalf("backend rest shards missing %q", required) + } + } + + shards := workflowJobBlock(t, workflow, "backend-api") + indexes := regexp.MustCompile(`(?m)^ index: \[([0-9, ]+)\]$`).FindStringSubmatch(shards) + count := regexp.MustCompile(`(?m)^ API_SHARD_COUNT: ([0-9]+)$`).FindStringSubmatch(shards) + if len(indexes) != 2 || len(count) != 2 { + t.Fatal("internal/api shard job must declare its index matrix and API_SHARD_COUNT") + } + want, _ := strconv.Atoi(count[1]) + listed := strings.Split(indexes[1], ", ") + if want < 2 || len(listed) != want { + t.Fatalf("internal/api shard matrix %v must list exactly API_SHARD_COUNT=%d indexes", listed, want) + } + for i, value := range listed { + if value != strconv.Itoa(i) { + t.Fatalf("internal/api shard indexes must be 0..%d in order, got %v", want-1, listed) + } + } + for _, required := range []string{ + "needs: changes", + "fail-fast: false", + // The list comes from the commit under test, so a new test cannot be + // missed, and contiguous slices of go test's own order put it in + // exactly one shard while keeping order-coupled neighbours together. + "go test -race -list . ./internal/api", + `'NR > int(total * i / n) && NR <= int(total * (i + 1) / n)'`, + "resolved to an empty test list", + "go test -list found no tests in ./internal/api", + `go test -race -timeout 50m -run "$pattern" ./internal/api`, + "PULSE_DATA_DIR: /tmp/pulse-test-data", + } { + if !strings.Contains(shards, required) { + t.Fatalf("internal/api shard job missing %q", required) + } + } + if strings.Contains(shards, "sort") { + t.Fatal("internal/api shards must keep go test's run order; sorting splits order-coupled tests") + } + if strings.Contains(shards, "\n if:") { + t.Fatal("internal/api shards must expand for every change so the verdict never sees skipped shards") + } + + verdict := workflowJobBlock(t, workflow, "backend-api-verdict") + for _, required := range []string{ + "name: Backend tests (api)\n", + "needs: backend-api", + "if: always()", + "API_SHARDS_RESULT: ${{ needs.backend-api.result }}", + `if [ "$API_SHARDS_RESULT" != success ]; then`, + } { + if !strings.Contains(verdict, required) { + t.Fatalf("Backend tests (api) verdict missing %q", required) + } + } + if got := strings.Count(workflow, "name: Backend tests (api)\n"); got != 1 { + t.Fatalf("exactly one job may carry the required Backend tests (api) name, found %d", got) + } +} + func TestFrontendDependencySecurityAuditsAreRequired(t *testing.T) { workflowPath := repoFile(".github", "workflows", "build-and-test.yml") assertFileContainsAll(t, workflowPath, diff --git a/scripts/tests/test_e2e_workflow_contract.py b/scripts/tests/test_e2e_workflow_contract.py index e3fa1cc49..f76ac973e 100644 --- a/scripts/tests/test_e2e_workflow_contract.py +++ b/scripts/tests/test_e2e_workflow_contract.py @@ -21,6 +21,8 @@ class E2EWorkflowContractTest(unittest.TestCase): probation_step = workflow[probation_start:report_start] self.assertIn("continue-on-error: true", probation_step) + # Non-gating and promotion counts only main runs, so PRs skip it. + self.assertIn("github.event_name != 'pull_request'", probation_step) self.assertIn("timeout-minutes: 12", probation_step) self.assertIn("--max-failures=5", probation_step) self.assertIn("--global-timeout=600000", probation_step)