diff --git a/.changeset/sunny-finches-wave.md b/.changeset/sunny-finches-wave.md new file mode 100644 index 000000000..6e4bf75ea --- /dev/null +++ b/.changeset/sunny-finches-wave.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 4903 +--- +**Windows conformance-tier CI no longer blows its own chunk timeout on unmeasured test files.** `scripts/run-tests.cjs` now weighs a test file absent from `tests/test-timings.json` at the documented ~2.2x Windows-cost floor instead of the plain (Linux-measured) table mean, on win32 only — a cluster of unmeasured conformance-tier files could otherwise pack into one chunk and exceed the 600s per-chunk backstop even though the chunk looked in-budget. (#4434) diff --git a/scripts/run-tests.cjs b/scripts/run-tests.cjs index f0a6a5385..1de8df61e 100644 --- a/scripts/run-tests.cjs +++ b/scripts/run-tests.cjs @@ -535,12 +535,36 @@ function loadTestTimings(timingsPath) { return { timings, mean, medianWeight: median / mean }; } +// #4434: the table's own `sources` are Linux-only (test-events-linux-node22/24 +// .jsonl — never a Windows event stream), and this runner's own chunk-kill +// diagnostic already tells operators "real Windows cost runs ~2.2x the +// recorded figure, so treat every number as a floor" (see the catch block +// around the per-chunk timeout, below). That string was, until this fix, +// advisory text ONLY — nothing in the weigher actually applied it. Verified +// live: `next` @ ccfed6335 (unrelated to the chunk's own diff — see #4434) +// killed Windows conformance shard 3/3 chunk 6/8 at 600016ms; 6 of that +// chunk's 17 files were wholly absent from the table and were packed at the +// table's plain mean, identically to how a Linux/macOS chunk would price +// them. A MEASURED file does not get this multiplier: #4733 already +// calibrates measured-file packing against a real Windows wall-clock via +// MAX_FILES_PER_CHUNK, so inflating measured weights again here would +// double-apply the correction. An unmeasured file has no real data at all — +// it is exactly the "floor, not a verdict" case the diagnostic already warns +// about, so only its fallback gets the multiplier, and only on win32. +const WINDOWS_UNMEASURED_COST_MULTIPLIER = 2.2; + // Build the packer's weight function from a loaded timing table. // // A file present in the table weighs its measured duration relative to the // table mean. A file ABSENT from it weighs 1 — the table MEAN, the same // value a null table (missing or unparseable file) yields for every file, -// because both states mean the same thing: cost unknown. +// because both states mean the same thing: cost unknown. On win32 the +// fallback is WINDOWS_UNMEASURED_COST_MULTIPLIER instead of 1 (see #4434, +// above) — but ONLY for a file absent from an otherwise-loaded table; a +// completely missing/corrupt/empty timings file (`timings` is `null`) still +// degrades to uniform weight 1 on every platform, matching the pre-#2456 +// count-based-packing invariant many existing tests depend on. Every other +// platform keeps the plain mean. // // This was previously `timings.medianWeight`, on the claim that an absent // file "costs chunk balance, never a red build." That claim is false. In a @@ -550,11 +574,14 @@ function loadTestTimings(timingsPath) { // cause a red build: Windows conformance shard 2/3, chunk 4/6 was killed at // 600018ms with ZERO failing tests, because files absent from the table // packed as if they were nearly free and the chunk blew the 600s cap. -// Empirically, mean is the right estimate for an unknown file: 9 unmeasured -// files that caused the incident averaged 6659ms against a table mean of -// 7152ms — within 7%. -function makeFileWeigher(timings) { +// Empirically, mean is the right estimate for an unknown file on the +// platform the table was MEASURED on: 9 unmeasured files that caused the +// incident averaged 6659ms against a table mean of 7152ms — within 7%. That +// equivalence does not hold on win32, where the table's own sources are +// Linux-only (#4434). +function makeFileWeigher(timings, platform = process.platform) { if (!timings) return () => 1; + const unmeasuredWeight = platform === 'win32' ? WINDOWS_UNMEASURED_COST_MULTIPLIER : 1; return (f) => { const key = basename(f); // Own-property check before the lookup. This is defense-in-depth, NOT a @@ -567,7 +594,9 @@ function makeFileWeigher(timings) { // keeps the lookup correct for arbitrary input, since this function is // exported and does not control its caller's strings. const ms = Object.hasOwn(timings.timings, key) ? timings.timings[key] : undefined; - return typeof ms === 'number' && Number.isFinite(ms) && ms >= 0 ? ms / timings.mean : 1; + return typeof ms === 'number' && Number.isFinite(ms) && ms >= 0 + ? ms / timings.mean + : unmeasuredWeight; }; } @@ -1192,7 +1221,7 @@ function main() { let weigherMemo = null; const fileWeightOf = () => { if (weigherMemo === null) { - weigherMemo = makeFileWeigher(loadedTimings()); + weigherMemo = makeFileWeigher(loadedTimings(), process.platform); } return weigherMemo; }; @@ -1854,6 +1883,7 @@ module.exports = { defaultMaxFilesPerChunk, loadTestTimings, makeFileWeigher, + WINDOWS_UNMEASURED_COST_MULTIPLIER, makeMeasuredPredicate, packChunks, // 2026-09-07 (PR #4497): the codex-config.test.cjs chunk-isolation fix — diff --git a/tests/run-tests-harness.test.cjs b/tests/run-tests-harness.test.cjs index a75a627b1..69faf951f 100644 --- a/tests/run-tests-harness.test.cjs +++ b/tests/run-tests-harness.test.cjs @@ -409,6 +409,15 @@ test('ambient GSD workstream vars are stripped by the runner', () => { const r = runHarness(tmpDir, [], { RUN_TESTS_MAX_CMDLINE_CHARS: '100000', RUN_TESTS_MAX_FILES_PER_CHUNK: '3', + // #4434: without this, these tiny-*.test.cjs names fall through to the + // REAL committed tests/test-timings.json as unmeasured entries, and on + // win32 an unmeasured entry in a loaded table now weighs + // WINDOWS_UNMEASURED_COST_MULTIPLIER (2.2), not 1 — breaking the exact + // {3,2,2} count-based split this test asserts. Point at a path that + // cannot exist so the harness takes the no-table branch (uniform + // weight 1 on every platform), matching this test's actual intent: + // pure file-count chunking, independent of any cost table. + RUN_TESTS_TIMINGS_FILE: path.join(tmpDir, 'no-such-timings-4434.json'), }); assert.strictEqual( r.status, @@ -2294,6 +2303,7 @@ describe('bug #969 C — ensureBuiltHooks populates hooks/dist before concurrent const { packChunks, makeFileWeigher, + WINDOWS_UNMEASURED_COST_MULTIPLIER, loadTestTimings, positiveNumberEnv, DEFAULT_TIMINGS_PATH, @@ -2428,13 +2438,99 @@ describe('chunk packing weights measured cost (#2456)', () => { // not 20/30 (the median) — see #2456 follow-up, red-next 2026-09-14. const t = tableFrom({ 'a.test.cjs': 10000, 'b.test.cjs': 20000, 'c.test.cjs': 60000 }); try { - const weigh = makeFileWeigher(loadTestTimings(t.path)); + const weigh = makeFileWeigher(loadTestTimings(t.path), 'linux'); assert.strictEqual(weigh('brand-new-test.test.cjs'), 1); } finally { cleanup(t.dir); } }); + test('#4434: on win32, a file missing from the table falls back to the documented Windows multiplier (2.2), not the mean', () => { + const t = tableFrom({ 'a.test.cjs': 10000, 'b.test.cjs': 20000, 'c.test.cjs': 60000 }); + try { + const weigh = makeFileWeigher(loadTestTimings(t.path), 'win32'); + assert.strictEqual(weigh('brand-new-test.test.cjs'), WINDOWS_UNMEASURED_COST_MULTIPLIER); + } finally { + cleanup(t.dir); + } + }); + + test('#4434: on linux/darwin, a file missing from the table still falls back to the plain mean (1)', () => { + const t = tableFrom({ 'a.test.cjs': 10000, 'b.test.cjs': 20000, 'c.test.cjs': 60000 }); + try { + assert.strictEqual(makeFileWeigher(loadTestTimings(t.path), 'linux')('brand-new-test.test.cjs'), 1); + assert.strictEqual(makeFileWeigher(loadTestTimings(t.path), 'darwin')('brand-new-test.test.cjs'), 1); + } finally { + cleanup(t.dir); + } + }); + + test('#4434: on win32, a MEASURED file is unaffected by the unmeasured-file multiplier', () => { + const t = tableFrom({ 'a.test.cjs': 10000, 'b.test.cjs': 20000, 'c.test.cjs': 60000 }); + try { + const weighWin = makeFileWeigher(loadTestTimings(t.path), 'win32'); + const weighLinux = makeFileWeigher(loadTestTimings(t.path), 'linux'); + assert.strictEqual( + weighWin('b.test.cjs'), + weighLinux('b.test.cjs'), + 'a measured file must weigh identically regardless of platform', + ); + } finally { + cleanup(t.dir); + } + }); + + test('#4434: a completely missing table still degrades to uniform weight 1 on win32 — the Windows multiplier only applies to a file absent FROM an otherwise-loaded table', () => { + const weigh = makeFileWeigher(null, 'win32'); + assert.strictEqual( + weigh('anything.test.cjs'), + 1, + 'no table at all must keep the pre-#2456 uniform-weight invariant on every platform, including win32', + ); + }); + + test('property: an unmeasured file weighs exactly the Windows multiplier on win32, and exactly 1 on every other platform, for any measured table', () => { + const fc = require('fast-check'); + fc.assert( + fc.property( + fc.array(fc.integer({ min: 1, max: 500000 }), { minLength: 1, maxLength: 30 }), + fc.constantFrom('win32', 'linux', 'darwin', 'freebsd', 'sunos'), + (mss, platform) => { + const timingsMap = Object.fromEntries( + mss.map((ms, i) => [`p${String(i).padStart(3, '0')}.test.cjs`, ms]), + ); + const mean = mss.reduce((a, b) => a + b, 0) / mss.length; + const timings = { timings: timingsMap, mean, medianWeight: 1 }; + const weigh = makeFileWeigher(timings, platform); + const expected = platform === 'win32' ? WINDOWS_UNMEASURED_COST_MULTIPLIER : 1; + assert.strictEqual(weigh('never-measured.test.cjs'), expected); + }, + ), + { numRuns: 200, seed: 44340 }, + ); + }); + + test('property: a MEASURED file weighs identically regardless of platform, for any measured table', () => { + const fc = require('fast-check'); + fc.assert( + fc.property( + fc.array(fc.integer({ min: 1, max: 500000 }), { minLength: 1, maxLength: 30 }), + fc.constantFrom('win32', 'linux', 'darwin', 'freebsd', 'sunos'), + (mss, platform) => { + const timingsMap = Object.fromEntries( + mss.map((ms, i) => [`p${String(i).padStart(3, '0')}.test.cjs`, ms]), + ); + const mean = mss.reduce((a, b) => a + b, 0) / mss.length; + const timings = { timings: timingsMap, mean, medianWeight: 1 }; + const weighPlatform = makeFileWeigher(timings, platform); + const weighLinux = makeFileWeigher(timings, 'linux'); + assert.strictEqual(weighPlatform('p000.test.cjs'), weighLinux('p000.test.cjs')); + }, + ), + { numRuns: 200, seed: 44341 }, + ); + }); + test('an unknown file packs without error rather than failing the run', () => { const chunks = packMeasured([...FILES, 'never-measured.test.cjs'], 4); assert.ok( @@ -2496,7 +2592,7 @@ describe('chunk packing weights measured cost (#2456)', () => { assert.ok(table.medianWeight < 0.05, 'fixture must actually be right-skewed'); const files = Array.from({ length: 30 }, (_, i) => `unmeasured-${i}.test.cjs`); const chunks = packChunks(files, { - weightOf: makeFileWeigher(table), + weightOf: makeFileWeigher(table, 'linux'), maxWeight: 6, maxChars: ROOMY_CHARS, fixedOverhead: FIXED_OVERHEAD, @@ -2542,7 +2638,7 @@ describe('chunk packing weights measured cost (#2456)', () => { table.medianWeight < 0.2, `fixture must be right-skewed; got medianWeight=${table.medianWeight}`, ); - const weigh = makeFileWeigher(table); + const weigh = makeFileWeigher(table, 'linux'); assert.strictEqual(weigh('never-measured.test.cjs'), 1); } finally { cleanup(t.dir); @@ -2552,7 +2648,7 @@ describe('chunk packing weights measured cost (#2456)', () => { test('that matches what a MISSING table already does — both mean "unknown"', () => { const t = tableFrom(SKEWED_MS); try { - const weigh = makeFileWeigher(loadTestTimings(t.path)); + const weigh = makeFileWeigher(loadTestTimings(t.path), 'linux'); const weighNull = makeFileWeigher(null); assert.strictEqual(weighNull('anything.test.cjs'), 1); assert.strictEqual(weighNull('anything.test.cjs'), weigh('never-measured.test.cjs')); @@ -2772,7 +2868,7 @@ describe('chunk packing weights measured cost (#2456)', () => { // present in the table weighs 1 (the mean), whatever it resolves to. const t = tableFrom({ 'a.test.cjs': 10000, 'b.test.cjs': 20000, 'c.test.cjs': 60000 }); try { - const weigh = makeFileWeigher(loadTestTimings(t.path)); + const weigh = makeFileWeigher(loadTestTimings(t.path), 'linux'); for (const name of ['constructor', 'toString', 'valueOf', 'hasOwnProperty', '__proto__']) { const w = weigh(name); assert.strictEqual(typeof w, 'number', `${name} must weigh a number, not a function`);