From 22f56f4431426e093970c6f019dbe5f6333e935e Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Sun, 14 Jun 2026 12:25:29 -0400 Subject: [PATCH] ci(#1212): shard windows full-test lane to remove timeout cliff (#1222) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `full test (windows-latest, *)` lane ran the entire unit suite (~740+ files) in one job whose wall-clock crept against the 20m cap and intermittently CANCELLED (false-negative gate, observed on PR #1207). Prior tactical fixes #869 (15→20m bump) and #1051 (handle-leak) deferred the cliff structurally. Shard the unit suite across 3 parallel runners per OS/node leg so per-job wall-clock is O(total/3) and stays under the cap as the suite grows. - scripts/run-tests.cjs: add `--shard /` — a deterministic, balanced round-robin partition (fileIndex % n === i-1) over the SORTED selected file list. parseShardArg strictly validates i∈1..n, n≥1, integer-only; n=1 is a pure no-op. The 28K Windows argv chunking is preserved within each shard. A legitimately-empty shard (n > file count) exits 0; a selection empty BEFORE sharding still hits the discovery hard error. Composes with --suite and is order-independent (sorted before partition). Exports selectShard/parseShardArg. - .github/workflows/test.yml: test-full becomes the 3 legs × 3 shards = 9-job cross-product (explicit include rows — a base shard dim does not cross-product with include legs, and a nested matrix.leg.os is unresolvable by the H1 shell-policy linter). Unit suite runs sharded; integration/security run once per leg (shard 1). The Required tests fan-in is unchanged: it already needs test-full and checks the matrix-aggregate result, so a failed/cancelled shard fails the gate; the branch-protection check name is preserved. - tests: partition/CLI + pure selectShard contract (completeness, disjointness, balance, determinism, boundaries, fast-check property) + parseShardArg validation, in run-tests-harness.test.cjs; a DEFECT.GENERATIVE-FIX parity guard (per-row shard values 1..N, every leg runs all shards, N == --shard /N denominator) + Required-tests name/needs pin, in ci-test-scope.test.cjs. Closes #1212 Co-authored-by: Claude Opus 4.8 (1M context) --- .github/workflows/test.yml | 62 +++++- scripts/run-tests.cjs | 113 ++++++++++- tests/ci-test-scope.test.cjs | 90 +++++++++ tests/run-tests-harness.test.cjs | 314 +++++++++++++++++++++++++++++++ 4 files changed, 567 insertions(+), 12 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 146bbf679..24d44cbde 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -281,16 +281,28 @@ jobs: run: node scripts/run-tests.cjs --files-from .ci-selected-tests.txt test-full: - name: full test (${{ matrix.os }}, ${{ matrix.node-version }}) + name: full test (${{ matrix.os }}, ${{ matrix.node-version }}, shard ${{ matrix.shard }}/3) needs: changes if: needs.changes.outputs.code_changed == 'true' && needs.changes.outputs.full_matrix == 'true' runs-on: ${{ matrix.os }} defaults: run: shell: ${{ matrix.shell }} - # 20m, not 15m: the Windows full lane runs all 656 unit files in one job and - # has crept to ~14m+, so 15m started cancelling jobs mid-run (#869). Durable - # fix is to shard this lane — tracked separately. + # The unit suite is sharded across 3 parallel runners per OS/node leg + # (#1212). Each shard runs a deterministic round-robin third of the sorted + # unit-file list via `run-tests.cjs --suite unit --shard i/3`, so per-job + # wall-clock scales as O(total/3) and stays well under the cap as the suite + # grows — replacing the #869 timeout bump (15→20m) which only deferred the + # cliff. The cap stays at 20m as a generous backstop; a healthy shard now + # finishes in roughly a third of the old single-lane wall-clock. + # + # The matrix is the cross-product of 3 OS/node legs × 3 shards = 9 jobs, + # enumerated explicitly as `include:` rows. (A base `shard: [1,2,3]` + # dimension would NOT cross-product against `include` legs — include rows + # sharing no key with the base matrix are appended as standalone combos — + # and a NESTED `leg.os` key is not resolvable by the H1 shell-policy linter + # in scripts/workflow-policy.cjs, which reads `matrix.os`/`matrix.shell` + # directly. Explicit rows keep both the cross-product and the linter happy.) timeout-minutes: 20 env: GSD_PLUGIN_ROOT: .ci-gsd-plugin-root-disabled @@ -301,12 +313,39 @@ jobs: - os: windows-latest node-version: 22 shell: pwsh + shard: 1 + - os: windows-latest + node-version: 22 + shell: pwsh + shard: 2 + - os: windows-latest + node-version: 22 + shell: pwsh + shard: 3 - os: macos-latest node-version: 22 shell: 'zsh {0}' + shard: 1 + - os: macos-latest + node-version: 22 + shell: 'zsh {0}' + shard: 2 + - os: macos-latest + node-version: 22 + shell: 'zsh {0}' + shard: 3 - os: macos-latest node-version: 24 shell: 'zsh {0}' + shard: 1 + - os: macos-latest + node-version: 24 + shell: 'zsh {0}' + shard: 2 + - os: macos-latest + node-version: 24 + shell: 'zsh {0}' + shard: 3 steps: - uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd # v5.0.1 (Windows) @@ -347,13 +386,24 @@ jobs: - name: Dependency integrity gate run: node scripts/check-npm-integrity.cjs - - name: Run unit tests - run: npm run test:unit + # The heavy unit suite is split across the 3 shards — each runs a + # deterministic round-robin third of the sorted unit-file list. The + # union of shards 1/3 + 2/3 + 3/3 is the full unit suite, so coverage is + # unchanged; only wall-clock per job drops to ~total/3. + - name: Run unit tests (shard ${{ matrix.shard }}/3) + run: node scripts/run-tests.cjs --suite unit --shard ${{ matrix.shard }}/3 + # Integration and security suites are small; run them once per OS/node + # leg (on shard 1 only) instead of redundantly on all three shards. They + # still run on every leg (3 times total, once per platform), so each + # platform's integration/security coverage is unchanged — only the + # 3x-per-leg duplication is removed. - name: Run integration tests + if: matrix.shard == 1 run: npm run test:integration - name: Run security tests + if: matrix.shard == 1 run: npm run test:security required-tests: diff --git a/scripts/run-tests.cjs b/scripts/run-tests.cjs index 19ab22cd3..c447779eb 100644 --- a/scripts/run-tests.cjs +++ b/scripts/run-tests.cjs @@ -13,6 +13,15 @@ // node scripts/run-tests.cjs --suite slow # *.slow.test.cjs // node scripts/run-tests.cjs --files "a.test.cjs b.test.cjs" // node scripts/run-tests.cjs --files-from /tmp/selected-tests.txt +// node scripts/run-tests.cjs --suite unit --shard 1/3 # shard 1 of 3 (#1212) +// +// Sharding (issue #1212): --shard / runs a deterministic, balanced +// round-robin slice of the SORTED selected file list (file index k → shard +// k % n). i is 1-based (1..n); n >= 1; n=1 is a pure no-op (all files). The +// CI windows full-test lane shards across N parallel runners so per-job +// wall-clock scales as O(total/N) and stops hitting the job time cap. Sharding +// composes with --suite (it slices the post-filter selection) and preserves +// the existing 28K argv chunking WITHIN each shard. // // Suite grouping convention: filename suffix marker before `.test.cjs`. // A file named `foo.security.test.cjs` belongs to the `security` suite. @@ -127,14 +136,75 @@ function walkTestFiles(dir, relBase) { return results; } +// Parse a `--shard i/n` value into { index, total } or { error }. +// i is 1-based and must satisfy 1 <= i <= n; n must be >= 1. Both parts must be +// plain non-negative integers (no decimals, signs, or surrounding whitespace). +// `n=1` is the pure no-op (every file). This is the strict-input boundary +// (Postel's Law: be strict in what a CLI flag accepts so a typo fails loudly +// rather than silently running the wrong slice of the suite). +function parseShardArg(value) { + if (typeof value !== 'string') { + return { error: `--shard requires a value of the form i/n` }; + } + const m = /^(\d+)\/(\d+)$/.exec(value); + if (!m) { + return { error: `--shard value "${value}" must be of the form i/n (e.g. 1/3)` }; + } + const index = Number(m[1]); + const total = Number(m[2]); + if (!Number.isInteger(total) || total < 1) { + return { error: `--shard total n must be an integer >= 1, got "${m[2]}"` }; + } + if (!Number.isInteger(index) || index < 1 || index > total) { + return { error: `--shard index i must be an integer in 1..${total}, got "${m[1]}"` }; + } + return { index, total }; +} + +// Deterministic, balanced round-robin partition of an ALREADY-SORTED file list. +// Shard `index` (1-based) receives every file whose position k in the sorted +// list satisfies k % total === index - 1. Round-robin (not contiguous blocks) +// spreads duration variance across shards and guarantees shard sizes differ by +// at most 1. Selection keys off array INDEX, never off the path string, so the +// partition is byte-identical across Windows/macOS/Linux as long as the caller +// sorts the list with the same (locale-independent) comparator. `total=1` +// returns the input unchanged (pure no-op). A shard with no files (total > +// file count) returns [] and is a legitimate result, not an error. +function selectShard(sortedFiles, { index, total }) { + if (total === 1) return sortedFiles; + return sortedFiles.filter((_, k) => k % total === index - 1); +} + function parseArgs(argv) { let suite = null; let seen = false; let files = null; let filesFrom = null; + let shard = null; + let shardSeen = false; for (let i = 0; i < argv.length; i++) { const a = argv[i]; - if (a === '--suite') { + if (a === '--shard' || a.startsWith('--shard=')) { + if (shardSeen) { + return { error: 'duplicate --shard flag' }; + } + shardSeen = true; + let v; + if (a === '--shard') { + v = argv[i + 1]; + if (v === undefined || (typeof v === 'string' && v.startsWith('--'))) { + return { error: '--shard requires a value of the form i/n' }; + } + i++; + } else { + v = a.slice('--shard='.length); + } + const parsed = parseShardArg(v); + if (parsed.error) { + return { error: parsed.error }; + } + shard = parsed; + } else if (a === '--suite') { if (seen) { return { error: 'duplicate --suite flag' }; } @@ -197,7 +267,7 @@ function parseArgs(argv) { if (files !== null && filesFrom !== null) { return { error: '--files and --files-from cannot be combined' }; } - return { suite, files, filesFrom }; + return { suite, files, filesFrom, shard }; } // Return the marked suite name embedded in a filename, or null if it's unmarked. @@ -331,12 +401,43 @@ function main() { } else { selectedNames = selectFiles(allFiles, suite); } + + // Shard partitioning (#1212): when --shard i/n is given, keep only this + // shard's deterministic round-robin slice of the selected list. Applied + // AFTER suite/explicit selection so it composes with --suite (each shard + // runs i/n of the post-filter selection). + // + // The partition keys off array index, so the slice is only reproducible if + // the input is in a stable order. --suite/default selections are already + // sorted (allFiles came from walkTestFiles(...).sort() and selectFiles + // preserves that order), but --files/--files-from preserve REQUEST order. + // Sort here so --shard is deterministic regardless of how the selection was + // produced — the runner's documented contract is a sorted partition. + // + // emptyBeforeShard distinguishes "this shard legitimately got zero files + // from a non-empty list" (total > file count — a valid no-op) from "the + // selection was already empty before sharding" (a genuinely empty suite, + // which must still hit the discovery hard-error below — Codex #1212 review). + const usingShard = parsed.shard !== null; + let emptyBeforeShard = false; + if (usingShard) { + emptyBeforeShard = selectedNames.length === 0; + selectedNames = selectShard([...selectedNames].sort(), parsed.shard); + } + const selected = selectedNames.map(f => join(testDir, f)); if (selected.length === 0) { - if (usingExplicitFiles) { - // Empty file list from --files/--files-from: allowed (e.g. CI passes an - // empty .ci-selected-tests.txt on docs-only/inert PRs). Exit 0 silently. + // A legitimately-empty shard: --shard was given, the pre-shard selection + // had files, but this shard index drew zero (total > file count). Exit 0. + const legitimatelyEmptyShard = usingShard && !emptyBeforeShard; + if (usingExplicitFiles || legitimatelyEmptyShard) { + // Empty file list from --files/--files-from (e.g. CI passes an empty + // .ci-selected-tests.txt on docs-only/inert PRs) OR a legitimately-empty + // shard: both are expected. Exit 0 silently rather than taking the + // "discovery broken" hard-error path below. An EMPTY suite that was + // empty BEFORE sharding falls through to the hard error so a broken + // --suite filter is still caught even with --shard present. console.error(`run-tests: no tests in suite "${suite || 'all'}"`); return 0; } @@ -484,4 +585,4 @@ if (require.main === module) { runMain(main); } -module.exports = { suiteOf, ensureBuiltArtifacts }; +module.exports = { suiteOf, ensureBuiltArtifacts, parseShardArg, selectShard }; diff --git a/tests/ci-test-scope.test.cjs b/tests/ci-test-scope.test.cjs index 59c77a831..47e8eb803 100644 --- a/tests/ci-test-scope.test.cjs +++ b/tests/ci-test-scope.test.cjs @@ -457,6 +457,96 @@ describe('test.yml changes job contract (#837)', () => { }); }); +describe('test-full shard matrix parity (#1212)', () => { + // DEFECT.GENERATIVE-FIX: the sharded windows full-test lane has TWO surfaces + // that must agree — the `shard:` matrix array (how many parallel jobs run) + // and the `/N` denominator in `run-tests.cjs --suite unit --shard i/N` (how + // many slices the runner partitions the suite into). If they diverge (e.g. + // someone grows `shard: [1,2,3,4]` but leaves `--shard ${{ matrix.shard }}/3`), + // shards silently overlap and one shard errors out. This parity assertion + // fails the moment the two drift. + const yaml = require('js-yaml'); + + function loadTestFull() { + const text = fs.readFileSync(path.join(WORKFLOWS_DIR, 'test.yml'), 'utf8'); + const doc = yaml.load(text); + return { text, job: doc.jobs['test-full'] }; + } + + test('distinct shard values are 1..N matching the --shard /N denominator, on every leg', () => { + const { job } = loadTestFull(); + const include = job.strategy.matrix.include; + assert.ok(Array.isArray(include), 'test-full matrix must enumerate `include:` rows'); + assert.ok( + include.every(r => Number.isInteger(r.shard)), + 'every include row must carry an integer `shard:` key', + ); + + const distinctShards = [...new Set(include.map(r => r.shard))].sort((a, b) => a - b); + const n = distinctShards.length; + + // Distinct shard values must be exactly 1..n (1-based, contiguous) so the + // runner's round-robin selection covers every file with no gaps/overlaps. + assert.deepStrictEqual( + distinctShards, + Array.from({ length: n }, (_, i) => i + 1), + `distinct shard values must be 1..${n} (1-based, contiguous), got ${JSON.stringify(distinctShards)}`, + ); + + // Every OS/node leg must appear once per shard (full cross-product) — no + // leg may silently skip a shard, which would drop a third of its coverage. + const legs = [...new Set(include.map(r => `${r.os}|${r['node-version']}`))]; + for (const leg of legs) { + const [os, node] = leg.split('|'); + const shardsForLeg = include + .filter(r => r.os === os && String(r['node-version']) === node) + .map(r => r.shard) + .sort((a, b) => a - b); + assert.deepStrictEqual( + shardsForLeg, + distinctShards, + `leg ${leg} must run all shards ${JSON.stringify(distinctShards)}, got ${JSON.stringify(shardsForLeg)}`, + ); + } + // Full cross-product: every (leg, shard) pair is present exactly once, so + // the row count equals legs × shards with no duplicate/missing combination. + const pairKey = r => `${r.os}|${r['node-version']}|${r.shard}`; + assert.strictEqual(new Set(include.map(pairKey)).size, legs.length * n); + assert.strictEqual(include.length, legs.length * n); + + // Find the `--shard ${{ matrix.shard }}/` denominator in the unit step. + const unitStep = job.steps.find( + s => typeof s.run === 'string' && s.run.includes('run-tests.cjs') && s.run.includes('--shard'), + ); + assert.ok(unitStep, 'test-full must have a step running run-tests.cjs --shard'); + const m = /--shard\s+\$\{\{\s*matrix\.shard\s*\}\}\/(\d+)/.exec(unitStep.run); + assert.ok(m, `could not parse --shard i/N denominator from: ${unitStep.run}`); + const denominator = Number(m[1]); + + assert.strictEqual( + denominator, + n, + `shard count (${n}) and --shard /N denominator (${denominator}) must match — ` + + `update both the per-row \`shard:\` values and the \`/N\` in the run command together.`, + ); + }); + + test('required-tests fan-in still needs test-full and keeps the protected name', () => { + // Hyrum's Law: branch protection requires a status check literally named + // "Required tests". Renaming it (or dropping test-full from its needs) + // would silently break the gate. Pin both. + const text = fs.readFileSync(path.join(WORKFLOWS_DIR, 'test.yml'), 'utf8'); + const doc = yaml.load(text); + const fanIn = doc.jobs['required-tests']; + assert.ok(fanIn, 'required-tests job must exist'); + assert.strictEqual(fanIn.name, 'Required tests', 'the branch-protection check name must stay "Required tests"'); + assert.ok( + Array.isArray(fanIn.needs) && fanIn.needs.includes('test-full'), + 'required-tests must `needs: test-full` so all shard legs aggregate into the gate', + ); + }); +}); + describe('code_changed=false implies clean output invariant', () => { // Fix 1: when code_changed is false, full_matrix, targeted_tests, windows_tests // must ALL be empty/false — even if a docs path coincidentally diff --git a/tests/run-tests-harness.test.cjs b/tests/run-tests-harness.test.cjs index e5501c498..1f3750a1c 100644 --- a/tests/run-tests-harness.test.cjs +++ b/tests/run-tests-harness.test.cjs @@ -391,6 +391,180 @@ test('ambient GSD workstream vars are stripped by the runner', () => { }); }); + describe('shard partitioning CLI (#1212)', () => { + // The windows full-test lane is sharded across N parallel runners via a + // GitHub Actions matrix shard dimension; each shard runs + // `run-tests.cjs --suite unit --shard i/n`. Selection is a deterministic + // round-robin over the SORTED file list so duration variance spreads + // across shards. The CLI surface is validated here; the pure partition + // contract (completeness/disjointness/balance/determinism) is validated + // against the exported selectShard() in the separate describe block below. + + // 9 files, sorted: shard-00..shard-08. With n=3, round-robin gives + // shard 1 -> {00,03,06}, shard 2 -> {01,04,07}, shard 3 -> {02,05,08}. + const SHARD_NAMES = Array.from( + { length: 9 }, + (_, i) => `shard-${String(i).padStart(2, '0')}.test.cjs`, + ); + + test('--shard 1/3 runs a balanced round-robin slice of the sorted files', () => { + seed(tmpDir, SHARD_NAMES); + const r = runHarness(tmpDir, ['--shard', '1/3']); + assert.strictEqual(r.status, 0, `stderr: ${r.stderr}`); + // The harness echoes the selected basenames on its `files=N:` line. + assert.match(r.stderr, /files=3:/); + assert.match(r.stderr, /shard-00\.test\.cjs/); + assert.match(r.stderr, /shard-03\.test\.cjs/); + assert.match(r.stderr, /shard-06\.test\.cjs/); + // Files belonging to other shards must NOT appear in this shard's run. + assert.doesNotMatch(r.stderr, /shard-01\.test\.cjs/); + assert.doesNotMatch(r.stderr, /shard-02\.test\.cjs/); + }); + + test('--shard 2/3 selects the second round-robin slice', () => { + seed(tmpDir, SHARD_NAMES); + const r = runHarness(tmpDir, ['--shard', '2/3']); + assert.strictEqual(r.status, 0, `stderr: ${r.stderr}`); + assert.match(r.stderr, /files=3:/); + assert.match(r.stderr, /shard-01\.test\.cjs/); + assert.match(r.stderr, /shard-04\.test\.cjs/); + assert.match(r.stderr, /shard-07\.test\.cjs/); + assert.doesNotMatch(r.stderr, /shard-00\.test\.cjs/); + }); + + test('--shard composes with --suite (shards the post-filter selection)', () => { + // 6 unit files + 2 security files. `--suite unit --shard 1/2` must shard + // only the unit selection, never pulling in the security files. + seed(tmpDir, [ + 'u0.test.cjs', + 'u1.test.cjs', + 'u2.test.cjs', + 'u3.test.cjs', + 's0.security.test.cjs', + 's1.security.test.cjs', + ]); + const r = runHarness(tmpDir, ['--suite', 'unit', '--shard', '1/2']); + assert.strictEqual(r.status, 0, `stderr: ${r.stderr}`); + assert.doesNotMatch(r.stderr, /\.security\.test\.cjs/); + }); + + test('--shard 1/1 is a pure no-op (runs every file)', () => { + seed(tmpDir, SHARD_NAMES); + const r = runHarness(tmpDir, ['--shard', '1/1']); + assert.strictEqual(r.status, 0, `stderr: ${r.stderr}`); + assert.match(r.stderr, /files=9:/); + }); + + test('an empty shard (n > file count) exits 0 without crashing', () => { + // n=5 with only 2 files: shards 3,4,5 are legitimately empty. An empty + // shard must NOT take the "discovery is broken" hard-error path. + seed(tmpDir, ['only-a.test.cjs', 'only-b.test.cjs']); + const r = runHarness(tmpDir, ['--shard', '5/5']); + assert.strictEqual( + r.status, + 0, + `empty shard must exit 0; got status=${r.status} signal=${r.signal}\nSTDERR:\n${r.stderr}`, + ); + }); + + test('an empty suite BEFORE sharding still hits the discovery hard error', () => { + // Regression (Codex #1212 review): a genuinely empty selection + // (e.g. --suite security with zero security files) must NOT be masked + // by the empty-shard escape hatch. Only a shard that emptied a NON-empty + // list is a legitimate no-op; a pre-empty selection is a broken filter. + seed(tmpDir, ['a.test.cjs', 'b.test.cjs']); // unit files only, no security + const r = runHarness(tmpDir, ['--suite', 'security', '--shard', '1/3']); + assert.notStrictEqual( + r.status, + 0, + `empty-before-shard must fail; got status=${r.status}\nSTDERR:\n${r.stderr}`, + ); + assert.match(r.stderr, /0 test files selected|discovery/i); + }); + + test('--shard over --files is order-independent (sorted before partition)', () => { + // Regression (Codex #1212 review): the partition keys off array index, + // so the same file set passed in different --files order must produce + // the same per-shard assignment. The runner sorts the selection before + // sharding to guarantee this. + seed(tmpDir, ['x0.test.cjs', 'x1.test.cjs', 'x2.test.cjs', 'x3.test.cjs']); + const forward = runHarness(tmpDir, [ + '--files', 'x0.test.cjs x1.test.cjs x2.test.cjs x3.test.cjs', + '--shard', '1/2', + ]); + const reversed = runHarness(tmpDir, [ + '--files', 'x3.test.cjs x2.test.cjs x1.test.cjs x0.test.cjs', + '--shard', '1/2', + ]); + assert.strictEqual(forward.status, 0, `stderr: ${forward.stderr}`); + assert.strictEqual(reversed.status, 0, `stderr: ${reversed.stderr}`); + // Both runs select the SAME files (sorted shard 1/2 of x0..x3 = x0,x2). + const filesLine = (s) => (s.match(/files=\d+: (.*)$/m) || [])[1] || ''; + const a = filesLine(forward.stderr).split(' ').sort().join(' '); + const b = filesLine(reversed.stderr).split(' ').sort().join(' '); + assert.strictEqual(a, b, `order-dependent shard assignment:\nforward=${a}\nreversed=${b}`); + assert.match(a, /x0\.test\.cjs/); + assert.match(a, /x2\.test\.cjs/); + }); + + test('--shard rejects i outside 1..n', () => { + seed(tmpDir, SHARD_NAMES); + const r = runHarness(tmpDir, ['--shard', '0/3']); + assert.notStrictEqual(r.status, 0); + assert.match(r.stderr, /shard/i); + }); + + test('--shard rejects i greater than n', () => { + seed(tmpDir, SHARD_NAMES); + const r = runHarness(tmpDir, ['--shard', '4/3']); + assert.notStrictEqual(r.status, 0); + assert.match(r.stderr, /shard/i); + }); + + test('--shard rejects n < 1', () => { + seed(tmpDir, SHARD_NAMES); + const r = runHarness(tmpDir, ['--shard', '1/0']); + assert.notStrictEqual(r.status, 0); + assert.match(r.stderr, /shard/i); + }); + + test('--shard rejects malformed (no slash) value', () => { + seed(tmpDir, SHARD_NAMES); + const r = runHarness(tmpDir, ['--shard', '2']); + assert.notStrictEqual(r.status, 0); + assert.match(r.stderr, /shard/i); + }); + + test('--shard rejects non-integer parts', () => { + seed(tmpDir, SHARD_NAMES); + const r = runHarness(tmpDir, ['--shard', '1.5/3']); + assert.notStrictEqual(r.status, 0); + assert.match(r.stderr, /shard/i); + }); + + test('duplicate --shard flag is rejected', () => { + seed(tmpDir, SHARD_NAMES); + const r = runHarness(tmpDir, ['--shard', '1/3', '--shard', '2/3']); + assert.notStrictEqual(r.status, 0); + assert.match(r.stderr, /duplicate/i); + }); + + test('--shard chunking engages within a shard (argv-overflow preserved)', () => { + // Each shard must still chunk its own slice so a large shard cannot + // overflow the Windows 32,767-char command-line ceiling (#3597). + const longPrefix = 'a-deliberately-long-test-filename-to-force-chunking-within-a-shard-'; + const names = Array.from( + { length: 30 }, + (_, i) => `${longPrefix}${String(i).padStart(4, '0')}.test.cjs`, + ); + seed(tmpDir, names); + // n=2 -> each shard gets 15 files; a 1500-char ceiling forces chunking. + const r = runHarness(tmpDir, ['--shard', '1/2'], { RUN_TESTS_MAX_CMDLINE_CHARS: '1500' }); + assert.strictEqual(r.status, 0, `stderr (tail):\n${r.stderr.split('\n').slice(-20).join('\n')}`); + assert.match(r.stderr, /run-tests: chunk \d+\/\d+ — \d+ files/); + }); + }); + describe('per-chunk timeout + force-exit (windows hang guard, #1051)', () => { // A unit test that leaks an open handle (un-terminated Worker, un-killed // child_process, ref'd timer) causes node --test to hang ~150s after its @@ -447,3 +621,143 @@ setInterval(() => {}, 1 << 30); }); }); }); + +// Pure partition contract for the shard selector (#1212). Imported directly +// (no subprocess) because these are deterministic in-memory assertions about +// the round-robin partition — the cheapest, most precise way to pin +// completeness / disjointness / balance / determinism, including a fast-check +// property test (RULESET.TESTS.property-based-testing: partition is a +// bijective transformation contract). +const { parseShardArg, selectShard } = require('../scripts/run-tests.cjs'); + +describe('selectShard round-robin partition (#1212)', () => { + // A deterministic sorted file list; selectShard MUST NOT re-sort — the caller + // sorts once and the partition keys off array index so ordering is identical + // across Windows/macOS/Linux. + const files = Array.from({ length: 25 }, (_, i) => `f${String(i).padStart(3, '0')}.test.cjs`); + + test('n=1 returns the full list unchanged (pure no-op)', () => { + assert.deepStrictEqual(selectShard(files, { index: 1, total: 1 }), files); + }); + + test('completeness: the union of all shards equals the full list', () => { + const n = 4; + const union = []; + for (let i = 1; i <= n; i++) union.push(...selectShard(files, { index: i, total: n })); + assert.deepStrictEqual([...union].sort(), [...files].sort()); + }); + + test('disjointness: no file appears in two shards', () => { + const n = 4; + const seen = new Set(); + for (let i = 1; i <= n; i++) { + for (const f of selectShard(files, { index: i, total: n })) { + assert.ok(!seen.has(f), `file ${f} appeared in more than one shard`); + seen.add(f); + } + } + assert.strictEqual(seen.size, files.length); + }); + + test('balance: shard sizes differ by at most 1', () => { + const n = 4; + const sizes = []; + for (let i = 1; i <= n; i++) sizes.push(selectShard(files, { index: i, total: n }).length); + assert.ok(Math.max(...sizes) - Math.min(...sizes) <= 1, `sizes=${sizes}`); + }); + + test('determinism: same input yields the same partition', () => { + const a = selectShard(files, { index: 2, total: 3 }); + const b = selectShard(files, { index: 2, total: 3 }); + assert.deepStrictEqual(a, b); + }); + + test('round-robin: shard i gets indices i-1, i-1+n, i-1+2n, …', () => { + const n = 3; + assert.deepStrictEqual( + selectShard(files, { index: 1, total: n }), + files.filter((_, k) => k % n === 0), + ); + assert.deepStrictEqual( + selectShard(files, { index: 2, total: n }), + files.filter((_, k) => k % n === 1), + ); + assert.deepStrictEqual( + selectShard(files, { index: 3, total: n }), + files.filter((_, k) => k % n === 2), + ); + }); + + test('preserves relative order within a shard', () => { + const slice = selectShard(files, { index: 1, total: 3 }); + const sorted = [...slice].sort(); + assert.deepStrictEqual(slice, sorted); + }); + + test('empty shard when total > file count returns []', () => { + const two = ['a.test.cjs', 'b.test.cjs']; + assert.deepStrictEqual(selectShard(two, { index: 5, total: 5 }), []); + assert.deepStrictEqual(selectShard(two, { index: 3, total: 5 }), []); + // shards 1 and 2 still get the two files + assert.deepStrictEqual(selectShard(two, { index: 1, total: 5 }), ['a.test.cjs']); + assert.deepStrictEqual(selectShard(two, { index: 2, total: 5 }), ['b.test.cjs']); + }); + + test('boundary: total exactly equals file count → one file per shard', () => { + const three = ['a.test.cjs', 'b.test.cjs', 'c.test.cjs']; + for (let i = 1; i <= 3; i++) { + assert.strictEqual(selectShard(three, { index: i, total: 3 }).length, 1); + } + }); + + test('boundary: total = count-1 and count+1', () => { + const four = ['a.test.cjs', 'b.test.cjs', 'c.test.cjs', 'd.test.cjs']; + // count-1 = 3 shards over 4 files → sizes {2,1,1} + const sizes3 = [1, 2, 3].map(i => selectShard(four, { index: i, total: 3 }).length).sort(); + assert.deepStrictEqual(sizes3, [1, 1, 2]); + // count+1 = 5 shards over 4 files → one shard empty + const sizes5 = [1, 2, 3, 4, 5].map(i => selectShard(four, { index: i, total: 5 }).length).sort(); + assert.deepStrictEqual(sizes5, [0, 1, 1, 1, 1]); + }); + + test('property: partition is complete, disjoint, and balanced for any n,N', () => { + const fc = require('fast-check'); + fc.assert( + fc.property( + fc.array(fc.string(), { minLength: 0, maxLength: 50 }), + fc.integer({ min: 1, max: 12 }), + (rawFiles, n) => { + // Caller contract: deduped + sorted list. Mirror it so the property + // exercises the same shape the runner feeds selectShard. + const list = [...new Set(rawFiles)].sort(); + const shards = []; + for (let i = 1; i <= n; i++) shards.push(selectShard(list, { index: i, total: n })); + // completeness + disjointness + const flat = shards.flat(); + assert.deepStrictEqual([...flat].sort(), [...list].sort()); + assert.strictEqual(new Set(flat).size, list.length); + // balance — shard sizes differ by at most 1 (sizes is never empty + // because n >= 1, so there is always at least one shard). + const sizes = shards.map(s => s.length); + assert.ok(Math.max(...sizes) - Math.min(...sizes) <= 1, `sizes=${sizes}`); + }, + ), + { numRuns: 200 }, + ); + }); +}); + +describe('parseShardArg (#1212)', () => { + test('parses i/n into { index, total }', () => { + assert.deepStrictEqual(parseShardArg('2/3'), { index: 2, total: 3 }); + assert.deepStrictEqual(parseShardArg('1/1'), { index: 1, total: 1 }); + }); + + const bad = ['', '2', '0/3', '4/3', '1/0', '-1/3', '1.5/3', 'a/b', '1/3/2', ' 1/3', '1 / 3']; + for (const v of bad) { + test(`rejects malformed/out-of-range value ${JSON.stringify(v)}`, () => { + const r = parseShardArg(v); + assert.ok(r && r.error, `expected an error result for ${JSON.stringify(v)}, got ${JSON.stringify(r)}`); + }); + } +});