* fix(#2472): weight-aware shard partition Windows shard 1/3 hit the 20-minute job cap with no failing assertion. Root cause is the shard layer, not the chunk layer: selectShard partitioned by sorted ARRAY INDEX (k % n, #1212), which balances file COUNTS and ignores file COST. On the real unit suite that produced 12.4m / 19.2m / 15.2m — a 1.23x max/ideal ratio leaving the heaviest shard 5% under the cap. Because assignment keyed off position, inserting one test file re-indexed every file after it and could tip that shard over; deterministic, so a re-run reproduced it exactly. This is NOT the chunk packer (#2456/#2463). That fix works and applies one level down, WITHIN a shard. The across-shard partition predated it and never consumed the cost table. Both layers now share one cost model. selectShard takes an optional weightOf and, when given one, partitions by LPT (longest-processing-time-first) — the same algorithm packChunks uses. Omitting it keeps the legacy round-robin byte-identical, so every existing test above still exercises that path unchanged and callers without timing data lose nothing. A missing timings table yields uniform weight 1, under which LPT degenerates to the equal-count split. Projected on the real suite: 16.4/17.3/13.0 -> 15.6/15.6/15.6 (worst shard 17.3m -> 15.6m). Tests: a skewed-cost regression (round-robin clusters all four heavy files onto one shard at 2.98x ideal; LPT does not), back-compat equivalence, determinism, tie-breaking, order preservation, and two fast-check properties — the partition is exhaustive and disjoint (getting this wrong silently DROPS tests from CI, the worst failure mode for a harness), and no shard exceeds average + heaviest file. Two assertions were corrected during authoring rather than shipped wrong: - an initial "LPT within 4/3 of ideal" bound was false. The 4/3 figure is relative to the OPTIMAL makespan, not the average, and the two differ when item sizes force a pairing. Replaced with Graham's average+max bound, which is what is actually provable. - "weighted is never worse than round-robin" is also false; fast-check falsified it with [19316,10190,1,9128,29353,20227] over 2 shards (rr 48670, lpt 48671). Round-robin can win by luck on a specific input. Dropped, with the counterexample recorded in place so it is not re-asserted later. Closes #2472 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(#2472): rotate tied bins; restore #1212 test block; lazy cost table Isolated-review findings, all fixed. HIGH — zero weights collapsed the whole partition onto shard 1. The lightest-bin scan compared weight only, and adding a zero-weight file leaves its bin's weight unchanged, so bin 0 stayed tied-minimum forever and every such file landed on it. Verified: all-zero weights gave shard1=[a..f], shard2=[], shard3=[] — two of three CI runners idle while one ran everything. Reachable through safeWeight's own clamp (a NaN/negative/Infinity entry in a corrupted or hand-edited timings table) and through any genuine 0ms measurement, so the clamp reproduced the exact failure its comment claimed to prevent. Ties now break on file COUNT after weight, which rotates. Pinned by two regression tests (all-zero, and clamped NaN/negative/Infinity) plus a property over list size x shard count. The live table has no 0ms entries (min 19ms), so production was not affected — but nothing prevented it. MEDIUM — the new describe block had swallowed #1212's pre-existing property test, which is why a test under a "weight-aware" heading never passed a weigher. That was a bad block boundary in the previous commit, not a bad test: the #2472 describe was opened before #1212's last test instead of after. Moved back where it belongs; #1212 is 762-879 and #2472 is 894-1082. LOW — that relocated property test ran unseeded. Seeded (12120) per the repo's property-test convention so a failure reproduces. Verified passing under the new seed. LOW — hoisting the timings load above the shard block charged a readFileSync + JSON.parse to invocations that exit before needing it (empty selection, --files matching nothing). Now lazily memoized, so neither consumer reads the table unless it is used and it is still read at most once. Real-suite projection unchanged at 15.6m / 15.6m / 15.6m. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(#2472): correct stale round-robin sharding descriptions The partition is now cost-balanced, so the header block in run-tests.cjs and the two comments in test.yml describing '--shard' as a round-robin over sorted file index were actively wrong. Updated to describe LPT over measured duration, and to state the degenerate case explicitly: with no timing data every file weighs the same and the partition collapses back to k % n, which is why the pre-existing #1212 CLI tests still pass unchanged (their nine synthetic files are absent from the timings table, so all take the identical median weight). Remaining 'round-robin' mentions are correct — they describe the unweighted fallback path. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(#2472): shard diagnostics, cost-routing E2E test, table validation Second orthogonal review (operational lens) findings, all fixed. HIGH — cross-runner partition divergence. Each of the up-to-12 CI jobs runs its own 'merge base into head' and computes its own partition, so if the inputs differ between jobs (the file list, or the timings table) two jobs can place the same file in different shards or in none. Every job stays internally exhaustive and disjoint, so nothing errors: a test simply never runs and CI stays green. The risk class is pre-existing — round-robin diverges identically when the file set differs between jobs, which is literally this issue's insertion instability — but weighting adds tests/test-timings.json as a second input that must match, so it widens the hole. Properly closing it means pinning the partition inputs per run, a workflow change beyond this fix. What IS closed here is the silence. Each shard now prints an input fingerprint over the FULL pre-partition list and the weight assigned to each file — deliberately not this shard's slice, which would differ by design and be useless for comparison. All shard jobs of one run must print an identical sig; a mismatch is direct proof the runners disagreed about the input. Verified: three independent computations agree, and the sig changes when the input drifts by one file. MEDIUM — nothing proved main() actually threads fileWeightOf() into selectShard. Every pre-existing --shard E2E test uses synthetic filenames absent from the real table, so all collapse to a uniform median weight, under which LPT is mathematically identical to k % n — a typo on that one wiring line would have passed the whole suite. Added an E2E test that injects a table via RUN_TESTS_TIMINGS_FILE with differing costs, placing the heavy files at exactly the indices round-robin hands to shard 1, and asserts shard 1 does NOT receive all three. Plus a test that all three shards emit the same sig. MEDIUM/LOW — no observability. The diagnostic line now reports files, weighed count, aggregate weight, and whether the table loaded, so a table that silently failed to parse shows table=absent/weighed=0 instead of being indistinguishable from a healthy load. (The reviewer confirmed the advisory fallback is already live on next: feat-2296-provider-escalation.test.cjs is missing from the table.) LOW — typeof [] === 'object', so a hand-edit turning the map into a list was accepted as a valid table. Now rejected via Array.isArray, falling back to uniform weight like any other malformed table. LOW — stale round-robin wording in ci-test-scope.test.cjs. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(#2472): pin every CI job to one base commit Closes the cross-runner divergence at its source instead of only making it visible. Each job of a run executes the rebase-check step independently, minutes apart across a 12-job matrix, and merged the MOVING origin/<branch> ref. If the base advanced mid-run, different jobs merged different trees. That was survivable when jobs only had to agree on pass/fail; it is not once they must agree on a PARTITION. Each shard job computes the whole split and keeps its own slice, so jobs working from different trees can place a file in two shards or in none — and every job still looks internally consistent, so nothing errors. A test silently never runs and CI stays green. ci-rebase-check.cjs now accepts CI_REBASE_BASE_SHA and pins BOTH the fetch and the merge to that one commit, so the two can never disagree. test.yml passes github.event.pull_request.base.sha on all three rebase-check steps; that value is fixed for the life of a run, so all jobs merge the identical base. This also closes the PRE-EXISTING half of the divergence. Round-robin had the same exposure whenever the test-file set differed between jobs — that is this issue's insertion instability — so the pin fixes the older hole too, not just the timings-table input weighting added. Only a full 40-hex sha is accepted; empty (push/workflow_dispatch), malformed, or injected values fall back to the branch ref rather than handing an arbitrary string to git fetch as a refspec. resolveBaseRefs is extracted pure and exported, and runMain is guarded behind require.main === module, so the pin contract is testable without spawning git. Tests (tests/ci-test-scope.test.cjs): every rebase-check step must carry the pin; a valid sha pins both refs; absence falls back correctly; and five hostile values — short sha, uppercase, --upload-pack= injection, ref expression, empty — are each rejected. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
@@ -492,7 +492,7 @@ describe('test-full shard matrix parity (#1212)', () => {
|
||||
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.
|
||||
// runner's cost-balanced shard selection covers every file with no gaps/overlaps.
|
||||
assert.deepStrictEqual(
|
||||
distinctShards,
|
||||
Array.from({ length: n }, (_, i) => i + 1),
|
||||
@@ -537,6 +537,62 @@ describe('test-full shard matrix parity (#1212)', () => {
|
||||
);
|
||||
});
|
||||
|
||||
// #2472: every job of a run must merge ONE base commit. Each job runs the
|
||||
// rebase-check step independently, minutes apart across the matrix, so
|
||||
// merging the moving branch ref lets jobs see different trees when the base
|
||||
// advances mid-run. The sharded lane makes jobs agree on a PARTITION, and
|
||||
// disagreement there places a file in two shards or none — silently, because
|
||||
// each job stays internally consistent and CI stays green.
|
||||
describe('#2472 base-commit pin', () => {
|
||||
const { resolveBaseRefs } = require('../scripts/ci-rebase-check.cjs');
|
||||
const SHA = 'a1b2c3d4e5f60718293a4b5c6d7e8f9012345678';
|
||||
|
||||
test('every rebase-check step pins CI_REBASE_BASE_SHA', () => {
|
||||
const doc = yaml.load(fs.readFileSync(path.join(WORKFLOWS_DIR, 'test.yml'), 'utf8'));
|
||||
const steps = Object.values(doc.jobs)
|
||||
.flatMap(j => j.steps || [])
|
||||
.filter(s => typeof s.run === 'string' && s.run.includes('ci-rebase-check.cjs'));
|
||||
assert.ok(steps.length > 0, 'expected at least one rebase-check step');
|
||||
for (const s of steps) {
|
||||
assert.ok(
|
||||
s.env && typeof s.env.CI_REBASE_BASE_SHA === 'string' && s.env.CI_REBASE_BASE_SHA.includes('base.sha'),
|
||||
'each rebase-check step must pin CI_REBASE_BASE_SHA to the PR base sha; '
|
||||
+ 'an unpinned job can merge a different tree than its siblings',
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
test('a full 40-hex sha pins both fetch and merge to that commit', () => {
|
||||
const r = resolveBaseRefs({ GITHUB_BASE_REF: 'next', CI_REBASE_BASE_SHA: SHA }, 'main');
|
||||
assert.strictEqual(r.fetchRef, SHA);
|
||||
assert.strictEqual(r.mergeRef, SHA, 'fetch and merge must target the same pinned commit');
|
||||
assert.strictEqual(r.pinned, true);
|
||||
});
|
||||
|
||||
test('no pin falls back to the branch ref (push / workflow_dispatch)', () => {
|
||||
const r = resolveBaseRefs({ GITHUB_BASE_REF: 'next' }, 'main');
|
||||
assert.strictEqual(r.fetchRef, 'next');
|
||||
assert.strictEqual(r.mergeRef, 'origin/next');
|
||||
assert.strictEqual(r.pinned, false);
|
||||
});
|
||||
|
||||
// A non-sha value must never reach `git fetch` as a refspec.
|
||||
for (const [label, value] of [
|
||||
['short sha', 'abc123'],
|
||||
['uppercase sha', 'A'.repeat(40)],
|
||||
['argument injection', 'next --upload-pack=evil'],
|
||||
['ref expression', 'next^{commit}'],
|
||||
['empty', ''],
|
||||
]) {
|
||||
test(`rejects ${label} and falls back to the branch ref`, () => {
|
||||
const r = resolveBaseRefs({ GITHUB_BASE_REF: 'next', CI_REBASE_BASE_SHA: value }, 'main');
|
||||
assert.strictEqual(r.pinned, false, `"${value}" must not be accepted as a pin`);
|
||||
assert.strictEqual(r.fetchRef, 'next');
|
||||
assert.strictEqual(r.mergeRef, 'origin/next');
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
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)
|
||||
|
||||
@@ -488,6 +488,54 @@ test('ambient GSD workstream vars are stripped by the runner', () => {
|
||||
assert.doesNotMatch(r.stderr, /shard-02\.test\.cjs/);
|
||||
});
|
||||
|
||||
// #2472 E2E: every other --shard test here uses synthetic filenames that
|
||||
// are absent from the real tests/test-timings.json, so they all collapse to
|
||||
// one uniform median weight — under which LPT is mathematically identical
|
||||
// to k % n. That means none of them can tell whether main() actually
|
||||
// threads fileWeightOf() into selectShard: a typo on that single line would
|
||||
// pass the entire existing suite. This test injects a table via
|
||||
// RUN_TESTS_TIMINGS_FILE with DIFFERING costs, so the weighted partition is
|
||||
// provably distinguishable from round-robin.
|
||||
test('--shard routes by measured cost end-to-end, not by index', (t) => {
|
||||
seed(tmpDir, SHARD_NAMES);
|
||||
// Heavy files sit at indices 0/3/6 — exactly the slice round-robin hands
|
||||
// to shard 1. Cost-weighting must NOT put all three on one shard.
|
||||
const timings = {
|
||||
schema_version: 1, unit: 'ms', timings: Object.fromEntries(
|
||||
SHARD_NAMES.map((n, i) => [n, i % 3 === 0 ? 30000 : 100]),
|
||||
),
|
||||
};
|
||||
const tablePath = path.join(tmpDir, 'injected-timings.json');
|
||||
fs.writeFileSync(tablePath, JSON.stringify(timings));
|
||||
t.after(() => { try { fs.unlinkSync(tablePath); } catch { /* best effort */ } });
|
||||
|
||||
const r = runHarness(tmpDir, ['--shard', '1/3'], { RUN_TESTS_TIMINGS_FILE: tablePath });
|
||||
assert.strictEqual(r.status, 0, `stderr: ${r.stderr}`);
|
||||
// Round-robin would give shard 1 exactly {00,03,06} — all three heavies.
|
||||
const heavies = ['shard-00', 'shard-03', 'shard-06']
|
||||
.filter(n => new RegExp(`${n}\\.test\\.cjs`).test(r.stderr)).length;
|
||||
assert.ok(
|
||||
heavies < 3,
|
||||
`cost-weighted shard 1 must not receive all three heavy files (that is the `
|
||||
+ `round-robin split, proving the weigher was not threaded); stderr: ${r.stderr}`,
|
||||
);
|
||||
// And the diagnostic must show the table was actually consumed.
|
||||
assert.match(r.stderr, /table=loaded/, 'shard diagnostics must report the injected table as loaded');
|
||||
assert.match(r.stderr, /sig=[0-9a-f]+/, 'shard diagnostics must emit an input fingerprint');
|
||||
});
|
||||
|
||||
test('shard diagnostics report an identical input fingerprint across shards', () => {
|
||||
seed(tmpDir, SHARD_NAMES);
|
||||
// The cross-runner divergence guard: every shard of one run computes the
|
||||
// partition independently, so all of them must agree on the INPUT. A
|
||||
// differing sig between shard jobs is the only observable signal that
|
||||
// they disagreed — which is how the union could silently drop a file.
|
||||
const sigOf = (out) => (out.match(/sig=([0-9a-f]+)/) || [])[1];
|
||||
const sigs = [1, 2, 3].map((i) => sigOf(runHarness(tmpDir, ['--shard', `${i}/3`]).stderr));
|
||||
assert.ok(sigs.every(Boolean), `every shard must emit a sig; got ${JSON.stringify(sigs)}`);
|
||||
assert.strictEqual(new Set(sigs).size, 1, `all shards must fingerprint the same input; got ${sigs}`);
|
||||
});
|
||||
|
||||
test('--shard 2/3 selects the second round-robin slice', () => {
|
||||
seed(tmpDir, SHARD_NAMES);
|
||||
const r = runHarness(tmpDir, ['--shard', '2/3']);
|
||||
@@ -871,7 +919,212 @@ describe('selectShard round-robin partition (#1212)', () => {
|
||||
assert.ok(Math.max(...sizes) - Math.min(...sizes) <= 1, `sizes=${sizes}`);
|
||||
},
|
||||
),
|
||||
{ numRuns: 200 },
|
||||
// Seed pinned so a failure is reproducible (repo convention for property
|
||||
// tests); previously unseeded, flagged by the #2472 isolated review.
|
||||
{ numRuns: 200, seed: 12120 },
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
// ─── #2472: weight-aware shard partition ────────────────────────────────────
|
||||
//
|
||||
// Equal file COUNTS is not equal file COST. Round-robin by array index balances
|
||||
// the former and ignores the latter, which on the real suite produced Windows
|
||||
// shard weights of 12.4m / 19.2m / 15.2m against a 20-minute job cap — a 1.23x
|
||||
// max/ideal ratio and a 5%-of-cap margin on the heaviest shard. Adding any test
|
||||
// file re-indexed the partition and tipped it over (#2472).
|
||||
//
|
||||
// Passing a weigher switches the partition to LPT (longest-processing-time
|
||||
// first), the same algorithm packChunks already uses one level down (#2456 /
|
||||
// #2463), so both layers share one cost model. Omitting the weigher keeps the
|
||||
// legacy round-robin exactly — every test above still exercises that path.
|
||||
|
||||
describe('selectShard weight-aware partition (#2472)', () => {
|
||||
const fc = require('fast-check');
|
||||
|
||||
const shardWeights = (files, total, weightOf) => {
|
||||
const out = [];
|
||||
for (let i = 1; i <= total; i++) {
|
||||
out.push(selectShard(files, { index: i, total }, weightOf).reduce((a, f) => a + weightOf(f), 0));
|
||||
}
|
||||
return out;
|
||||
};
|
||||
|
||||
// The regression: a right-skewed cost distribution (the real suite's shape —
|
||||
// a few dominant files among many cheap ones) laid out so that filename order
|
||||
// clusters the heavy files onto one shard.
|
||||
const skewed = {
|
||||
'a01.test.cjs': 30000, 'a02.test.cjs': 100, 'a03.test.cjs': 100,
|
||||
'a04.test.cjs': 28000, 'a05.test.cjs': 100, 'a06.test.cjs': 100,
|
||||
'a07.test.cjs': 26000, 'a08.test.cjs': 100, 'a09.test.cjs': 100,
|
||||
'a10.test.cjs': 24000, 'a11.test.cjs': 100, 'a12.test.cjs': 100,
|
||||
};
|
||||
const skewedFiles = Object.keys(skewed).sort();
|
||||
const skewedWeight = (f) => skewed[f];
|
||||
|
||||
test('REGRESSION: round-robin clusters heavy files; LPT does not', () => {
|
||||
const ideal = Object.values(skewed).reduce((a, b) => a + b, 0) / 3;
|
||||
|
||||
// Legacy partition (no weigher) scored with the same cost function, so the
|
||||
// two strategies are compared on identical inputs.
|
||||
const rr = [1, 2, 3].map((i) =>
|
||||
selectShard(skewedFiles, { index: i, total: 3 }).reduce((a, f) => a + skewedWeight(f), 0));
|
||||
const lpt = shardWeights(skewedFiles, 3, skewedWeight);
|
||||
|
||||
// Every heavy file sits at an index ≡ 0 (mod 3), so round-robin hands them
|
||||
// all to shard 1 — the exact clustering that produced the 19-minute shard.
|
||||
assert.ok(
|
||||
Math.max(...rr) / ideal > 1.5,
|
||||
`round-robin should be badly imbalanced on skewed costs; got ${rr} (ideal ${ideal})`,
|
||||
);
|
||||
// Graham's list-scheduling bound: makespan <= average + heaviest item. This
|
||||
// is the bound that is actually provable. The 4/3 figure quoted for LPT is
|
||||
// relative to the OPTIMAL makespan, not to the average — and the two differ
|
||||
// whenever item sizes force a pairing, as they do here: four items above
|
||||
// 24000 into three bins means some bin holds two of them, so 50000 is
|
||||
// optimal even though the average is 36267.
|
||||
const heaviest = Math.max(...Object.values(skewed));
|
||||
assert.ok(
|
||||
Math.max(...lpt) <= ideal + heaviest,
|
||||
`LPT must hold the average+max bound; got ${lpt} (bound ${ideal + heaviest})`,
|
||||
);
|
||||
assert.ok(
|
||||
Math.max(...lpt) < Math.max(...rr),
|
||||
`LPT must beat round-robin on skewed costs; lpt=${lpt} rr=${rr}`,
|
||||
);
|
||||
});
|
||||
|
||||
test('back-compat: omitting the weigher reproduces round-robin exactly', () => {
|
||||
for (let i = 1; i <= 3; i++) {
|
||||
assert.deepStrictEqual(
|
||||
selectShard(skewedFiles, { index: i, total: 3 }),
|
||||
skewedFiles.filter((_, k) => k % 3 === i - 1),
|
||||
'the unweighted path must stay byte-identical to the legacy partition',
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
test('a uniform weigher degenerates to equal counts', () => {
|
||||
const sizes = [1, 2, 3].map(
|
||||
(i) => selectShard(skewedFiles, { index: i, total: 3 }, () => 1).length,
|
||||
);
|
||||
assert.ok(
|
||||
Math.max(...sizes) - Math.min(...sizes) <= 1,
|
||||
`equal weights must give equal counts; got ${sizes}`,
|
||||
);
|
||||
});
|
||||
|
||||
test('n=1 is still a pure no-op with a weigher', () => {
|
||||
assert.deepStrictEqual(selectShard(skewedFiles, { index: 1, total: 1 }, skewedWeight), skewedFiles);
|
||||
});
|
||||
|
||||
test('preserves the caller sort order within a weighted shard', () => {
|
||||
const slice = selectShard(skewedFiles, { index: 1, total: 3 }, skewedWeight);
|
||||
assert.deepStrictEqual(slice, [...slice].sort(), 'downstream chunking assumes caller order');
|
||||
});
|
||||
|
||||
test('determinism: the weighted partition is stable across calls', () => {
|
||||
const a = selectShard(skewedFiles, { index: 2, total: 3 }, skewedWeight);
|
||||
const b = selectShard(skewedFiles, { index: 2, total: 3 }, skewedWeight);
|
||||
assert.deepStrictEqual(a, b);
|
||||
});
|
||||
|
||||
test('ties break deterministically, not by object iteration order', () => {
|
||||
const flat = () => 5;
|
||||
const a = selectShard(skewedFiles, { index: 1, total: 3 }, flat);
|
||||
const b = selectShard([...skewedFiles], { index: 1, total: 3 }, flat);
|
||||
assert.deepStrictEqual(a, b, 'equal weights must still partition identically');
|
||||
});
|
||||
|
||||
// A partition is a covering contract: every file lands in exactly one shard.
|
||||
// Getting this wrong silently DROPS tests from CI — the worst possible failure
|
||||
// mode for a test harness — so it is property-tested rather than sampled.
|
||||
test('property: the weighted partition is exhaustive and disjoint', () => {
|
||||
fc.assert(
|
||||
fc.property(
|
||||
fc.array(fc.integer({ min: 0, max: 60000 }), { minLength: 1, maxLength: 60 }),
|
||||
fc.integer({ min: 1, max: 8 }),
|
||||
(weights, total) => {
|
||||
const files = weights.map((_, i) => `p${String(i).padStart(3, '0')}.test.cjs`);
|
||||
const w = (f) => weights[Number(f.slice(1, 4))];
|
||||
const seen = [];
|
||||
for (let i = 1; i <= total; i++) seen.push(...selectShard(files, { index: i, total }, w));
|
||||
assert.strictEqual(new Set(seen).size, seen.length, 'a file appeared in two shards');
|
||||
assert.deepStrictEqual([...seen].sort(), [...files].sort(), 'a file was dropped or invented');
|
||||
},
|
||||
),
|
||||
{ numRuns: 200, seed: 24720 },
|
||||
);
|
||||
});
|
||||
|
||||
// Graham's bound for any greedy-into-lightest schedule, and the reason the
|
||||
// shard cap stops being reachable: the heaviest shard cannot exceed the
|
||||
// average by more than one file's cost, however the names happen to sort.
|
||||
test('property: no shard exceeds average + heaviest file', () => {
|
||||
fc.assert(
|
||||
fc.property(
|
||||
fc.array(fc.integer({ min: 1, max: 60000 }), { minLength: 3, maxLength: 60 }),
|
||||
fc.integer({ min: 2, max: 6 }),
|
||||
(weights, total) => {
|
||||
const files = weights.map((_, i) => `p${String(i).padStart(3, '0')}.test.cjs`);
|
||||
const w = (f) => weights[Number(f.slice(1, 4))];
|
||||
const sums = shardWeights(files, total, w);
|
||||
const bound = weights.reduce((a, b) => a + b, 0) / total + Math.max(...weights);
|
||||
assert.ok(
|
||||
Math.max(...sums) <= bound + 1e-9,
|
||||
`bound violated: max=${Math.max(...sums)} bound=${bound}`,
|
||||
);
|
||||
},
|
||||
),
|
||||
{ numRuns: 200, seed: 24721 },
|
||||
);
|
||||
});
|
||||
|
||||
// Deliberately NOT asserted: "weighted is never worse than round-robin".
|
||||
// fast-check falsifies it — weights [19316,10190,1,9128,29353,20227] over 2
|
||||
// shards give round-robin 48670 and LPT 48671. Round-robin can win by luck on
|
||||
// a specific input; LPT's guarantee is the worst-case bound above, not
|
||||
// universal dominance. The value for #2472 is that the bound holds for EVERY
|
||||
// distribution, so no arrangement of filenames can produce the 1.23x cluster
|
||||
// that round-robin allowed — which the skewed regression above pins directly.
|
||||
|
||||
// Zero-weight regression (isolated review, HIGH). Adding a zero-weight file
|
||||
// leaves its bin's weight unchanged, so a weight-only tiebreak kept bin 0
|
||||
// tied-minimum forever and every such file landed there: all-zero weights put
|
||||
// the entire suite on shard 1 and left the other runners idle. Reachable via
|
||||
// safeWeight's clamp of a NaN/negative/Infinity entry, or any genuine 0ms
|
||||
// measurement. The file-count tiebreak is what makes ties rotate.
|
||||
test('REGRESSION: zero weights still split evenly across shards', () => {
|
||||
const files = Array.from({ length: 9 }, (_, i) => `z${i}.test.cjs`);
|
||||
const sizes = [1, 2, 3].map((i) => selectShard(files, { index: i, total: 3 }, () => 0).length);
|
||||
assert.deepStrictEqual(sizes, [3, 3, 3], `all-zero weights must not collapse onto one shard; got ${sizes}`);
|
||||
});
|
||||
|
||||
test('REGRESSION: clamped NaN/negative/Infinity weights do not collapse', () => {
|
||||
const files = ['a', 'b', 'c', 'd', 'e', 'f'].map((x) => `${x}.test.cjs`);
|
||||
const hostile = { 'a.test.cjs': NaN, 'b.test.cjs': -5, 'c.test.cjs': Infinity };
|
||||
const sizes = [1, 2, 3].map(
|
||||
(i) => selectShard(files, { index: i, total: 3 }, (f) => (f in hostile ? hostile[f] : 1)).length,
|
||||
);
|
||||
assert.ok(
|
||||
Math.max(...sizes) - Math.min(...sizes) <= 1,
|
||||
`weights that clamp to 0 must still spread; got ${sizes}`,
|
||||
);
|
||||
});
|
||||
|
||||
test('property: zero weights spread evenly for any list size and shard count', () => {
|
||||
fc.assert(
|
||||
fc.property(
|
||||
fc.integer({ min: 1, max: 40 }),
|
||||
fc.integer({ min: 2, max: 6 }),
|
||||
(n, total) => {
|
||||
const files = Array.from({ length: n }, (_, i) => `p${String(i).padStart(3, '0')}.test.cjs`);
|
||||
const sizes = Array.from({ length: total }, (_, i) =>
|
||||
selectShard(files, { index: i + 1, total }, () => 0).length);
|
||||
assert.ok(Math.max(...sizes) - Math.min(...sizes) <= 1, `sizes=${sizes}`);
|
||||
},
|
||||
),
|
||||
{ numRuns: 200, seed: 24723 },
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user