diff --git a/.github/actions/setup-workbench/action.yml b/.github/actions/setup-workbench/action.yml new file mode 100644 index 000000000..df80c3596 --- /dev/null +++ b/.github/actions/setup-workbench/action.yml @@ -0,0 +1,18 @@ +name: Setup Workbench +description: Install Bun, restore caches, and bun install +runs: + using: composite + steps: + - uses: oven-sh/setup-bun@v2 + with: + bun-version-file: .bun-version + - uses: actions/cache@v4 + with: + path: ~/.bun/install/cache + key: bun-${{ runner.os }}-${{ hashFiles('bun.lock', '.bun-version') }} + - uses: actions/cache@v4 + with: + path: node_modules + key: node-modules-${{ runner.os }}-${{ hashFiles('bun.lock', '.bun-version') }} + - run: bun install --frozen-lockfile + shell: bash diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 07a378850..3a06063e3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -24,24 +24,13 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 - with: - fetch-depth: 0 - - uses: oven-sh/setup-bun@v2 - with: - bun-version-file: .bun-version - - uses: actions/cache@v4 - with: - path: ~/.bun/install/cache - key: bun-${{ runner.os }}-${{ hashFiles('bun.lock') }} - restore-keys: bun-${{ runner.os }}- - - run: bun install --frozen-lockfile + - uses: ./.github/actions/setup-workbench - uses: actions/cache@v4 with: path: | .eslintcache node_modules/.cache/prettier - key: lint-${{ runner.os }}-${{ github.sha }} - restore-keys: lint-${{ runner.os }}- + key: lint-${{ runner.os }}-${{ hashFiles('bun.lock', 'eslint.config.ts', '.prettierrc.json', '.bun-version') }} - run: bun run lint typecheck: @@ -50,15 +39,7 @@ jobs: - uses: actions/checkout@v4 with: fetch-depth: 0 - - uses: oven-sh/setup-bun@v2 - with: - bun-version-file: .bun-version - - uses: actions/cache@v4 - with: - path: ~/.bun/install/cache - key: bun-${{ runner.os }}-${{ hashFiles('bun.lock') }} - restore-keys: bun-${{ runner.os }}- - - run: bun install --frozen-lockfile + - uses: ./.github/actions/setup-workbench - run: bun run typecheck env: WORKBENCH_CHECK_SINCE: ${{ github.event.pull_request.base.sha }} @@ -69,15 +50,7 @@ jobs: - uses: actions/checkout@v4 with: fetch-depth: 0 - - uses: oven-sh/setup-bun@v2 - with: - bun-version-file: .bun-version - - uses: actions/cache@v4 - with: - path: ~/.bun/install/cache - key: bun-${{ runner.os }}-${{ hashFiles('bun.lock') }} - restore-keys: bun-${{ runner.os }}- - - run: bun install --frozen-lockfile + - uses: ./.github/actions/setup-workbench - run: bun run build - run: bun run test env: @@ -89,15 +62,7 @@ jobs: - uses: actions/checkout@v4 with: fetch-depth: 0 - - uses: oven-sh/setup-bun@v2 - with: - bun-version-file: .bun-version - - uses: actions/cache@v4 - with: - path: ~/.bun/install/cache - key: bun-${{ runner.os }}-${{ hashFiles('bun.lock') }} - restore-keys: bun-${{ runner.os }}- - - run: bun install --frozen-lockfile + - uses: ./.github/actions/setup-workbench - name: Structural check self-tests run: bun test scripts/checks/test - run: bun run check:deletion @@ -116,7 +81,7 @@ jobs: env: CHECK_BASE_REF: ${{ github.event.pull_request.base.sha }} - walking-skeleton: + e2e: runs-on: ubuntu-latest timeout-minutes: 20 @@ -138,17 +103,7 @@ jobs: steps: - uses: actions/checkout@v4 - with: - fetch-depth: 0 - - uses: oven-sh/setup-bun@v2 - with: - bun-version-file: .bun-version - - uses: actions/cache@v4 - with: - path: ~/.bun/install/cache - key: bun-${{ runner.os }}-${{ hashFiles('bun.lock') }} - restore-keys: bun-${{ runner.os }}- - - run: bun install --frozen-lockfile + - uses: ./.github/actions/setup-workbench # The committed template ships a placeholder secret and a # credential-less local DATABASE_URL; point the URL at the service @@ -181,8 +136,100 @@ jobs: E2E_REQUIRED: "1" DATABASE_URL: postgres://postgres:postgres@localhost:5432/workbench + isolation: + runs-on: ubuntu-latest + timeout-minutes: 15 + + services: + postgres: + # pgvector-enabled Postgres 17, matching the local development + # database (brew postgresql@17 + pgvector). + image: pgvector/pgvector:pg17 + env: + POSTGRES_USER: postgres + POSTGRES_PASSWORD: postgres + ports: + - 5432:5432 + options: >- + --health-cmd "pg_isready -U postgres -d postgres" + --health-interval 10s + --health-timeout 5s + --health-retries 5 + + steps: + - uses: actions/checkout@v4 + - uses: ./.github/actions/setup-workbench + + - name: Write env file + run: | + cp .env.example .env + secret=$(openssl rand -hex 32) + sed -i "s|^SESSION_SECRET=.*|SESSION_SECRET=${secret}|" .env + sed -i "s|^DATABASE_URL=.*|DATABASE_URL=postgres://postgres:postgres@localhost:5432/workbench|" .env + + - name: Assert the e2e test env is wired + run: | + test -f .env + grep -q '^DATABASE_URL=postgres://postgres:postgres@localhost:5432/workbench$' .env + grep -Eq '^SESSION_SECRET=.{32,}$' .env + pg_isready -h localhost -p 5432 + git --version + - name: Run the two-org isolation suite run: bun test test/isolation + env: + E2E_REQUIRED: "1" + DATABASE_URL: postgres://postgres:postgres@localhost:5432/workbench + + db-suites: + runs-on: ubuntu-latest + timeout-minutes: 20 + + services: + postgres: + # pgvector-enabled Postgres 17, matching the local development + # database (brew postgresql@17 + pgvector). + image: pgvector/pgvector:pg17 + env: + POSTGRES_USER: postgres + POSTGRES_PASSWORD: postgres + ports: + - 5432:5432 + options: >- + --health-cmd "pg_isready -U postgres -d postgres" + --health-interval 10s + --health-timeout 5s + --health-retries 5 + + steps: + - uses: actions/checkout@v4 + - uses: ./.github/actions/setup-workbench + + - name: Write env file + run: | + cp .env.example .env + secret=$(openssl rand -hex 32) + sed -i "s|^SESSION_SECRET=.*|SESSION_SECRET=${secret}|" .env + sed -i "s|^DATABASE_URL=.*|DATABASE_URL=postgres://postgres:postgres@localhost:5432/workbench|" .env + + - name: Assert the e2e test env is wired + run: | + test -f .env + grep -q '^DATABASE_URL=postgres://postgres:postgres@localhost:5432/workbench$' .env + grep -Eq '^SESSION_SECRET=.{32,}$' .env + pg_isready -h localhost -p 5432 + git --version + + # apps/hub/test and most of the package suites below connect + # straight to DATABASE_URL (or its `_e2e`-suffixed sibling from + # `e2eDatabaseUrl()`) rather than booting through the harness, so + # — unlike e2e and isolation, which provision their own schema — + # this job must create and migrate both databases itself before + # those suites run. + - name: Set up the databases + run: | + DATABASE_URL=postgres://postgres:postgres@localhost:5432/workbench bun scripts/db-setup.ts + DATABASE_URL=postgres://postgres:postgres@localhost:5432/workbench_e2e bun scripts/db-setup.ts # apps/hub's own DB-backed suites (e.g. the sign-up rate-limit # proof) never run under the plain `checks` job, which has no diff --git a/scripts/ci-jobs.test.ts b/scripts/ci-jobs.test.ts new file mode 100644 index 000000000..c7ea9c3eb --- /dev/null +++ b/scripts/ci-jobs.test.ts @@ -0,0 +1,115 @@ +// Pins the CI job split so a flake names the suite that failed, and so +// checkout depth / cache keys cannot silently revert to the old shared job. +import { expect, test } from "bun:test"; +import { readFile } from "node:fs/promises"; +import { join } from "node:path"; + +const ROOT = join(import.meta.dir, ".."); + +function jobBodies(yaml: string): Map { + const marker = "\njobs:\n"; + const jobsIndex = yaml.indexOf(marker); + if (jobsIndex < 0) throw new Error("ci.yml has no jobs: block"); + const jobsSection = yaml.slice(jobsIndex + marker.length); + const heading = /^ {2}([a-z][a-z0-9-]*):$/gm; + const matches = [...jobsSection.matchAll(heading)]; + const bodies = new Map(); + for (let i = 0; i < matches.length; i++) { + const name = matches[i]?.[1]; + if (name === undefined) continue; + const start = (matches[i]?.index ?? 0) + (matches[i]?.[0].length ?? 0); + const end = matches[i + 1]?.index ?? jobsSection.length; + bodies.set(name, jobsSection.slice(start, end)); + } + return bodies; +} + +const SETUP = "./.github/actions/setup-workbench"; +const POSTGRES_IMAGE = "pgvector/pgvector:pg17"; +const DB_JOBS = ["e2e", "isolation", "db-suites"] as const; +const MERGE_BASE_JOBS = ["typecheck", "build-test", "structural"] as const; + +test("CI splits e2e, isolation, and db-suites onto their own Postgres jobs", async () => { + const yaml = await readFile(join(ROOT, ".github/workflows/ci.yml"), "utf8"); + const jobs = jobBodies(yaml); + + expect(jobs.has("walking-skeleton")).toBe(false); + for (const name of DB_JOBS) { + const body = jobs.get(name); + expect(body).toBeDefined(); + expect(body).toContain("postgres:"); + expect(body).toContain(`image: ${POSTGRES_IMAGE}`); + expect(body).toContain(`uses: ${SETUP}`); + expect(body).not.toContain("fetch-depth: 0"); + } + + const dbSuites = jobs.get("db-suites") ?? ""; + expect(dbSuites).toContain("E2E_REQUIRED:"); + expect(dbSuites).toContain("bun test apps/hub/test"); + expect(dbSuites).toContain("grep -rl DATABASE_URL"); + + // Unlike e2e and isolation, which provision their own schema through + // the harness, apps/hub/test and most package suites connect straight + // to DATABASE_URL or its `_e2e`-suffixed sibling from + // `e2eDatabaseUrl()` — both databases must exist and be migrated + // before those suites run, or their queries fail with "database ... + // does not exist". + const plainSetupIndex = dbSuites.indexOf( + "postgres://postgres:postgres@localhost:5432/workbench bun scripts/db-setup.ts", + ); + const e2eSetupIndex = dbSuites.indexOf( + "postgres://postgres:postgres@localhost:5432/workbench_e2e bun scripts/db-setup.ts", + ); + const hubSuiteIndex = dbSuites.indexOf("bun test apps/hub/test"); + expect(plainSetupIndex).toBeGreaterThan(-1); + expect(e2eSetupIndex).toBeGreaterThan(-1); + expect(plainSetupIndex).toBeLessThan(hubSuiteIndex); + expect(e2eSetupIndex).toBeLessThan(hubSuiteIndex); +}); + +test("jobs that need merge-base fetch full history; the rest stay shallow", async () => { + const yaml = await readFile(join(ROOT, ".github/workflows/ci.yml"), "utf8"); + const jobs = jobBodies(yaml); + + for (const name of MERGE_BASE_JOBS) { + const body = jobs.get(name); + expect(body).toBeDefined(); + expect(body).toContain(`uses: ${SETUP}`); + expect(body).toContain("fetch-depth: 0"); + } + + const lint = jobs.get("lint"); + expect(lint).toBeDefined(); + expect(lint).toContain(`uses: ${SETUP}`); + expect(lint).not.toContain("fetch-depth: 0"); +}); + +test("lint cache keys on tool and config versions, not an OS-wide restore", async () => { + const yaml = await readFile(join(ROOT, ".github/workflows/ci.yml"), "utf8"); + const lint = jobBodies(yaml).get("lint") ?? ""; + + expect(lint).toContain( + "hashFiles('bun.lock', 'eslint.config.ts', '.prettierrc.json', '.bun-version')", + ); + expect(lint).not.toContain("lint-${{ runner.os }}-${{ github.sha }}"); + expect(lint).not.toContain("restore-keys: lint-${{ runner.os }}-"); +}); + +test("setup-workbench caches bun install and node_modules on the lockfile", async () => { + const action = await readFile( + join(ROOT, ".github/actions/setup-workbench/action.yml"), + "utf8", + ); + + expect(action).toContain("using: composite"); + expect(action).not.toContain("actions/checkout"); + expect(action).not.toContain("fetch-depth:"); + expect(action).toContain( + "key: bun-${{ runner.os }}-${{ hashFiles('bun.lock', '.bun-version') }}", + ); + expect(action).toContain( + "key: node-modules-${{ runner.os }}-${{ hashFiles('bun.lock', '.bun-version') }}", + ); + expect(action).not.toContain("restore-keys:"); + expect(action).toContain("bun install --frozen-lockfile"); +}); diff --git a/scripts/run-all.test.ts b/scripts/run-all.test.ts index e0686cefd..cff184d18 100644 --- a/scripts/run-all.test.ts +++ b/scripts/run-all.test.ts @@ -7,6 +7,7 @@ import { mkdir, mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { basename, join } from "node:path"; +import { resolveConcurrency } from "./run-all.ts"; import { SEQUENTIAL_SCRIPTS } from "./sequential-scripts.ts"; const RUNNER = join(import.meta.dir, "run-all.ts"); @@ -68,9 +69,14 @@ async function runProbe( // WORKBENCH_CHECK_SINCE exported resolves the filter against the real repo's // git rather than the fixture, every probe package reads as unaffected, and // the runner correctly reports "nothing affected" — failing tests that are - // actually about something else. A case that wants either variable sets it - // through `extraEnv`. - const { WORKBENCH_CHECK_SINCE: _since, ...ambient } = process.env; + // actually about something else. GITHUB_ACTIONS would also change the + // default fan-out, so a case that wants either variable sets it through + // `extraEnv`. + const { + WORKBENCH_CHECK_SINCE: _since, + GITHUB_ACTIONS: _actions, + ...ambient + } = process.env; const child = Bun.spawn(["bun", "run", RUNNER, script], { cwd: workspace, env: { ...ambient, PROBE_LOG: logPath, ...extraEnv }, @@ -191,6 +197,21 @@ describe("run-all", () => { expect(result.stderr).toContain("WORKBENCH_CHECK_CONCURRENCY"); }); + test("uses every core in GitHub Actions and leaves two free locally", () => { + expect(resolveConcurrency("typecheck", {}, 8)).toBe(6); + expect(resolveConcurrency("typecheck", { GITHUB_ACTIONS: "true" }, 8)).toBe( + 8, + ); + expect( + resolveConcurrency( + "typecheck", + { GITHUB_ACTIONS: "true", WORKBENCH_CHECK_CONCURRENCY: "3" }, + 8, + ), + ).toBe(3); + expect(resolveConcurrency("test", { GITHUB_ACTIONS: "true" }, 8)).toBe(1); + }); + test("fails the run and names the package whose script failed", async () => { const result = await runProbe({ PROBE_FAIL: "charlie" }); diff --git a/scripts/run-all.ts b/scripts/run-all.ts index 19830635e..958e63abf 100644 --- a/scripts/run-all.ts +++ b/scripts/run-all.ts @@ -10,13 +10,20 @@ const CONCURRENCY_ENV = "WORKBENCH_CHECK_CONCURRENCY"; type Job = { readonly name: string; readonly dir: string }; -function resolveConcurrency(script: string): number { - const raw = process.env[CONCURRENCY_ENV]; +export function resolveConcurrency( + script: string, + env: NodeJS.ProcessEnv = process.env, + cores: number = availableParallelism(), +): number { + const raw = env[CONCURRENCY_ENV]; if (raw === undefined || raw === "") { if (SEQUENTIAL_SCRIPTS.has(script)) return 1; - // Each job saturates about one core, so leave a couple free for the editor - // and type server a developer runs alongside the gate. - return Math.max(1, availableParallelism() - 2); + // Locally each job saturates about one core, so leave a couple free for + // the editor and type server a developer runs alongside the gate. CI + // runners have no editor — use every core so package fan-out is not + // artificially capped. + if (env["GITHUB_ACTIONS"] === "true") return Math.max(1, cores); + return Math.max(1, cores - 2); } const parsed = Number.parseInt(raw, 10); if (!Number.isInteger(parsed) || parsed < 1) { @@ -136,59 +143,61 @@ async function narrowToAffected(jobs: readonly Job[]): Promise { return narrowed; } -const scriptArg = process.argv[2]; -if (!scriptArg) { - console.error("usage: bun run scripts/run-all.ts "); - process.exit(1); -} -const script: string = scriptArg; - -let concurrency: number; -try { - concurrency = resolveConcurrency(script); -} catch (cause) { - console.error(cause instanceof Error ? cause.message : String(cause)); - process.exit(1); -} +if (import.meta.main) { + const scriptArg = process.argv[2]; + if (!scriptArg) { + console.error("usage: bun run scripts/run-all.ts "); + process.exit(1); + } + const script: string = scriptArg; + + let concurrency: number; + try { + concurrency = resolveConcurrency(script); + } catch (cause) { + console.error(cause instanceof Error ? cause.message : String(cause)); + process.exit(1); + } -const discovered = await discover(script); -const jobs = await narrowToAffected(discovered); -if (jobs.length === 0) { - const reason = - discovered.length === 0 - ? "no workspace packages define it yet" - : "nothing affected by this change"; - console.log(`${script}: ${reason}`); - process.exit(0); -} + const discovered = await discover(script); + const jobs = await narrowToAffected(discovered); + if (jobs.length === 0) { + const reason = + discovered.length === 0 + ? "no workspace packages define it yet" + : "nothing affected by this change"; + console.log(`${script}: ${reason}`); + process.exit(0); + } -const failures: string[] = []; -let nextJob = 0; - -async function worker(): Promise { - while (nextJob < jobs.length) { - const job = jobs[nextJob]; - nextJob += 1; - if (job === undefined) return; - - // Progress goes to stderr so stdout stays a clean sequence of per-package - // blocks; a ten-minute gate that prints nothing until the end reads as hung. - console.error(`started ${job.name}`); - const code = await runJob(job, script); - if (code !== 0) { - failures.push(job.name); - console.error(`${job.name}: ${script} exited with code ${code}`); + const failures: string[] = []; + let nextJob = 0; + + async function worker(): Promise { + while (nextJob < jobs.length) { + const job = jobs[nextJob]; + nextJob += 1; + if (job === undefined) return; + + // Progress goes to stderr so stdout stays a clean sequence of per-package + // blocks; a ten-minute gate that prints nothing until the end reads as hung. + console.error(`started ${job.name}`); + const code = await runJob(job, script); + if (code !== 0) { + failures.push(job.name); + console.error(`${job.name}: ${script} exited with code ${code}`); + } } } -} -await Promise.all( - Array.from({ length: Math.min(concurrency, jobs.length) }, () => worker()), -); - -if (failures.length > 0) { - console.error( - `${script} failed in ${failures.length} package(s): ${failures.join(", ")}`, + await Promise.all( + Array.from({ length: Math.min(concurrency, jobs.length) }, () => worker()), ); - process.exit(1); + + if (failures.length > 0) { + console.error( + `${script} failed in ${failures.length} package(s): ${failures.join(", ")}`, + ); + process.exit(1); + } }