golem15/msd-core
3
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

fix(#4949): cap unmeasured files per Windows conformance chunk (#4950)

Browse Source 
* fix(#4949): cap unmeasured files per Windows conformance chunk

The windows conformance CI lane keeps going red every few updates:
scripts/run-tests.cjs kills a chunk at the 600s per-chunk backstop with
zero failing tests, pure slowness. Both recent incidents (ccfed63355,
af822a8024) killed a chunk holding 5-6 files absent from
tests/test-timings.json, packed alongside the chunk's measured files —
each unmeasured file's guessed weight looked affordable alone, but
several guesses compounded into a real overrun no single file's weight
predicted. tests/test-timings.json is manually regenerated and
routinely stale (30.5% of the conformance-tier pool is currently
unmeasured), so this keeps recurring as new conformance-tier files are
added.

Add a third, independent per-chunk budget to packChunks: a cap on how
many unmeasured files any one chunk may hold (win32-only, default 2,
RUN_TESTS_MAX_UNMEASURED_PER_CHUNK-overridable), using the same
"skip this bin, try the next; grow chunk count if none has room"
mechanism already proven safe for the existing char budget. This
bounds the compounding directly and protects every future batch of
newly-added, not-yet-measured conformance-tier files without depending
on anyone remembering to regenerate the timings table.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* chore(#4949): add changeset for windows conformance chunk cap fix

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* fix(#4949): address standards-review findings — changeset format, cap-1 boundary test

Reformat the changeset body to the documented **bold** — explanation.
format (was missing the em-dash separator and had a period inside the
bold clause) and add the trailing (#4949) backlink. Add the missing
cap-1 boundary test case alongside the existing cap/cap+1 rows.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* chore(#4949): backfill changeset PR number (#4950)

* fix(#4949): don't apply the unmeasured-cap when no timings table loads at all

Caught live: PR #4950's own conformance test (windows-latest, 24,
shard 1/3) failed a pre-existing pinned test. makeMeasuredPredicate(null)
returns false for every file when the timings table is completely
missing/corrupt -- a different fact than "a loaded table exists but
doesn't cover this file". main() derived MAX_UNMEASURED_PER_CHUNK
unconditionally, so with no table at all every file counted as
"unmeasured" and the win32 cap of 2 split 7 files into 4 chunks instead
of the 3 the file-count-chunking contract (and a pinned test) require.

Gate the cap on loadedTimings() itself: no table -> Infinity (no cap,
matching the long-standing uniform-weight-1 no-table contract).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

---------

Co-authored-by: sim <sim@local>
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
2026-09-23 15:26:20 -04:00
committed by GitHub
parent af822a8024
commit 6486647626
3 changed files with 298 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
.changeset/noble-hawks-rally.md Normal file
View File

@@ -0,0 +1,5 @@
---
type: Fixed
pr: 4950
---
**Windows conformance CI no longer times out on newly-added test files** — a chunk holding several files not yet in the timing table could blow the 600s per-chunk budget even though each looked individually affordable; unmeasured files are now capped at 2 per chunk on Windows so a batch of new conformance-tier tests can no longer compound into a red `next`. (#4949)

76
scripts/run-tests.cjs
View File

@@ -356,6 +356,20 @@ function defaultMaxFilesPerChunk(platform) {
return platform === 'win32' ? 22 : 60;
}
// Per-chunk UNMEASURED-file-count budget default, by platform. See the
// isMeasured/maxUnmeasuredPerChunk comment above packChunks's definition for
// the incident this guards (red next @ccfed6335 2026-09-20, @af822a80
// 2026-09-23 — both windows conformance shards, chunks killed at 600000ms+
// with zero failing tests) and why the bound is a COUNT of unmeasured files,
// not a weight: an unmeasured file's weight is a guess, and this cap exists
// precisely because several guesses compounding is what blew the budget, not
// because any individual guess was too large. win32-only — the failure has
// only ever been observed there; every other platform stays unbounded
// (Infinity), unchanged from today's behavior.
function defaultMaxUnmeasuredPerChunk(platform) {
return platform === 'win32' ? 2 : Infinity;
}
// ── #4020: run-scoped temp root ─────────────────────────────────────────────
//
// Fixture trees leak under os.tmpdir() on the SUCCESS path (the untouched half
@@ -640,10 +654,31 @@ function makeMeasuredPredicate(timings) {
// that file; when no chunk has room, the chunk count grows and packing restarts.
// A single file longer than the budget lands alone rather than looping forever.
//
// `isMeasured`/`maxUnmeasuredPerChunk` (2026-09-23, red `next` @ccfed6335 and
// @af822a80, both windows conformance shards, chunks killed at 600000ms+) add a
// THIRD, independent budget alongside weight and chars, for the same reason
// #4434 gave an unmeasured file its own windows multiplier instead of trusting
// its weight: an unmeasured file's weight is a GUESS, not a measurement, and
// several guesses landing in the same chunk let their individual uncertainty
// compound into a real failure no single file's weight predicted. Both
// incidents killed a chunk holding 5-6 files absent from tests/test-timings.json
// (new conformance-tier files added since the table was last regenerated) packed
// alongside the chunk's measured files — each guess looked affordable alone, the
// chunk's TOTAL weight still cleared the budget, and it still blew the 600s
// backstop. Isolation (partitionIsolatedFiles, above) cannot help here — it only
// pulls out a file PROVEN heavy, and an unmeasured file has no proof either way.
// Capping how many unmeasured files ANY one chunk may hold bounds the compounding
// directly, independent of what their guessed weight happens to be, using the
// exact same "skip this bin, try the next; grow the chunk count if none has
// room" mechanism already proven safe for the char budget below. `isMeasured` is
// optional (omitted callers/tests get today's behavior unchanged — no cap), and
// `maxUnmeasuredPerChunk` degrades to "no cap" for any non-finite or negative
// value, matching `maxWeight`/`maxChars`'s own degrade-safely contract.
//
// Ordering is fully deterministic — ties break on the separator-normalized file
// path, and each chunk's files are emitted in their original selection order —
// so the packing is byte-identical across Windows/macOS/Linux.
function packChunks(files, { weightOf, maxWeight, maxChars, fixedOverhead }) {
function packChunks(files, { weightOf, maxWeight, maxChars, fixedOverhead, isMeasured, maxUnmeasuredPerChunk }) {
if (files.length === 0) return [];
// packChunks is exported, so it cannot assume its caller normalized these.
// A non-finite or non-positive budget makes the chunk-count arithmetic
@@ -653,6 +688,10 @@ function packChunks(files, { weightOf, maxWeight, maxChars, fixedOverhead }) {
const weightBudget = Number.isFinite(maxWeight) && maxWeight > 0 ? maxWeight : files.length;
const charBudget = Number.isFinite(maxChars) && maxChars > 0 ? maxChars : Number.MAX_SAFE_INTEGER;
const overhead = Number.isFinite(fixedOverhead) && fixedOverhead >= 0 ? fixedOverhead : 0;
const unmeasuredBudget =
Number.isFinite(maxUnmeasuredPerChunk) && maxUnmeasuredPerChunk >= 0
? maxUnmeasuredPerChunk
: Infinity;
const safeWeight = (file) => {
const w = weightOf(file);
return Number.isFinite(w) && w >= 0 ? w : 0;
@@ -662,6 +701,7 @@ function packChunks(files, { weightOf, maxWeight, maxChars, fixedOverhead }) {
index,
weight: safeWeight(file),
chars: file.length + 1, // +1 for the inter-arg separator
measured: isMeasured ? !!isMeasured(file) : true,
}));
const totalWeight = entries.reduce((sum, e) => sum + e.weight, 0);
// Ties break on a SEPARATOR-NORMALIZED path so a subdir file orders the same
@@ -693,14 +733,18 @@ function packChunks(files, { weightOf, maxWeight, maxChars, fixedOverhead }) {
entries: [],
weight: 0,
chars: overhead,
unmeasuredCount: 0,
}));
let overflowed = false;
for (const entry of heaviestFirst) {
let target = null;
for (const bin of bins) {
// An empty bin always accepts, so an over-long single file lands alone
// instead of growing the chunk count forever.
// instead of growing the chunk count forever. Same rule for the
// unmeasured-count budget below — it exists to spread uncertainty
// across chunks, not to make a lone unmeasured file unplaceable.
if (bin.entries.length > 0 && bin.chars + entry.chars > charBudget) continue;
if (bin.entries.length > 0 && !entry.measured && bin.unmeasuredCount >= unmeasuredBudget) continue;
if (target === null || bin.weight < target.weight) target = bin;
}
if (target === null) {
@@ -710,6 +754,7 @@ function packChunks(files, { weightOf, maxWeight, maxChars, fixedOverhead }) {
target.entries.push(entry);
target.weight += entry.weight;
target.chars += entry.chars;
if (!entry.measured) target.unmeasuredCount += 1;
}
if (!overflowed) {
return bins
@@ -1490,6 +1535,30 @@ function main() {
process.env.RUN_TESTS_MAX_FILES_PER_CHUNK,
DEFAULT_MAX_FILES_PER_CHUNK,
);
// See packChunks' isMeasured/maxUnmeasuredPerChunk comment (above its
// definition) and defaultMaxUnmeasuredPerChunk (above) for the incident and
// rationale. RUN_TESTS_MAX_UNMEASURED_PER_CHUNK overrides for operators/tests,
// same pattern as every other *_PER_CHUNK knob in this file.
//
// Gated on loadedTimings() itself (not just deferring to isMeasured's
// per-file answer): makeMeasuredPredicate(null) — a completely missing or
// corrupt table — returns `false` for EVERY file, which is a different fact
// than "a loaded table exists but doesn't cover this file." The cap exists
// to bound uncertainty among files a mostly-reliable table failed to cover,
// not to re-litigate the no-table case, which has its own long-standing
// contract (makeFileWeigher's `if (!timings) return () => 1`): uniform
// weight 1, pure count-based packing, unaffected by this cap. Caught live
// (red conformance test (windows-latest, 24, shard 1/3), PR #4950): with no
// table loaded, every one of 7 files in
// "chunks by file count even when argv length is below the ceiling" was
// "unmeasured", and the win32 cap of 2 split them into 4 chunks instead of
// the 3 that test — and the uniform-weight-1 contract — require.
const MAX_UNMEASURED_PER_CHUNK = loadedTimings()
? positiveNumberEnv(
process.env.RUN_TESTS_MAX_UNMEASURED_PER_CHUNK,
defaultMaxUnmeasuredPerChunk(process.platform),
)
: Infinity;
// #2088 established that file COUNT is a poor proxy for a chunk's wall-clock:
// install-heavy files (real installs) cost ~10x a unit file, and when several
// land in the SAME chunk it blows the 600s backstop while unit-only chunks
@@ -1632,6 +1701,8 @@ function main() {
maxWeight: MAX_FILES_PER_CHUNK,
maxChars: MAX_CMDLINE_CHARS,
fixedOverhead: FIXED_OVERHEAD,
isMeasured: fileMeasuredOf(),
maxUnmeasuredPerChunk: MAX_UNMEASURED_PER_CHUNK,
}),
];
@@ -1881,6 +1952,7 @@ module.exports = {
selectShard,
positiveNumberEnv,
defaultMaxFilesPerChunk,
defaultMaxUnmeasuredPerChunk,
loadTestTimings,
makeFileWeigher,
WINDOWS_UNMEASURED_COST_MULTIPLIER,

219
tests/run-tests-harness.test.cjs
View File

@@ -439,6 +439,39 @@ test('ambient GSD workstream vars are stripped by the runner', () => {
);
});
test('the unmeasured-file cap has no effect when no timings table loads at all', () => {
// Same 7-file seed and file-count-chunking env as the sibling test above,
// but with RUN_TESTS_MAX_UNMEASURED_PER_CHUNK set to 1 — lower than any
// chunk's file count under plain {3,2,2} file-count chunking. If the
// unmeasured cap wrongly applied when no table loads (main() deriving it
// unconditionally instead of gating on loadedTimings()), a cap of 1 would
// force 7 single-file chunks instead of 3. Proves the cap genuinely has
// NO effect in the no-table case, not just that it happens not to bind.
const names = Array.from({ length: 7 }, (_, i) => `tiny-${String(i).padStart(2, '0')}.test.cjs`);
seed(tmpDir, names);
const r = runHarness(tmpDir, [], {
RUN_TESTS_MAX_CMDLINE_CHARS: '100000',
RUN_TESTS_MAX_FILES_PER_CHUNK: '3',
RUN_TESTS_MAX_UNMEASURED_PER_CHUNK: '1',
RUN_TESTS_TIMINGS_FILE: path.join(tmpDir, 'no-such-timings-4434.json'),
});
assert.strictEqual(
r.status,
0,
`expected zero exit; got status=${r.status} signal=${r.signal}\nSTDERR:\n${r.stderr}`,
);
assert.match(
r.stderr,
/run-tests: chunk 1\/3 — 3 files/,
`expected file-count chunking marker (unaffected by unmeasured cap) in stderr; STDERR:\n${r.stderr}`,
);
assert.match(
r.stderr,
/run-tests: chunk 3\/3 — 2 files/,
`expected final file-count chunking marker (unaffected by unmeasured cap) in stderr; STDERR:\n${r.stderr}`,
);
});
// #2088: expensive files must never all land in one chunk — otherwise the
// unsharded targeted lane packs the whole install surface into a single chunk
// that blows the 600s per-chunk backstop on the slow Windows runner. #2088
@@ -2307,6 +2340,7 @@ const {
loadTestTimings,
positiveNumberEnv,
DEFAULT_TIMINGS_PATH,
defaultMaxUnmeasuredPerChunk,
} = require('../scripts/run-tests.cjs');
describe('chunk packing weights measured cost (#2456)', () => {
@@ -2912,6 +2946,191 @@ describe('chunk packing weights measured cost (#2456)', () => {
});
});
// ---------------------------------------------------------------------------
// packChunks unmeasured-file cap — red `next` @ccfed6335 (2026-09-20, shard
// 3/3 chunk 6/8) and @af822a80 (2026-09-23, shard 2/3 chunk 7/9). Both chunks
// were killed at 600000ms+ with zero failing tests, each holding 5-6 files
// absent from tests/test-timings.json (new conformance-tier files, never
// profiled) packed alongside the chunk's measured files: each unmeasured
// file's GUESSED weight looked affordable alone, the chunk's total weight
// still cleared the existing budget, and the chunk still blew the 600s
// backstop because several guesses compounded. See packChunks'
// isMeasured/maxUnmeasuredPerChunk comment for the fix rationale.
// ---------------------------------------------------------------------------
describe('packChunks unmeasured-file cap (red next @ccfed6335, @af822a80, 2026-09-20/23)', () => {
const UC_FIXED_OVERHEAD = 120;
const UC_ROOMY_CHARS = 100000;
// Trivial, uniform weight model — this suite is about the COUNT cap, not
// weight — so any weight-driven splitting is ruled out by a generous
// maxWeight.
const weightOf = () => 1;
const isNewFile = (file) => /^new-\d+\.test\.cjs$/.test(file);
const countUnmeasured = (chunk) => chunk.filter((f) => isNewFile(f)).length;
test('reproduces the incident shape and proves the fix', () => {
const unmeasured = Array.from({ length: 6 }, (_, i) => `new-${i}.test.cjs`);
const measured = ['heavy-a.test.cjs', 'heavy-b.test.cjs'];
const files = [...unmeasured, ...measured];
const capped = packChunks(files, {
weightOf,
maxWeight: 100, // roomy — weight alone would never force a split
maxChars: UC_ROOMY_CHARS,
fixedOverhead: UC_FIXED_OVERHEAD,
isMeasured: (f) => !isNewFile(f),
maxUnmeasuredPerChunk: 2,
});
assert.ok(capped.length >= 3, `expected at least 3 chunks with the cap applied, got ${capped.length}`);
for (const [i, chunk] of capped.entries()) {
assert.ok(countUnmeasured(chunk) <= 2, `chunk ${i} holds ${countUnmeasured(chunk)} unmeasured files, cap is 2`);
}
assert.strictEqual(capped.flat().length, files.length, 'no file may be dropped or duplicated');
// Without the cap (omitted), all 8 files pack into a single chunk — this
// is the pre-fix behavior the cap must change.
const uncapped = packChunks(files, {
weightOf,
maxWeight: 100,
maxChars: UC_ROOMY_CHARS,
fixedOverhead: UC_FIXED_OVERHEAD,
isMeasured: (f) => !isNewFile(f),
// maxUnmeasuredPerChunk intentionally omitted
});
assert.strictEqual(uncapped.length, 1, 'without a cap, weight/chars alone keep all 8 files in one chunk');
});
describe('boundary (cap-1 / cap / cap+1)', () => {
const CAP = 3;
test('cap-1 unmeasured files fit one chunk', () => {
const files = Array.from({ length: CAP - 1 }, (_, i) => `new-${i}.test.cjs`);
const chunks = packChunks(files, {
weightOf,
maxWeight: 100,
maxChars: UC_ROOMY_CHARS,
fixedOverhead: UC_FIXED_OVERHEAD,
isMeasured: (f) => !isNewFile(f),
maxUnmeasuredPerChunk: CAP,
});
assert.strictEqual(chunks.length, 1, `${CAP - 1} unmeasured files under cap ${CAP} must fit one chunk`);
});
test('exactly cap unmeasured files fit one chunk', () => {
const files = Array.from({ length: CAP }, (_, i) => `new-${i}.test.cjs`);
const chunks = packChunks(files, {
weightOf,
maxWeight: 100,
maxChars: UC_ROOMY_CHARS,
fixedOverhead: UC_FIXED_OVERHEAD,
isMeasured: (f) => !isNewFile(f),
maxUnmeasuredPerChunk: CAP,
});
assert.strictEqual(chunks.length, 1, `${CAP} unmeasured files at cap ${CAP} must fit one chunk`);
});
test('cap+1 unmeasured files require a second chunk', () => {
const files = Array.from({ length: CAP + 1 }, (_, i) => `new-${i}.test.cjs`);
const chunks = packChunks(files, {
weightOf,
maxWeight: 100,
maxChars: UC_ROOMY_CHARS,
fixedOverhead: UC_FIXED_OVERHEAD,
isMeasured: (f) => !isNewFile(f),
maxUnmeasuredPerChunk: CAP,
});
assert.strictEqual(chunks.length, 2, `${CAP + 1} unmeasured files over cap ${CAP} must split into 2 chunks`);
for (const [i, chunk] of chunks.entries()) {
assert.ok(countUnmeasured(chunk) <= CAP, `chunk ${i} holds ${countUnmeasured(chunk)} unmeasured, cap is ${CAP}`);
}
});
});
test('backward compatible: isMeasured omitted treats every file as measured, cap is a no-op', () => {
const files = Array.from({ length: 8 }, (_, i) => `new-${i}.test.cjs`);
const withoutIsMeasured = packChunks(files, {
weightOf,
maxWeight: 100,
maxChars: UC_ROOMY_CHARS,
fixedOverhead: UC_FIXED_OVERHEAD,
maxUnmeasuredPerChunk: 2, // has no effect: nothing is "unmeasured" without isMeasured
});
const noCapAtAll = packChunks(files, {
weightOf,
maxWeight: 100,
maxChars: UC_ROOMY_CHARS,
fixedOverhead: UC_FIXED_OVERHEAD,
});
assert.deepStrictEqual(withoutIsMeasured, noCapAtAll, 'omitting isMeasured must reproduce the no-cap packing exactly');
assert.strictEqual(withoutIsMeasured.length, 1, 'all 8 files land in one chunk, matching pre-fix behavior');
});
describe('maxUnmeasuredPerChunk degrades safely', () => {
test('no-op when zero files are unmeasured', () => {
const files = ['measured-a.test.cjs', 'measured-b.test.cjs', 'measured-c.test.cjs'];
const chunks = packChunks(files, {
weightOf,
maxWeight: 100,
maxChars: UC_ROOMY_CHARS,
fixedOverhead: UC_FIXED_OVERHEAD,
isMeasured: () => true,
maxUnmeasuredPerChunk: 2,
});
assert.strictEqual(chunks.length, 1, 'a cap with nothing unmeasured must not affect packing');
});
test('a NaN override degrades to no cap rather than throwing', () => {
const files = Array.from({ length: 8 }, (_, i) => `new-${i}.test.cjs`);
assert.doesNotThrow(() => {
const chunks = packChunks(files, {
weightOf,
maxWeight: 100,
maxChars: UC_ROOMY_CHARS,
fixedOverhead: UC_FIXED_OVERHEAD,
isMeasured: (f) => !isNewFile(f),
maxUnmeasuredPerChunk: NaN,
});
assert.strictEqual(chunks.length, 1, 'NaN cap must degrade to unbounded, all 8 files in one chunk');
});
});
test('a negative override degrades to no cap rather than throwing', () => {
const files = Array.from({ length: 8 }, (_, i) => `new-${i}.test.cjs`);
assert.doesNotThrow(() => {
const chunks = packChunks(files, {
weightOf,
maxWeight: 100,
maxChars: UC_ROOMY_CHARS,
fixedOverhead: UC_FIXED_OVERHEAD,
isMeasured: (f) => !isNewFile(f),
maxUnmeasuredPerChunk: -1,
});
assert.strictEqual(chunks.length, 1, 'negative cap must degrade to unbounded, all 8 files in one chunk');
});
});
});
describe('defaultMaxUnmeasuredPerChunk platform behavior', () => {
test('win32 default is 2', () => {
assert.strictEqual(defaultMaxUnmeasuredPerChunk('win32'), 2);
});
test('linux and darwin defaults are Infinity (unbounded)', () => {
assert.strictEqual(defaultMaxUnmeasuredPerChunk('linux'), Infinity);
assert.strictEqual(defaultMaxUnmeasuredPerChunk('darwin'), Infinity);
});
test('the default is still overridable by RUN_TESTS_MAX_UNMEASURED_PER_CHUNK', () => {
const def = defaultMaxUnmeasuredPerChunk('win32');
const override = positiveNumberEnv('7', def);
assert.strictEqual(override, 7);
assert.notStrictEqual(override, def);
});
});
});
// ─── analyzeChunkEvents (#3889 durability fix) ──────────────────────────────
//
// scripts/lib/ndjson-reporter.cjs now writes durably (fs.appendFileSync to a