diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 804a04346..19e07b031 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -46,23 +46,59 @@ concurrency: jobs: test: - name: ${{ matrix.os }} + name: ${{ matrix.name }} runs-on: ${{ matrix.os }} - # Windows dominates this matrix: on run 30459554635 the same suite took - # ubuntu 4.6min / macos 5.6min / windows 11.8min. Against the previous - # 12-minute ceiling that left ~12s of headroom, so runner variance decided - # the result rather than the code under review — #711's rerun finished at - # 11.8min and passed while #653's was killed at 12.0min (issue #717). - # A cancelled job renders as `fail` in `gh pr checks`, so that flakiness - # reads as a broken PR. 20 minutes keeps a green Windows run green with - # real margin; it is not a licence for the suite to grow into it. If - # Windows approaches this ceiling too, fix the 2.5x platform gap instead - # of raising the number again. + # Keep the 20-minute ceiling as a real performance boundary. The combined provider-security + # and admission suites pushed the serial macOS/Windows jobs into this ceiling even though the + # root tests themselves remained green. Slow platforms therefore separate root tests from GUI + # quality work, and Windows root tests are deterministically balanced over two shards. timeout-minutes: 20 strategy: fail-fast: false matrix: - os: [ubuntu-latest, windows-latest, macos-latest] + include: + - name: ubuntu-latest + os: ubuntu-latest + run_tests: true + run_quality: true + run_typecheck: true + shard_index: 0 + shard_count: 1 + - name: macos-latest + os: macos-latest + run_tests: true + run_quality: false + run_typecheck: true + shard_index: 0 + shard_count: 1 + - name: macos-quality + os: macos-latest + run_tests: false + run_quality: true + run_typecheck: false + shard_index: 0 + shard_count: 1 + - name: windows-latest + os: windows-latest + run_tests: true + run_quality: false + run_typecheck: true + shard_index: 0 + shard_count: 2 + - name: windows-latest shard 2/2 + os: windows-latest + run_tests: true + run_quality: false + run_typecheck: false + shard_index: 1 + shard_count: 2 + - name: windows-quality + os: windows-latest + run_tests: false + run_quality: true + run_typecheck: false + shard_index: 0 + shard_count: 1 steps: - name: Checkout uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7 @@ -72,32 +108,47 @@ jobs: with: bun-version: 1.3.14 - - name: Install dependencies - run: | - bun install --frozen-lockfile - cd gui - bun install --frozen-lockfile + - name: Install root dependencies + run: bun install --frozen-lockfile + + - name: Install GUI dependencies + # Root tests need gui/node_modules too: several tests/ suites import GUI components + # (e.g. provider-workspace rail, quota bars), which resolve react from gui/. + if: ${{ matrix.run_quality || matrix.run_tests }} + run: cd gui && bun install --frozen-lockfile - name: Typecheck + if: ${{ matrix.run_typecheck }} run: bun x tsc --noEmit - name: Test + if: ${{ matrix.run_tests }} shell: bash env: MATRIX_OS: ${{ matrix.os }} + TEST_SHARD_INDEX: ${{ matrix.shard_index }} + TEST_SHARD_COUNT: ${{ matrix.shard_count }} run: | set -euo pipefail + run_suite() { + if [[ "$TEST_SHARD_COUNT" == "1" ]]; then + bun run scripts/test.ts + else + bun run scripts/ci-test-shard.ts "$TEST_SHARD_INDEX" "$TEST_SHARD_COUNT" + fi + } + if [[ "$MATRIX_OS" != "windows-latest" ]]; then - bun test --isolate tests + run_suite exit 0 fi - # Bun 1.3.14 on Windows intermittently panics under Worker - # spawn/terminate churn in storage policy tests ("Internal assertion - # failure" / "Bun has crashed"). Retry once on that runtime crash - # only — ordinary assertion failures still fail the job. + + # Bun 1.3.14 on Windows intermittently panics under Worker spawn/terminate churn in + # storage policy tests ("Internal assertion failure" / "Bun has crashed"). Retry the + # current shard once on that runtime crash only; ordinary assertion failures still fail. out="$(mktemp)" set +e - bun test --isolate tests >"$out" 2>&1 + run_suite >"$out" 2>&1 code=$? set -e cat "$out" @@ -105,32 +156,38 @@ jobs: exit 0 fi if grep -q 'Bun has crashed' "$out"; then - echo "::warning::Bun runtime crash on Windows; retrying suite once" - bun test --isolate tests + echo "::warning::Bun runtime crash on Windows shard; retrying once" + run_suite exit $? fi exit "$code" - name: GUI tests + if: ${{ matrix.run_quality }} run: cd gui && bun test tests - name: Privacy scan + if: ${{ matrix.run_quality }} run: bun run privacy:scan - name: Check release helper syntax + if: ${{ matrix.run_quality }} run: bun build scripts/release.ts --target=bun --outdir=.tmp/ci-release-script-check - name: GUI lint + if: ${{ matrix.run_quality }} run: | cd gui bun run lint - name: GUI build + if: ${{ matrix.run_quality }} run: | cd gui bun run build - name: CLI help smoke + if: ${{ matrix.run_quality }} run: bun run src/cli/index.ts help npm-global-smoke: diff --git a/scripts/ci-test-shard.ts b/scripts/ci-test-shard.ts new file mode 100644 index 000000000..e78b43e9b --- /dev/null +++ b/scripts/ci-test-shard.ts @@ -0,0 +1,130 @@ +import { readdir, stat } from "node:fs/promises"; +import { join, relative, resolve, sep } from "node:path"; + +import { createIsolatedTestEnvironment } from "./test"; + +export const TEST_ROOT = resolve("tests"); +const TEST_FILE = /(?:\.test\.|\.spec\.|_test\.|_spec\.)(?:[cm]?[jt]sx?)$/i; +const BATCH_SIZE = 80; + +export interface TestFile { + path: string; + bytes: number; +} + +export async function collectTestFiles(directory: string): Promise { + const entries = await readdir(directory, { withFileTypes: true }); + const files: TestFile[] = []; + + for (const entry of entries) { + const absolute = join(directory, entry.name); + if (entry.isDirectory()) { + files.push(...await collectTestFiles(absolute)); + continue; + } + if (!entry.isFile() || !TEST_FILE.test(entry.name)) continue; + const metadata = await stat(absolute); + files.push({ + path: relative(process.cwd(), absolute).split(sep).join("/"), + bytes: metadata.size, + }); + } + + return files; +} + +export function parseInteger(value: string | undefined, name: string): number { + // Number("") coerces to 0, so a blank matrix value must be rejected explicitly. + const parsed = value === undefined || value.trim() === "" ? Number.NaN : Number(value); + if (!Number.isInteger(parsed)) throw new Error(`${name} must be an integer; got ${value ?? ""}`); + return parsed; +} + +export interface ShardSelection { + shardIndex: number; + shardCount: number; +} + +export function parseShardSelection( + shardIndexRaw: string | undefined, + shardCountRaw: string | undefined, +): ShardSelection { + const shardIndex = parseInteger(shardIndexRaw, "shardIndex"); + const shardCount = parseInteger(shardCountRaw, "shardCount"); + if (shardCount < 1) throw new Error("shardCount must be at least 1"); + if (shardIndex < 0 || shardIndex >= shardCount) { + throw new Error(`shardIndex must be between 0 and ${shardCount - 1}; got ${shardIndex}`); + } + return { shardIndex, shardCount }; +} + +export function assignBalancedShards(files: TestFile[], shardCount: number): TestFile[][] { + const shards = Array.from({ length: shardCount }, () => [] as TestFile[]); + const totals = Array.from({ length: shardCount }, () => 0); + + // File size is a stable, repository-local proxy for test cost. Greedy assignment avoids the + // severe imbalance produced by alphabetical modulo sharding while remaining deterministic. + const ordered = [...files].sort((left, right) => right.bytes - left.bytes || left.path.localeCompare(right.path)); + for (const file of ordered) { + let target = 0; + for (let index = 1; index < shardCount; index += 1) { + if (totals[index]! < totals[target]!) target = index; + } + shards[target]!.push(file); + totals[target] += file.bytes; + } + + for (const shard of shards) shard.sort((left, right) => left.path.localeCompare(right.path)); + return shards; +} + +async function runBatch(paths: string[], env: Record): Promise { + // Match the canonical scripts/test.ts orchestration: spawn the current Bun binary and run + // against an isolated HOME so shards never read or mutate the runner's real configuration. + // Bun treats bare positional test arguments as substring filters; a "./" prefix forces each + // argument to be resolved as an exact file path so a batch never pulls in unrelated tests. + const child = Bun.spawn([process.execPath, "test", "--isolate", ...paths.map(path => `./${path}`)], { + cwd: process.cwd(), + env, + stdin: "inherit", + stdout: "inherit", + stderr: "inherit", + }); + return await child.exited; +} + +async function main(): Promise { + const { shardIndex, shardCount } = parseShardSelection(Bun.argv[2], Bun.argv[3]); + + const files = await collectTestFiles(TEST_ROOT); + if (files.length === 0) throw new Error("No Bun test files found under tests/"); + + const shards = assignBalancedShards(files, shardCount); + const selected = shards[shardIndex]!; + const totalBytes = selected.reduce((sum, file) => sum + file.bytes, 0); + console.log( + `[ci-test-shard] shard ${shardIndex + 1}/${shardCount}: ${selected.length}/${files.length} files, ${totalBytes} source bytes`, + ); + + const isolated = createIsolatedTestEnvironment(); + try { + for (let offset = 0; offset < selected.length; offset += BATCH_SIZE) { + const batch = selected.slice(offset, offset + BATCH_SIZE).map(file => file.path); + console.log(`[ci-test-shard] batch ${Math.floor(offset / BATCH_SIZE) + 1}: ${batch.length} files`); + const exitCode = await runBatch(batch, isolated.env); + if (exitCode !== 0) { + process.exitCode = exitCode; + return; + } + } + } finally { + isolated.cleanup(); + } +} + +if (import.meta.main) { + main().catch(error => { + console.error(error instanceof Error ? error.stack ?? error.message : String(error)); + process.exit(1); + }); +} diff --git a/tests/ci-test-shard.test.ts b/tests/ci-test-shard.test.ts new file mode 100644 index 000000000..6bc5201f6 --- /dev/null +++ b/tests/ci-test-shard.test.ts @@ -0,0 +1,81 @@ +import { describe, expect, test } from "bun:test"; + +import { + assignBalancedShards, + collectTestFiles, + parseInteger, + parseShardSelection, + TEST_ROOT, + type TestFile, +} from "../scripts/ci-test-shard"; + +// CI relies on the shard partitioner to run every root test exactly once across the configured +// shard invocations (Windows currently uses two). A regression in discovery, path normalization, +// or assignment would silently skip or duplicate tests on the sharded platforms only, so pin the +// coverage invariant here where the full suite catches it on every platform. +describe("ci-test-shard partition invariants", () => { + const shardCounts = [1, 2, 3, 4]; + + test("discovers this test file among the root tests", async () => { + const files = await collectTestFiles(TEST_ROOT); + expect(files.map(file => file.path)).toContain("tests/ci-test-shard.test.ts"); + }); + + test.each(shardCounts)("%i shard(s) cover every discovered test exactly once", async shardCount => { + const files = await collectTestFiles(TEST_ROOT); + expect(files.length).toBeGreaterThan(0); + + const shards = assignBalancedShards(files, shardCount); + expect(shards.length).toBe(shardCount); + + const union = shards.flat().map(file => file.path); + expect(union.length).toBe(files.length); + expect(new Set(union).size).toBe(union.length); + expect([...union].sort()).toEqual(files.map(file => file.path).sort()); + }); + + test("assignment is deterministic regardless of discovery order", () => { + const files: TestFile[] = [ + { path: "tests/a.test.ts", bytes: 500 }, + { path: "tests/b.test.ts", bytes: 300 }, + { path: "tests/c.test.ts", bytes: 300 }, + { path: "tests/d.test.ts", bytes: 100 }, + ]; + const shuffled = [files[2]!, files[0]!, files[3]!, files[1]!]; + + const fromOrdered = assignBalancedShards(files, 2).map(shard => shard.map(file => file.path)); + const fromShuffled = assignBalancedShards(shuffled, 2).map(shard => shard.map(file => file.path)); + expect(fromShuffled).toEqual(fromOrdered); + }); +}); + +// A misconfigured CI matrix (missing, malformed, or out-of-range shard arguments) must fail loudly +// instead of silently running the wrong slice of the suite, so pin the CLI validation branches. +describe("ci-test-shard argument validation", () => { + test("parseInteger rejects missing values", () => { + expect(() => parseInteger(undefined, "shardIndex")).toThrow("shardIndex must be an integer; got "); + }); + + test.each(["", " ", "two", "1.5", "NaN"])("parseInteger rejects non-integer value %j", value => { + expect(() => parseInteger(value, "shardCount")).toThrow(/shardCount must be an integer/); + }); + + test("parseInteger accepts integer strings", () => { + expect(parseInteger("0", "shardIndex")).toBe(0); + expect(parseInteger("3", "shardCount")).toBe(3); + }); + + test("parseShardSelection rejects shardCount below 1", () => { + expect(() => parseShardSelection("0", "0")).toThrow("shardCount must be at least 1"); + expect(() => parseShardSelection("0", "-2")).toThrow("shardCount must be at least 1"); + }); + + test("parseShardSelection rejects out-of-range shardIndex", () => { + expect(() => parseShardSelection("-1", "2")).toThrow("shardIndex must be between 0 and 1; got -1"); + expect(() => parseShardSelection("2", "2")).toThrow("shardIndex must be between 0 and 1; got 2"); + }); + + test("parseShardSelection accepts a valid in-range configuration", () => { + expect(parseShardSelection("1", "2")).toEqual({ shardIndex: 1, shardCount: 2 }); + }); +}); diff --git a/tests/ci-workflows.test.ts b/tests/ci-workflows.test.ts index a142d1e62..ab1fcc3e5 100644 --- a/tests/ci-workflows.test.ts +++ b/tests/ci-workflows.test.ts @@ -42,7 +42,11 @@ describe("GitHub Actions hardening", () => { expect(workflow).toContain("actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0"); expect(workflow).toContain("oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6"); expect(workflow).toContain("actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e"); - expect(workflow).toContain("bun test --isolate tests"); + // Both test paths must go through the canonical isolated-environment entry points, never a + // bare `bun test` that inherits the runner's real HOME and configuration. + expect(workflow).toContain("bun run scripts/test.ts"); + expect(workflow).toContain("bun run scripts/ci-test-shard.ts"); + expect(workflow).not.toContain("bun test --isolate tests"); expect(workflow).not.toMatch(/uses:\s+\S+@(?:v\d+|main|master)\b/); });