* ci(#4335): shard release.yml rc/finalize unit-suite tests The finalize job's unsharded unit-coverage step outgrew the 30-minute job timeout that was already raised once for this exact symptom (#2280): run 33988966357 finished all tests with 0 failures at 28m26s, then got cancelled ~80s into the post-test coverage merge — a phase that historically completes in 54-101s. The suite's wall-clock time, not a hang, ate the budget. test.yml already fixed the identical cliff for its own full-scope lane (#2952, #3057) by sharding the unit suite 3 ways with a separate merged coverage-gate job. Apply the same pattern to rc and finalize (rc has the byte-identical unsharded shape and would hit the same wall next): each gains a `*-test` matrix job (raw coverage only, no report/gate) and a `*-coverage-gate` job that merges the shards' raw V8 dumps before enforcing the existing gsd-core/bin/lib coverage floor. rc/finalize now depend on their gate job instead of running the suite inline. Updates release-coverage-scope.test.cjs's exact-count assertion for the new command surface and adds release-shard-lane-sharding.test.cjs to pin shard-set completeness and gate wiring, mirroring ci-full-lane-sharding.test.cjs. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Potential fix for pull request finding 'CodeQL / Cache Poisoning via execution of untrusted code' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> * Potential fix for pull request finding 'CodeQL / Cache Poisoning via execution of untrusted code' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> * fix(#4335): close CodeQL cache-poisoning and missing-permissions findings CodeQL flagged the PR (10 actions/cache-poisoning/poisonable-step errors, 4 actions/missing-workflow-permissions warnings) on release.yml. Remove `cache: 'npm'` from every actions/setup-node step in the file (7 occurrences, not just the 4 newly-added jobs the alerts pointed at) — restoring an npm cache before running install/build code in a write-permissioned job is exactly the shape this query targets, and the same pattern was already present unchanged in create/rc/finalize. These are short CI/release jobs; losing npm's install cache costs a few seconds per job, closing the finding everywhere it appears in this file rather than only where the alert happened to land on a changed line. Add explicit `permissions: contents: read` to rc-test, rc-coverage-gate, finalize-test, finalize-coverage-gate — the four new jobs had no permissions block at all and inherited the ambient default. Matches validate-version's existing least-privilege pattern; create/rc/finalize keep their own broader write/publish scopes unchanged. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: sim <sim@local> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
This commit is contained in:
Tom Boucher
@@ -12,15 +12,36 @@ const path = require('node:path');
|
||||
const RELEASE_WORKFLOW = path.join(__dirname, '..', '.github', 'workflows', 'release.yml');
|
||||
|
||||
describe('release-coverage-scope', () => {
|
||||
test('release.yml uses test:coverage:unit (not full suite) in both rc and finalize gates', () => {
|
||||
const lines = fs.readFileSync(RELEASE_WORKFLOW, 'utf8').split(/\r?\n/).map(l => l.trim());
|
||||
// #4335: rc/finalize used to run one unsharded `npm run test:coverage:unit`
|
||||
// line each. Both now shard the unit suite (raw coverage only, one line per
|
||||
// *-test job) and gate on a separate merged-coverage job (one report line
|
||||
// per *-coverage-gate job) — see rc-test/finalize-test and
|
||||
// rc-coverage-gate/finalize-coverage-gate. This asserts the new surface is
|
||||
// still unit-scoped end to end: the unsharded single-shot invocation is
|
||||
// gone for good (not silently reintroduced), the raw/report scripts appear
|
||||
// exactly once per job pair, and no bare full-suite line ever appears.
|
||||
test('release.yml uses the sharded unit-coverage scripts (not the unsharded or full suite) in both rc and finalize gates', () => {
|
||||
// Normalize an optional leading `run: ` so a single-line `run: npm run
|
||||
// …` step (the style rc-coverage-gate/finalize-coverage-gate use) counts
|
||||
// the same as a bare command line inside a multi-line `run: |` block
|
||||
// (the style the raw-coverage and legacy unit steps use).
|
||||
const lines = fs.readFileSync(RELEASE_WORKFLOW, 'utf8').split(/\r?\n/)
|
||||
.map(l => l.trim().replace(/^run:\s*/, ''));
|
||||
const bareCount = lines.filter(l => l === 'npm run test:coverage').length;
|
||||
const unitCount = lines.filter(l => l === 'npm run test:coverage:unit').length;
|
||||
const unshardedUnitCount = lines.filter(l => l === 'npm run test:coverage:unit').length;
|
||||
const rawShardedCount = lines.filter(l => l === 'npm run test:coverage:unit:raw -- --shard ${{ matrix.shard }}').length;
|
||||
const reportCount = lines.filter(l => l === 'npm run test:coverage:report').length;
|
||||
|
||||
assert.strictEqual(bareCount, 0,
|
||||
`release.yml still has ${bareCount} bare 'npm run test:coverage' line(s); expected 0`);
|
||||
assert.strictEqual(unitCount, 2,
|
||||
`release.yml has ${unitCount} 'npm run test:coverage:unit' line(s); expected 2`);
|
||||
assert.strictEqual(unshardedUnitCount, 0,
|
||||
`release.yml has ${unshardedUnitCount} unsharded 'npm run test:coverage:unit' line(s); ` +
|
||||
'expected 0 — the #4335 fix sharded rc/finalize onto test:coverage:unit:raw + test:coverage:report');
|
||||
assert.strictEqual(rawShardedCount, 2,
|
||||
`release.yml has ${rawShardedCount} sharded raw-coverage line(s) (one expected in each of ` +
|
||||
`rc-test and finalize-test); expected 2`);
|
||||
assert.strictEqual(reportCount, 2,
|
||||
`release.yml has ${reportCount} 'npm run test:coverage:report' line(s) (one expected in each ` +
|
||||
'of rc-coverage-gate and finalize-coverage-gate); expected 2');
|
||||
});
|
||||
|
||||
|
||||
});
|
||||
|
||||
166
tests/release-shard-lane-sharding.test.cjs
Normal file
166
tests/release-shard-lane-sharding.test.cjs
Normal file
@@ -0,0 +1,166 @@
|
||||
'use strict';
|
||||
|
||||
/**
|
||||
* release.yml's rc and finalize jobs both shard the unit suite the same way
|
||||
* test.yml's `scope: full` lane does (#2952) — see #4335 (recurrence of
|
||||
* #2280/#2281): the unsharded `npm run test:coverage:unit` step outgrew even
|
||||
* a 30-minute job timeout as the suite grew (run 33988966357 — all tests
|
||||
* passed, 0 failures, cancelled ~80s into the post-test coverage merge).
|
||||
*
|
||||
* Sharding introduces the same two failure modes ci-full-lane-sharding.test.cjs
|
||||
* pins for test.yml, so both are pinned here too:
|
||||
*
|
||||
* 1. An incomplete shard set. If the matrix declares shards 1/3 and 2/3 but
|
||||
* never 3/3, a third of the unit suite simply stops running and every
|
||||
* check still passes.
|
||||
*
|
||||
* 2. A dropped coverage gate. A sharded run leaves each runner with a
|
||||
* partial picture — shard 2 never executes shard 1's files, so those
|
||||
* read 0%. The gate therefore cannot live on the shards; it moved to
|
||||
* rc-coverage-gate/finalize-coverage-gate, which merge every shard's raw
|
||||
* V8 dumps. If those jobs silently stopped being required, coverage
|
||||
* enforcement would vanish without any red check.
|
||||
*/
|
||||
|
||||
const test = require('node:test');
|
||||
const assert = require('node:assert/strict');
|
||||
const fs = require('node:fs');
|
||||
const path = require('node:path');
|
||||
const yaml = require('js-yaml');
|
||||
|
||||
const WORKFLOWS_DIR = path.join(__dirname, '..', '.github', 'workflows');
|
||||
|
||||
function loadWorkflow(name) {
|
||||
return yaml.load(fs.readFileSync(path.join(WORKFLOWS_DIR, name), 'utf8'));
|
||||
}
|
||||
|
||||
/** Mirrors ci-full-lane-sharding.test.cjs's shard-spec grammar. */
|
||||
function parseShardSpec(spec) {
|
||||
const m = /^(\d+)\/(\d+)$/.exec(String(spec));
|
||||
if (!m) return null;
|
||||
const index = Number(m[1]);
|
||||
const total = Number(m[2]);
|
||||
if (!Number.isInteger(index) || !Number.isInteger(total)) return null;
|
||||
if (total < 1 || index < 1 || index > total) return null;
|
||||
return { index, total };
|
||||
}
|
||||
|
||||
/** True iff `specs` is exactly one complete shard set: same N, numerators 1..N. */
|
||||
function isCompleteShardSet(specs) {
|
||||
if (specs.length === 0) return false;
|
||||
const parsed = specs.map(parseShardSpec);
|
||||
if (parsed.some((p) => p === null)) return false;
|
||||
const total = parsed[0].total;
|
||||
if (parsed.some((p) => p.total !== total)) return false;
|
||||
if (parsed.length !== total) return false;
|
||||
const seen = new Set(parsed.map((p) => p.index));
|
||||
return seen.size === total && [...seen].every((i) => i >= 1 && i <= total);
|
||||
}
|
||||
|
||||
const LANES = [
|
||||
{ label: 'rc', testJob: 'rc-test', gateJob: 'rc-coverage-gate', finalJob: 'rc', artifactPrefix: 'coverage-tmp-rc-shard-' },
|
||||
{ label: 'finalize', testJob: 'finalize-test', gateJob: 'finalize-coverage-gate', finalJob: 'finalize', artifactPrefix: 'coverage-tmp-finalize-shard-' },
|
||||
];
|
||||
|
||||
test('release.yml rc/finalize unit-suite lanes are sharded and complete (#4335)', async (t) => {
|
||||
const workflow = loadWorkflow('release.yml');
|
||||
|
||||
for (const lane of LANES) {
|
||||
await t.test(`${lane.label}: the *-test job's matrix declares a complete shard set`, () => {
|
||||
const job = workflow.jobs[lane.testJob];
|
||||
assert.ok(job, `release.yml declares no \`${lane.testJob}\` job`);
|
||||
const shards = (job.strategy && job.strategy.matrix && job.strategy.matrix.shard) || [];
|
||||
assert.ok(
|
||||
Array.isArray(shards) && shards.length > 1,
|
||||
`\`${lane.testJob}\` is not actually sharded (matrix.shard: ${JSON.stringify(shards)}) — `
|
||||
+ 'that is the #4335/#2952 regression: the whole suite on one runner grows past its cap.',
|
||||
);
|
||||
assert.ok(
|
||||
isCompleteShardSet(shards),
|
||||
`\`${lane.testJob}\`'s declared shards ${JSON.stringify(shards)} are not a complete set. `
|
||||
+ 'Every entry must share one denominator N and the numerators must be exactly 1..N — a '
|
||||
+ 'missing numerator silently stops running that slice of the suite while every check stays green.',
|
||||
);
|
||||
});
|
||||
|
||||
await t.test(`${lane.label}: each shard runs its own slice, not the whole suite`, () => {
|
||||
const job = workflow.jobs[lane.testJob];
|
||||
const unitStep = job.steps.find(
|
||||
(s) => typeof s.run === 'string' && s.run.includes('test:coverage:unit:raw'),
|
||||
);
|
||||
assert.ok(unitStep, `no step in \`${lane.testJob}\` runs the raw unit coverage script`);
|
||||
assert.match(
|
||||
unitStep.run, /--shard \$\{\{ matrix\.shard \}\}/,
|
||||
`the unit step in \`${lane.testJob}\` does not pass matrix.shard through to run-tests.cjs, `
|
||||
+ 'so every shard would run the ENTIRE suite — N times the cost, no speedup.',
|
||||
);
|
||||
});
|
||||
|
||||
await t.test(`${lane.label}: each shard uploads a distinctly-named raw coverage artifact`, () => {
|
||||
const job = workflow.jobs[lane.testJob];
|
||||
const uploadStep = job.steps.find(
|
||||
(s) => String(s.uses || '').includes('upload-artifact'),
|
||||
);
|
||||
assert.ok(uploadStep, `no upload-artifact step in \`${lane.testJob}\``);
|
||||
assert.ok(
|
||||
String(uploadStep.with && uploadStep.with.name).startsWith(lane.artifactPrefix),
|
||||
`\`${lane.testJob}\`'s upload-artifact name does not start with '${lane.artifactPrefix}' `
|
||||
+ `(was: ${JSON.stringify(uploadStep.with && uploadStep.with.name)})`,
|
||||
);
|
||||
});
|
||||
|
||||
await t.test(`${lane.label}: a dedicated coverage-gate job exists and merges the shards`, () => {
|
||||
const gate = workflow.jobs[lane.gateJob];
|
||||
assert.ok(gate, `release.yml declares no \`${lane.gateJob}\` job`);
|
||||
assert.ok(
|
||||
(gate.needs || []).includes(lane.testJob),
|
||||
`\`${lane.gateJob}\` must depend on \`${lane.testJob}\` — it merges that job's shard artifacts`,
|
||||
);
|
||||
|
||||
const merges = gate.steps.some(
|
||||
(s) => String(s.uses || '').includes('download-artifact')
|
||||
&& s.with && s.with['merge-multiple'] === true
|
||||
&& String(s.with.pattern || '').startsWith(lane.artifactPrefix),
|
||||
);
|
||||
assert.ok(
|
||||
merges,
|
||||
`\`${lane.gateJob}\` does not download the \`${lane.artifactPrefix}*\` shard artifacts with `
|
||||
+ 'merge-multiple. Without every shard merged into one coverage/tmp, the gate scores a '
|
||||
+ 'partial run: files no shard in hand executed read 0%.',
|
||||
);
|
||||
|
||||
const reports = gate.steps.some(
|
||||
(s) => typeof s.run === 'string' && s.run.includes('test:coverage:report'),
|
||||
);
|
||||
assert.ok(
|
||||
reports,
|
||||
`\`${lane.gateJob}\` never runs the coverage report+gate script — the gsd-core/bin/lib `
|
||||
+ 'coverage floor would be gone',
|
||||
);
|
||||
});
|
||||
|
||||
await t.test(`${lane.label}: the final job depends on its coverage gate`, () => {
|
||||
const finalJob = workflow.jobs[lane.finalJob];
|
||||
assert.ok(finalJob, `release.yml declares no \`${lane.finalJob}\` job`);
|
||||
assert.ok(
|
||||
(finalJob.needs || []).includes(lane.gateJob),
|
||||
`\`${lane.finalJob}\` does not depend on \`${lane.gateJob}\` — a red/skipped coverage gate `
|
||||
+ `could not block ${lane.label} from tagging/publishing`,
|
||||
);
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
test('release.yml shard-spec parsing is exact at its boundaries (#4335)', () => {
|
||||
assert.deepEqual(parseShardSpec('1/3'), { index: 1, total: 3 });
|
||||
assert.deepEqual(parseShardSpec('3/3'), { index: 3, total: 3 });
|
||||
assert.equal(parseShardSpec('0/3'), null);
|
||||
assert.equal(parseShardSpec('4/3'), null);
|
||||
assert.equal(parseShardSpec('a/3'), null);
|
||||
|
||||
assert.equal(isCompleteShardSet(['1/3', '2/3', '3/3']), true);
|
||||
assert.equal(isCompleteShardSet(['1/3', '2/3']), false, 'missing numerator');
|
||||
assert.equal(isCompleteShardSet(['1/3', '2/3', '2/3']), false, 'duplicate numerator');
|
||||
assert.equal(isCompleteShardSet(['1/3', '2/3', '3/4']), false, 'mixed denominator');
|
||||
assert.equal(isCompleteShardSet([]), false, 'empty set');
|
||||
});
|
||||
Reference in New Issue
Block a user