diff --git a/.github/workflows/node-ci.yml b/.github/workflows/node-ci.yml index 0ca7b58284..9a47bae567 100644 --- a/.github/workflows/node-ci.yml +++ b/.github/workflows/node-ci.yml @@ -80,9 +80,16 @@ jobs: ;; .github/workflows/node-ci.yml | .github/actions/setup-tools/*) container_validate=true - test_quality=true ;; - .github/workflows/test-quality.yml) + esac + case "$path" in + .github/workflows/node-ci.yml | .github/workflows/test-quality.yml | \ + .github/actions/setup-tools/* | sdk/typescript/bunfig.toml | \ + sdk/typescript/scripts/compare-test-reports.mts | \ + sdk/typescript/scripts/prepare-windows-test-root.ps1 | \ + sdk/typescript/scripts/run-ci-tests.mts | \ + sdk/typescript/scripts/test-shards.mts | \ + sdk/typescript/scripts/test-timings.json) test_quality=true ;; esac diff --git a/.github/workflows/test-quality.yml b/.github/workflows/test-quality.yml index 83d13c9a7b..011949f140 100644 --- a/.github/workflows/test-quality.yml +++ b/.github/workflows/test-quality.yml @@ -30,68 +30,33 @@ jobs: runner: needs: native if: ${{ !cancelled() && (inputs.native-artifacts-ready || needs.native.result == 'success') }} - name: ${{ matrix.os }} / ${{ matrix.mode }} - runs-on: ${{ matrix.os }} - timeout-minutes: ${{ matrix.os == 'windows-latest' && 90 || 30 }} + name: ${{ matrix.target.os }} / ${{ matrix.mode.name }} / shard-${{ matrix.target.shard }} + runs-on: ${{ matrix.target.os }} + timeout-minutes: ${{ matrix.target.os == 'windows-latest' && 90 || 30 }} env: CODEX_SECURITY_PROPERTY_SEED: ${{ github.event_name == 'pull_request' && 1 || github.run_number }} strategy: fail-fast: false matrix: - os: [ubuntu-latest, windows-latest] - mode: [baseline, isolated, parallel] - exclude: - - os: windows-latest - mode: isolated - include: - - mode: baseline - args: "" - - mode: isolated - args: --isolate - - mode: parallel - args: --parallel=2 - - os: windows-latest - mode: isolated-1 - args: --isolate --shard=1/7 - - os: windows-latest - mode: isolated-2 - args: --isolate --shard=2/7 - - os: windows-latest - mode: isolated-3 - args: --isolate --shard=3/7 - - os: windows-latest - mode: isolated-4 - args: --isolate --shard=4/7 - - os: windows-latest - mode: isolated-5 - args: --isolate --shard=5/7 - - os: windows-latest - mode: isolated-6 - args: --isolate --shard=6/7 - - os: windows-latest - mode: isolated-7 - args: --isolate --shard=7/7 - - os: windows-latest - mode: shard-1 - args: --shard=1/7 - - os: windows-latest - mode: shard-2 - args: --shard=2/7 - - os: windows-latest - mode: shard-3 - args: --shard=3/7 - - os: windows-latest - mode: shard-4 - args: --shard=4/7 - - os: windows-latest - mode: shard-5 - args: --shard=5/7 - - os: windows-latest - mode: shard-6 - args: --shard=6/7 - - os: windows-latest - mode: shard-7 - args: --shard=7/7 + # Every mode uses the same production shard count and timing snapshot. + target: + - { os: ubuntu-latest, shard: 1, count: 3 } + - { os: ubuntu-latest, shard: 2, count: 3 } + - { os: ubuntu-latest, shard: 3, count: 3 } + - { os: macos-latest, shard: 1, count: 3 } + - { os: macos-latest, shard: 2, count: 3 } + - { os: macos-latest, shard: 3, count: 3 } + - { os: windows-latest, shard: 1, count: 7 } + - { os: windows-latest, shard: 2, count: 7 } + - { os: windows-latest, shard: 3, count: 7 } + - { os: windows-latest, shard: 4, count: 7 } + - { os: windows-latest, shard: 5, count: 7 } + - { os: windows-latest, shard: 6, count: 7 } + - { os: windows-latest, shard: 7, count: 7 } + mode: + - { name: baseline, args: "" } + - { name: isolated, args: --isolate } + - { name: parallel, args: --parallel=2 } steps: - &actions-checkout uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 @@ -116,17 +81,9 @@ jobs: sdk/typescript/pnpm-lock.yaml plugins/codex-security/mcp-app/pnpm-lock.yaml - uses: oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6 # v2 - if: runner.os == 'Windows' with: - bun-version: "1.4.2" - - name: Set up Bun on Linux - if: runner.os == 'Linux' - shell: bash - run: | - # Includes the subprocess fix from https://github.com/oven-sh/bun/pull/44581 - # for the isolated-runner hang tracked in https://github.com/oven-sh/bun/issues/34069. - npm install --prefix "$RUNNER_TEMP/bun" --ignore-scripts --no-audit --no-fund @oven/bun-linux-x64@1.4.2-canary.20261008.1 - echo "$RUNNER_TEMP/bun/node_modules/@oven/bun-linux-x64/bin" >> "$GITHUB_PATH" + # Includes the isolated spawnSync fix from oven-sh/bun#44581. + bun-version: "1.4.3" - name: Install dependencies run: | pnpm --dir sdk/typescript install --frozen-lockfile @@ -136,20 +93,25 @@ jobs: id: windows-temp shell: pwsh run: ./sdk/typescript/scripts/prepare-windows-test-root.ps1 + - name: Build bundled plugin + run: pnpm --dir sdk/typescript run build:plugin + - name: Install ripgrep + if: runner.os == 'Linux' + run: | + sudo apt-get update + sudo apt-get install --yes ripgrep - name: Test runner mode + working-directory: sdk/typescript env: - CODEX_SECURITY_TEST_TIMEOUT_MS: ${{ runner.os == 'Windows' && '120000' || '30000' }} TEMP: ${{ steps.windows-temp.outputs.path || runner.temp }} TMP: ${{ steps.windows-temp.outputs.path || runner.temp }} TMPDIR: ${{ steps.windows-temp.outputs.path || runner.temp }} - run: | - node -e "require('node:fs').mkdirSync('sdk/typescript/reports',{recursive:true})" - pnpm --dir sdk/typescript run test ${{ runner.os == 'Windows' && '--timeout=120000' || '' }} ${{ matrix.args }} --seed=${{ env.CODEX_SECURITY_PROPERTY_SEED }} --reporter=junit --reporter-outfile=reports/runner-${{ matrix.os }}-${{ matrix.mode }}.xml + run: node --experimental-strip-types scripts/run-ci-tests.mts ${{ matrix.target.shard }}/${{ matrix.target.count }} ${{ matrix.mode.args }} --seed=${{ env.CODEX_SECURITY_PROPERTY_SEED }} --reporter-outfile=reports/runner-${{ matrix.target.os }}-${{ matrix.mode.name }}-${{ matrix.target.shard }}.xml - name: Upload runner report if: always() uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2 with: - name: runner-${{ matrix.os }}-${{ matrix.mode }} + name: runner-${{ matrix.target.os }}-${{ matrix.mode.name }}-${{ matrix.target.shard }} overwrite: true path: sdk/typescript/reports/runner-*.xml if-no-files-found: error @@ -178,12 +140,28 @@ jobs: shell: bash run: | comparison_status=0 - for os in ubuntu-latest windows-latest; do + shopt -s nullglob + for os in ubuntu-latest macos-latest windows-latest; do + shards=3 + if [[ "$os" == windows-latest ]]; then shards=7; fi + for mode in baseline isolated parallel; do + reports=(reports/runner-"$os"-"$mode"-*.xml) + if (( ${#reports[@]} != shards )); then + echo "Expected $shards reports for $os/$mode, found ${#reports[@]}" >&2 + comparison_status=1 + fi + for ((shard=1; shard<=shards; shard++)); do + report="reports/runner-$os-$mode-$shard.xml" + if [[ ! -f "$report" ]]; then + echo "Missing shard report: $report" >&2 + comparison_status=1 + fi + done + done for mode in isolated parallel; do - node --experimental-strip-types sdk/typescript/scripts/compare-test-reports.mts "reports/runner-$os-baseline.xml" "reports/runner-$os-$mode*.xml" >> "$GITHUB_STEP_SUMMARY" || comparison_status=1 + node --experimental-strip-types sdk/typescript/scripts/compare-test-reports.mts "reports/runner-$os-baseline-*.xml" "reports/runner-$os-$mode-*.xml" >> "$GITHUB_STEP_SUMMARY" || comparison_status=1 done done - node --experimental-strip-types sdk/typescript/scripts/compare-test-reports.mts reports/runner-windows-latest-baseline.xml 'reports/runner-windows-latest-shard-*.xml' >> "$GITHUB_STEP_SUMMARY" || comparison_status=1 exit "$comparison_status" mutation: diff --git a/sdk/typescript/TESTING.md b/sdk/typescript/TESTING.md index 1c3ccdf515..fe5c8a1650 100644 --- a/sdk/typescript/TESTING.md +++ b/sdk/typescript/TESTING.md @@ -119,7 +119,8 @@ The requirements file is the single list of pinned Python test dependencies. Ruf - Add property tests for meaningful invariants, with accepted and rejected inputs. Keep example-based regression tests for readable failure cases. - Give parameterized cases distinct names. Use `%p`, `%j`, or `%#` for values - that are not strings. The JUnit comparison rejects duplicate identities. + that are not strings. Duplicate identities remain valid; the JUnit comparison + checks their occurrence and outcome counts. Property tests use a fixed default seed. Fast-check prints the seed, shrink path, and counterexample on failure. To replay one property, select its file @@ -174,19 +175,40 @@ Failed tests and missing package artifacts block CI; failed diagnostic uploads do not. Use `python -m pytest -n 0` to reproduce Python failures serially. The separate `test-quality` workflow runs weekly, can be dispatched manually, -and runs on pull requests that change its workflow file. It compares Bun's -default runner, `--isolate`, `--parallel=2`, and seven-way Windows sharding. -Every mode uses the same seed for property cases and test ordering: pull -requests replay seed 1; scheduled and manual runs use the workflow run number. -The workflow compares test identities and outcomes against the unsharded -default run and records timings, including failed shards, in the job summary. -It is not a required check or part of the release trigger. - -Runner trials use Bun 1.4.2 to avoid the -[async-module initialization bug in 1.3.14](https://github.com/oven-sh/bun/issues/31410) -that breaks the Ink UI tests under isolation. Keep the trial pin until a newer -release passes the full SDK suite in every mode. Required CI and the mutation -trial remain on Bun 1.3.14. +and runs on pull requests that change its workflow, setup tools, Bun configuration, +shard runner, timing metadata, report comparator, or Windows test-root helper. +It compares Bun's default runner, `--isolate`, and `--parallel=2` on Linux, macOS, +and Windows. +Each mode uses the same duration-based file assignment and production shard count: +three on Linux/macOS and seven on Windows. This gives 39 sharded runner jobs in place +of 19 jobs that included an unsharded Windows baseline. It measures the production +partition rather than cross-file interactions across one unsharded suite; local +full-suite runs still cover that ordering. More jobs add setup overhead, so compare +workflow wall time and runner minutes, alongside shard test timings, before +adopting a runner change. + +Every mode uses the same seed for property cases and test ordering: pull requests +replay seed 1; scheduled and manual runs use the workflow run number. Reports have +unique OS/mode/shard names. The workflow requires every expected shard report +before accepting comparisons, then compares complete shard groups by test +identity, outcome, and occurrence count, retaining legitimate repeated names from +parameterized tests while detecting missing or extra instances. It records every +shard's timing, including failed shards, in the job summary. It is not a required +check or part of the release trigger. + +Runner trials use Bun 1.4.3, including the +[isolated subprocess fix](https://github.com/oven-sh/bun/pull/44581). +`--parallel=2` runs files in two worker processes and isolates each file; it does +not make tests inside one file concurrent. Required CI and the mutation trial +remain on Bun 1.3.14 until a newer release passes the full SDK suite in every mode +on all three platforms. + +Runner modes use the same property seed and coverage settings as each other. +Required CI uses the default property seed and collects Linux coverage, while +these diagnostics use the seed above and omit coverage instrumentation. Compare +inventories across required CI and diagnostics, but measure scheduling speed +between diagnostic modes to avoid attributing these differences to the runner. +Both workflows install ripgrep on Linux so the inventory-path test executes. Keep the machine-policy test serial. Do not replace the full required suite with `--changed`: Python files, schemas, fixtures, and workflows loaded at diff --git a/sdk/typescript/scripts/compare-test-reports.mts b/sdk/typescript/scripts/compare-test-reports.mts index 75ed2307b5..0e44f840e0 100644 --- a/sdk/typescript/scripts/compare-test-reports.mts +++ b/sdk/typescript/scripts/compare-test-reports.mts @@ -114,7 +114,6 @@ function readReport(path: string) { ); parser.write(content).close(); - const identities = new Set(); const cases = new Map(); for (const { attributes, status } of records) { const identity: TestIdentity = [ @@ -124,18 +123,13 @@ function readReport(path: string) { attributes["classname"]?.value ?? "", attributes["name"]?.value ?? "", ]; - const key = JSON.stringify(identity); - if (identities.has(key)) - throw new Error( - `${path}: duplicate test identity: ${identity.join(" > ")}`, - ); - identities.add(key); - cases.set(JSON.stringify([...identity, status]), 1); + const key = JSON.stringify([...identity, status]); + cases.set(key, (cases.get(key) ?? 0) + 1); } if (!cases.size) throw new Error(`${path}: no test cases`); if ( - integer(root!.attributes["tests"]?.value ?? String(cases.size)) !== - BigInt(cases.size) + integer(root!.attributes["tests"]?.value ?? String(records.length)) !== + BigInt(records.length) ) { throw new Error(`${path}: reported test count does not match test cases`); } @@ -150,7 +144,7 @@ function readReport(path: string) { const duration = seconds(root!.attributes["time"]?.value ?? "0"); const skipped = records.filter(({ status }) => status === "skipped").length; console.log( - `| ${basename(path)} | ${cases.size} | ${skipped} | ${duration.toFixed(2)} |`, + `| ${basename(path)} | ${records.length} | ${skipped} | ${duration.toFixed(2)} |`, ); return { cases, @@ -159,6 +153,26 @@ function readReport(path: string) { }; } +function readReports(patterns: string[]) { + const cases = new Map(); + const durations: number[] = []; + let failed = false; + for (const pattern of patterns) { + const paths = matchingReports(pattern).sort((a, b) => + Buffer.compare(Buffer.from(a), Buffer.from(b)), + ); + if (!paths.length) throw new Error(`No reports match ${pattern}`); + for (const path of paths) { + const report = readReport(path); + failed ||= report.failed; + for (const [identity, count] of report.cases) + cases.set(identity, (cases.get(identity) ?? 0) + count); + durations.push(report.duration); + } + } + return { cases, durations, failed }; +} + function main(): number { let args: { values: { help?: boolean }; positionals: string[] }; try { @@ -176,32 +190,18 @@ function main(): number { } if (args.values.help) { console.log( - "Compare Bun JUnit inventories before changing the required CI runner.\n\nUsage: node --experimental-strip-types compare-test-reports.mts baseline candidates [candidates ...]\nCandidates are JUnit files or glob patterns.", + "Compare Bun JUnit inventories before changing the required CI runner.\n\nUsage: node --experimental-strip-types compare-test-reports.mts baseline candidates [candidates ...]\nBaseline and candidates are JUnit files or glob patterns.", ); return 0; } console.log("| Report | Cases | Skipped | Seconds |"); console.log("| --- | ---: | ---: | ---: |"); - const baseline = readReport(args.positionals[0]!); - let failed = baseline.failed; - const candidates = new Map(); - const durations: number[] = []; - for (const pattern of args.positionals.slice(1)) { - const paths = matchingReports(pattern).sort((a, b) => - Buffer.compare(Buffer.from(a), Buffer.from(b)), - ); - if (!paths.length) throw new Error(`No reports match ${pattern}`); - for (const path of paths) { - const report = readReport(path); - failed ||= report.failed; - for (const [identity, count] of report.cases) - candidates.set(identity, (candidates.get(identity) ?? 0) + count); - durations.push(report.duration); - } - } + const baseline = readReports([args.positionals[0]!]); + const candidate = readReports(args.positionals.slice(1)); + let failed = baseline.failed || candidate.failed; for (const [label, left, right] of [ - ["Missing", baseline.cases, candidates], - ["Extra", candidates, baseline.cases], + ["Missing", baseline.cases, candidate.cases], + ["Extra", candidate.cases, baseline.cases], ] as const) { const differences = [...left] .map(([identity, count]): [TestOutcome, number] => [ @@ -225,8 +225,13 @@ function main(): number { } } if (failed) return 1; + if (baseline.durations.length > 1) { + console.log( + `\nBaseline shards: slowest ${Math.max(...baseline.durations).toFixed(2)}s; combined test time: ${baseline.durations.reduce((sum, duration) => sum + duration, 0).toFixed(2)}s.`, + ); + } console.log( - `\nIdentical test inventory and outcomes. Slowest candidate: ${Math.max(...durations).toFixed(2)}s; combined test time: ${durations.reduce((sum, duration) => sum + duration, 0).toFixed(2)}s.\n`, + `\nIdentical test inventory and outcomes. Slowest candidate: ${Math.max(...candidate.durations).toFixed(2)}s; combined test time: ${candidate.durations.reduce((sum, duration) => sum + duration, 0).toFixed(2)}s.\n`, ); return 0; } diff --git a/sdk/typescript/tests-ts/cli-skill-native-config.test.ts b/sdk/typescript/tests-ts/cli-skill-native-config.test.ts index 007ce942a0..59a63a8cf8 100644 --- a/sdk/typescript/tests-ts/cli-skill-native-config.test.ts +++ b/sdk/typescript/tests-ts/cli-skill-native-config.test.ts @@ -1,6 +1,13 @@ import { execFileSync, spawn } from "node:child_process"; import { once } from "node:events"; -import { mkdir, mkdtemp, realpath, rm, writeFile } from "node:fs/promises"; +import { + mkdir, + mkdtemp, + readFile, + realpath, + rm, + writeFile, +} from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { fileURLToPath } from "node:url"; @@ -18,6 +25,97 @@ import { } from "../src/runtime.js"; import { capture } from "./cli-fixtures.js"; +test.each([ + ["codex_security_deep_scan", true, false], + ["codex_security_deep_scan", true, true], + ["codex_security_deep_scan", false, false], + ["codex-security", true, false], +] as const)( + "native configuration fixture owns preflight shutdown: client=%s, layers=%p, error=%p", + async (clientName, includeLayers, error) => { + const root = await mkdtemp(join(tmpdir(), "native-config-lifetime-")); + const exited = join(root, "native-exited"); + const response = { + id: 2, + ...(error + ? { error: { code: -32603, message: "synthetic configuration error" } } + : { + result: { + config: { model: "synthetic-model", daybreak: true }, + origins: { daybreak: { name: { type: "user" } } }, + layers: [{ name: { type: "user" }, config: { daybreak: true } }], + }, + }), + }; + const child = spawn( + process.execPath, + [ + fileURLToPath( + new URL("./fixtures/skill-native-config.mjs", import.meta.url), + ), + "-e", + `const { createInterface } = require("node:readline"); + const { writeFileSync } = require("node:fs"); + createInterface({ input: process.stdin }).on("line", line => { + const request = JSON.parse(line); + const response = request.method === "config/read" + ? ${JSON.stringify(response)} + : { id: request.id, result: {} }; + process.stdout.write(JSON.stringify(response) + "\\n"); + }); + process.on("exit", () => writeFileSync(${JSON.stringify(exited)}, "exited"));`, + ], + { + env: { ...process.env, SYNTHETIC_NATIVE_CODEX: process.execPath }, + stdio: ["pipe", "pipe", "pipe"], + windowsHide: true, + }, + ); + const closed = once(child, "close"); + const lines = createInterface({ input: child.stdout }); + const timeout = setTimeout(() => child.kill(), 10_000); + try { + child.stdin.write( + JSON.stringify({ + id: 1, + method: "initialize", + params: { clientInfo: { name: clientName } }, + }) + "\n", + ); + await once(lines, "line"); + child.stdin.write( + JSON.stringify({ + id: 2, + method: "config/read", + params: { includeLayers }, + }) + "\n", + ); + const [line] = await once(lines, "line"); + expect(line).toBe(JSON.stringify(response)); + if (clientName === "codex_security_deep_scan" && includeLayers) { + expect(await readFile(exited, "utf8")).toBe("exited"); + child.stdin.end(); + child.kill("SIGTERM"); + } else { + await expect(readFile(exited, "utf8")).rejects.toHaveProperty( + "code", + "ENOENT", + ); + child.stdin.end(); + } + await closed; + expect(await readFile(exited, "utf8")).toBe("exited"); + } finally { + child.stdin.end(); + await closed; + clearTimeout(timeout); + lines.close(); + child.stderr.destroy(); + await rm(root, { recursive: true, force: true }); + } + }, +); + test.each([0, 23])( "native configuration fixture preserves early exit %p with its input still open", async (exitCode) => { diff --git a/sdk/typescript/tests-ts/container-ci-workflow.test.ts b/sdk/typescript/tests-ts/container-ci-workflow.test.ts index 31330b63b8..4fe3d3d3f2 100644 --- a/sdk/typescript/tests-ts/container-ci-workflow.test.ts +++ b/sdk/typescript/tests-ts/container-ci-workflow.test.ts @@ -94,6 +94,7 @@ test.each([ ["pull_request", false, ".github/workflows/node-ci.yml", true], ["pull_request", false, ".github/actions/setup-tools/action.yml", true], ["pull_request", false, "sdk/typescript/src/index.ts", true], + ["pull_request", false, "sdk/typescript/scripts/run-ci-tests.mts", true], [ "pull_request", false, diff --git a/sdk/typescript/tests-ts/fixtures/skill-native-config.mjs b/sdk/typescript/tests-ts/fixtures/skill-native-config.mjs index 39b478635d..c22736c97e 100644 --- a/sdk/typescript/tests-ts/fixtures/skill-native-config.mjs +++ b/sdk/typescript/tests-ts/fixtures/skill-native-config.mjs @@ -10,11 +10,16 @@ const child = spawn(process.env.SYNTHETIC_NATIVE_CODEX, process.argv.slice(2), { }); child.stderr.pipe(process.stderr); const input = createInterface({ input: process.stdin }); -child.once("close", (code) => { - process.exitCode = code ?? 1; - input.close(); - process.stdin.destroy(); +const childClosed = new Promise((resolve) => { + child.once("close", (code) => { + process.exitCode = code ?? 1; + input.close(); + process.stdin.destroy(); + resolve(); + }); }); +let preflightClient = false; +let preflightConfigId; let turnRequest; let modelProvider; let initialConfig; @@ -24,6 +29,15 @@ await Promise.all([ for await (const line of input) { const request = JSON.parse(line); if (request.id !== undefined) methods.set(request.id, request.method); + if (request.method === "initialize") + preflightClient = + request.params.clientInfo.name === "codex_security_deep_scan"; + if ( + preflightClient && + request.method === "config/read" && + request.params.includeLayers === true + ) + preflightConfigId = request.id; if (request.method === "thread/start") { daybreakEnabled = request.params.daybreakEnabled ?? null; } @@ -45,8 +59,19 @@ await Promise.all([ child.stdin.end(); })(), (async () => { + let preflightResponse; for await (const line of createInterface({ input: child.stdout })) { const response = JSON.parse(line); + if ( + preflightConfigId !== undefined && + response.id === preflightConfigId + ) { + // The one-shot origins reader terminates this proxy after the response. + // Close its native child first, including on Windows where SIGTERM is forced. + preflightResponse = line; + child.stdin.end(); + continue; + } if (methods.get(response.id) === "thread/start") modelProvider = response.result?.modelProvider; if (methods.get(response.id) === "config/read") @@ -77,5 +102,10 @@ await Promise.all([ threadId, ); } + if (preflightResponse !== undefined) { + await childClosed; + process.stdout.write(preflightResponse + "\n"); + } })(), + childClosed, ]); diff --git a/sdk/typescript/tests-ts/skeleton.test.ts b/sdk/typescript/tests-ts/skeleton.test.ts index f44deb4731..8e79afa8db 100644 --- a/sdk/typescript/tests-ts/skeleton.test.ts +++ b/sdk/typescript/tests-ts/skeleton.test.ts @@ -500,15 +500,6 @@ describe("TypeScript package skeleton", () => { "false", ); expect(quality.env?.["CODEX_SECURITY_INTEGRATION"]).toBe("0"); - for (let shard = 1; shard <= 7; shard += 1) { - expect( - quality.jobs["runner"]?.strategy?.matrix["include"], - ).toContainEqual({ - os: "windows-latest", - mode: `shard-${shard}`, - args: `--shard=${shard}/7`, - }); - } }); test("keeps runner modes reproducible and report uploads rerunnable", async () => { @@ -519,36 +510,64 @@ describe("TypeScript package skeleton", () => { "${{ github.event_name == 'pull_request' && 1 || github.run_number }}"; expect(quality.env).not.toHaveProperty("CODEX_SECURITY_PROPERTY_SEED"); expect(runner.env?.["CODEX_SECURITY_PROPERTY_SEED"]).toBe(seed); - expect(runner.strategy?.matrix["mode"]).toEqual([ - "baseline", - "isolated", - "parallel", - ]); - for (const [mode, args] of [ - ["baseline", ""], - ["isolated", "--isolate"], - ["parallel", "--parallel=2"], + const unixMatrix = ci.jobs["test"]!.strategy!.matrix; + const unixShards = unixMatrix["shard"] as number[]; + const windowsShards = ci.jobs["windows-test"]!.strategy!.matrix[ + "shard" + ] as number[]; + expect(runner.strategy?.matrix).toEqual({ + target: [ + ...(unixMatrix["os"] as string[]).flatMap((os) => + unixShards.map((shard) => ({ os, shard, count: unixShards.length })), + ), + ...windowsShards.map((shard) => ({ + os: "windows-latest", + shard, + count: windowsShards.length, + })), + ], + mode: [ + { name: "baseline", args: "" }, + { name: "isolated", args: "--isolate" }, + { name: "parallel", args: "--parallel=2" }, + ], + }); + for (const [job, version] of [ + [runner, "1.4.3"], + [ci.jobs["test"]!, "1.3.14"], + [ci.jobs["windows-test"]!, "1.3.14"], + [quality.jobs["mutation"]!, "1.3.14"], ] as const) { - expect(runner.strategy?.matrix["include"]).toContainEqual({ - mode, - args, - }); - } - for (let shard = 1; shard <= 7; shard += 1) { - expect(runner.strategy?.matrix["include"]).toContainEqual({ - os: "windows-latest", - mode: `isolated-${shard}`, - args: `--isolate --shard=${shard}/7`, - }); + expect( + job + .steps!.filter((step) => step.uses?.startsWith("oven-sh/setup-bun@")) + .map((step) => step.with?.["bun-version"]), + ).toEqual([version]); } - const command = runner.steps!.find( + const execution = runner.steps!.find( (step) => step.name === "Test runner mode", - )?.run; - expect(command).toContain( - "${{ runner.os == 'Windows' && '--timeout=120000' || '' }}", ); - expect(command).toContain("${{ matrix.args }}"); - expect(command).toContain("--seed=${{ env.CODEX_SECURITY_PROPERTY_SEED }}"); + const ripgrep = runner.steps!.find( + (step) => step.name === "Install ripgrep", + ); + expect(ripgrep).toMatchObject({ + if: "runner.os == 'Linux'", + run: ci.jobs["test"]!.steps!.find( + (step) => step.name === "Install ripgrep", + )?.run, + }); + expect(runner.steps!.indexOf(ripgrep!)).toBeLessThan( + runner.steps!.indexOf(execution!), + ); + expect(execution).toMatchObject({ + "working-directory": "sdk/typescript", + }); + expect(execution?.run).toContain( + "scripts/run-ci-tests.mts ${{ matrix.target.shard }}/${{ matrix.target.count }} ${{ matrix.mode.args }}", + ); + expect(execution?.run).toContain( + "--seed=${{ env.CODEX_SECURITY_PROPERTY_SEED }}", + ); const uploads = [...Object.values(ci.jobs), ...Object.values(quality.jobs)] .flatMap((job) => job.steps ?? []) @@ -559,9 +578,13 @@ describe("TypeScript package skeleton", () => { expect( uploads.find((step) => step.name === "Upload mutation report"), ).toMatchObject({ "continue-on-error": true }); - expect( - uploads.find((step) => step.name === "Upload runner report"), - ).not.toHaveProperty("continue-on-error"); + const runnerUpload = uploads.find( + (step) => step.name === "Upload runner report", + ); + expect(runnerUpload).not.toHaveProperty("continue-on-error"); + expect(runnerUpload?.with?.["name"]).toBe( + "runner-${{ matrix.target.os }}-${{ matrix.mode.name }}-${{ matrix.target.shard }}", + ); expect( quality.jobs["mutation"]?.steps!.find( (step) => step.name === "Run mutation trial", diff --git a/sdk/typescript/tests-ts/test-reports.test.ts b/sdk/typescript/tests-ts/test-reports.test.ts index 341858145d..74756174b2 100644 --- a/sdk/typescript/tests-ts/test-reports.test.ts +++ b/sdk/typescript/tests-ts/test-reports.test.ts @@ -1,4 +1,4 @@ -import { mkdir, readFile, writeFile } from "node:fs/promises"; +import { mkdir, readFile, rm, writeFile } from "node:fs/promises"; import { join } from "node:path"; import { afterEach, describe, expect, test } from "bun:test"; import { bashCommand, readSubprocess } from "./support/shell.js"; @@ -32,6 +32,43 @@ async function fixtures() { }; } +async function workflowFixtures() { + const fixture = await fixtures(); + const workflow = Bun.YAML.parse( + await readFile( + new URL("../../../.github/workflows/test-quality.yml", import.meta.url), + "utf8", + ), + ) as { + jobs: { + runner: { + strategy: { + matrix: { + target: Array<{ os: string; shard: number }>; + mode: Array<{ name: string }>; + }; + }; + }; + compare: { steps: Array<{ name?: string; run?: string }> }; + }; + }; + await mkdir(join(fixture.root, "reports")); + const { target, mode } = workflow.jobs.runner.strategy.matrix; + for (const { os, shard } of target) { + for (const { name } of mode) { + await fixture.report(`reports/runner-${os}-${name}-${shard}.xml`, [ + testcase(`${os} shard ${shard}`), + ]); + } + } + return { + ...fixture, + script: workflow.jobs.compare.steps.find( + (step) => step.name === "Compare inventories and outcomes", + )!.run!, + }; +} + function testcase(name: string, status = "") { return `${status}`; } @@ -54,29 +91,21 @@ async function compare(baseline: string, ...candidates: string[]) { describe("JUnit inventory comparison", () => { test("runs every workflow comparison before reporting a mismatch", async () => { - const fixture = await fixtures(); - const workflow = Bun.YAML.parse( - await readFile( - new URL("../../../.github/workflows/test-quality.yml", import.meta.url), - "utf8", - ), - ) as { - jobs: { compare: { steps: Array<{ name?: string; run?: string }> } }; - }; - const script = workflow.jobs.compare.steps.find( - (step) => step.name === "Compare inventories and outcomes", - )!.run!; + const fixture = await workflowFixtures(); + const script = fixture.script; const expected = [ - ...["ubuntu-latest", "windows-latest"].flatMap((os) => - ["isolated", "parallel"].map( - (mode) => `reports/runner-${os}-${mode}*.xml`, - ), + "ubuntu-latest", + "macos-latest", + "windows-latest", + ].flatMap((os) => + ["isolated", "parallel"].map( + (mode) => + `reports/runner-${os}-baseline-*.xml|reports/runner-${os}-${mode}-*.xml`, ), - "reports/runner-windows-latest-shard-*.xml", - ]; + ); const mock = `node() { - printf '%s\\n' "$4" - [[ "$4" != "$CODEX_SECURITY_TEST_FAIL_REPORT" ]] + printf '%s\\n' "$3|$4" + [[ "$3|$4" != "$CODEX_SECURITY_TEST_FAIL_REPORT" ]] }`; const summary = join(fixture.root, "summary.md"); for (const failedReport of ["", expected[0]!]) { @@ -102,6 +131,71 @@ describe("JUnit inventory comparison", () => { } }); + for (const scenario of [ + { name: "missing Linux shard", os: "ubuntu-latest", missing: 3 }, + { name: "missing Windows shard", os: "windows-latest", missing: 7 }, + { + name: "missing shard replaced by an unexpected index", + os: "ubuntu-latest", + missing: 3, + extra: 4, + }, + { name: "extra shard", os: "windows-latest", extra: 8 }, + ]) { + test(`rejects identically incomplete workflow groups: ${scenario.name}`, async () => { + const fixture = await workflowFixtures(); + for (const mode of ["baseline", "isolated", "parallel"]) { + if (scenario.missing !== undefined) { + await rm( + join( + fixture.root, + `reports/runner-${scenario.os}-${mode}-${scenario.missing}.xml`, + ), + ); + } + if (scenario.extra !== undefined) { + await fixture.report( + `reports/runner-${scenario.os}-${mode}-${scenario.extra}.xml`, + [testcase("unexpected shard")], + ); + } + } + const child = Bun.spawn({ + cmd: [ + bash, + "-e", + "-o", + "pipefail", + "-c", + `node() { printf '%s\\n' "$3|$4"; }\n${fixture.script}`, + ], + cwd: fixture.root, + stdin: "ignore", + stdout: "ignore", + stderr: "pipe", + env: { ...process.env, GITHUB_STEP_SUMMARY: "summary.md" }, + timeout: 10_000, + }); + const { status, stderr } = await readSubprocess(child); + expect(status, stderr).toBe(1); + if (scenario.missing !== undefined) { + expect(stderr).toContain( + `Missing shard report: reports/runner-${scenario.os}-baseline-${scenario.missing}.xml`, + ); + } else { + expect(stderr).toContain( + `Expected 7 reports for ${scenario.os}/baseline, found 8`, + ); + } + // Missing artifacts must not prevent the other OS/mode diagnostics. + expect( + (await readFile(join(fixture.root, "summary.md"), "utf8")) + .trim() + .split(/\r?\n/u), + ).toHaveLength(6); + }); + } + test("merges native shards without depending on test order", async () => { const fixture = await fixtures(); const passed = testcase("accepts & preserves"); @@ -257,27 +351,68 @@ ${testcase("a & " ' < >")} expect(result.stdout).toContain("| defaults.xml | 1 | 0 | 0.00 |"); }); - test("rejects ambiguous test identities even when totals match", async () => { + test("preserves repeated parameterized identities and their outcome counts", async () => { const fixture = await fixtures(); - const first = testcase("same parameterized name"); - for (const [name, repeated] of [ - ["same-outcome", first], - ["different-outcome", testcase("same parameterized name", "")], + const passed = testcase("same parameterized name"); + const skipped = testcase("same parameterized name", ""); + const baseline = await fixture.report("baseline.xml", [ + passed, + passed, + skipped, + ]); + const candidate = await fixture.report("candidate.xml", [ + skipped, + passed, + passed, + ]); + const result = await compare(baseline, candidate); + expect(result.status, result.stderr).toBe(0); + expect(result.stdout).toContain("| baseline.xml | 3 | 1 | 1.25 |"); + for (const [name, cases, diagnostic] of [ + ["missing", [passed, skipped], "Missing (1)"], + ["extra", [passed, passed, passed, skipped], "Extra (1)"], + ["changed", [passed, skipped, skipped], "Missing (1)"], ] as const) { - const baseline = await fixture.report(`${name}-baseline.xml`, [ - first, - repeated, - ]); - const candidate = await fixture.report(`${name}-candidate.xml`, [ - first, - repeated, - ]); - const result = await compare(baseline, candidate); - expect(result.status).toBe(1); - expect(result.stderr).toContain("duplicate test identity"); + const changed = await fixture.report(`${name}.xml`, [...cases]); + const comparison = await compare(baseline, changed); + expect(comparison.status, name).toBe(1); + expect(comparison.stderr).toContain(diagnostic); } }); + test("compares complete baseline and candidate shard groups", async () => { + const fixture = await fixtures(); + const first = testcase("first"); + const repeated = testcase("parameterized name"); + await fixture.report("baseline-1.xml", [first, repeated]); + await fixture.report("baseline-2.xml", [repeated]); + await fixture.report("candidate-1.xml", [repeated, repeated]); + await fixture.report("candidate-2.xml", [first]); + const baseline = join(fixture.root, "baseline-*.xml"); + const candidate = join(fixture.root, "candidate-*.xml"); + const result = await compare(baseline, candidate); + expect(result.status, result.stderr).toBe(0); + expect(result.stdout).toContain( + "Baseline shards: slowest 1.25s; combined test time: 2.50s.", + ); + expect(result.stdout).toContain( + "Slowest candidate: 1.25s; combined test time: 2.50s.", + ); + const extra = await fixture.report("extra.xml", [repeated]); + const duplicated = await compare(baseline, candidate, extra); + expect(duplicated.status).toBe(1); + expect(duplicated.stderr).toContain("Extra (1)"); + const missing = await compare( + baseline, + join(fixture.root, "candidate-1.xml"), + ); + expect(missing.status).toBe(1); + expect(missing.stderr).toContain("Missing (1)"); + expect( + (await compare(join(fixture.root, "absent-*.xml"), candidate)).status, + ).toBe(1); + }); + test("reports every shard's timing even when an earlier shard fails", async () => { const fixture = await fixtures(); const first = testcase("first"); diff --git a/sdk/typescript/tests-ts/workflow-quality.test.ts b/sdk/typescript/tests-ts/workflow-quality.test.ts index 568228d482..16cc320629 100644 --- a/sdk/typescript/tests-ts/workflow-quality.test.ts +++ b/sdk/typescript/tests-ts/workflow-quality.test.ts @@ -193,16 +193,22 @@ bashTest( ); bashTest( - "selects workflow checks for workflow changes and main pushes", + "selects workflow and runner checks for relevant changes and main pushes", async () => { const scope = ci.jobs["classify-changes"]!.steps.find( (step) => step.name === "Select additional checks", )!; const directory = await mkdtemp(join(tmpdir(), "workflow-quality-")); try { - for (const [event, baseChanged, paths, expected] of [ + for (const [ + event, + baseChanged, + paths, + expected, + runnerExpected = "false", + ] of [ ["push", false, [], "true"], - ["pull_request", true, [], "true"], + ["pull_request", true, [], "true", "true"], ["pull_request", false, [".github/workflows/node-release.yml"], "true"], [ "pull_request", @@ -233,6 +239,37 @@ bashTest( ["pull_request", false, ["docker/verify-container-compose.sh"], "true"], ["pull_request", false, ["sdk/typescript/src/index.ts"], "false"], ["pull_request", false, ["README.md"], "false"], + [ + "pull_request", + false, + [".github/workflows/node-ci.yml"], + "true", + "true", + ], + [ + "pull_request", + false, + [".github/workflows/test-quality.yml"], + "true", + "true", + ], + [ + "pull_request", + false, + [".github/actions/setup-tools/action.yml"], + "true", + "true", + ], + ...[ + "sdk/typescript/bunfig.toml", + "sdk/typescript/scripts/compare-test-reports.mts", + "sdk/typescript/scripts/prepare-windows-test-root.ps1", + "sdk/typescript/scripts/run-ci-tests.mts", + "sdk/typescript/scripts/test-shards.mts", + "sdk/typescript/scripts/test-timings.json", + ].map( + (path) => ["pull_request", false, [path], "false", "true"] as const, + ), ] as const) { const output = join(directory, "output"); await rm(output, { force: true }); @@ -263,6 +300,7 @@ bashTest( .map((line) => line.split("=")), ); expect(outputs["workflow-quality"], paths.join(",")).toBe(expected); + expect(outputs["test-quality"], paths.join(",")).toBe(runnerExpected); } } finally { await rm(directory, { recursive: true, force: true });