Skip to content

Commit 52d5a52

Browse files
claude[bot]claude
andauthored
test(scripts): floor only the unconditional cases of two self-tests (#15399)
`run-with-stall-guard.mjs` (floor 41) and `pm/release-rehearsal-clone.mjs` (floor 32) each pinned an assertion floor over a battery whose case count depends on the HOST, not on whether the cases ran: 21 of the stall guard's cases sit behind `existsSync('/proc')`, and 2 of the rehearsal clone's behind `existsSync` of the rehearsal doc and of `lint.yml`. Off Linux, or in a checkout missing either file, those floors red for the ENVIRONMENT — and the remedy an author reaches for is editing the floor down, the one habit these floors exist to prevent. The environment-conditional cases now go through a sink variant that reports into the same `failures`/`results` sinks and registers nothing (`checkConditional`, `tConditional`). Their conditions, messages and arguments are unchanged; the guards are untouched and no condition is inverted. Why they are outside the roster is stated at the guard and in the roster comment. Floors re-measured off a run at pin 9999, never derived by subtraction: 41 -> 20 and 32 -> 30, read from the breach line each self-test printed. The skipped block still names itself: the stall guard keeps its existing `(process-classification cases skipped: /proc not available)` line, and C10 gains the equivalent `(C10 wiring cases skipped: ... not present)` line so a guard that skips can never read as one that ran and held. Claude-Session: https://claude.ai/code/session_012zGPuVVX3deAx9LdjK8jCk Co-authored-by: Claude <noreply@anthropic.com>
1 parent c550baf commit 52d5a52

2 files changed

Lines changed: 112 additions & 29 deletions

File tree

scripts/pm/release-rehearsal-clone.mjs

Lines changed: 49 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -553,14 +553,30 @@ function cloneOf(root, source, name, { depth = 0 } = {}) {
553553
// battery is the shape PR #14896, PR #15003 and PR #15217 landed for exactly
554554
// this case.
555555
//
556+
// ⚠️ What the floor deliberately does NOT count (#15317): C10's two wiring
557+
// cases, each guarded by an `existsSync` — the rehearsal doc and `lint.yml`.
558+
// They still assert exactly as they always did; they go through
559+
// `tConditional`, which reports into the same `failures` tally and registers
560+
// nothing. A floor over an ENVIRONMENT-CONDITIONAL case counts what the
561+
// checkout happens to contain, so it would red for the environment rather than
562+
// for a case that stopped running — and the remedy an author reaches for is
563+
// editing the floor down, the one habit these floors exist to prevent (#13797's
564+
// ruling, carried forward from the ALLOWLIST loops in
565+
// check-whole-set-label-write.mjs). A skipped guard names ITSELF on C10's
566+
// `cases skipped` line. The floor pins the part that does not move with the
567+
// checkout.
568+
//
556569
// ⛔ A pinned TOTAL is not the repair: a battery dropping from 9 cases to 3
557570
// keeps a total "right" the moment a sibling grows.
558571
//
559572
// The count is a FLOOR, not an equality — adding cases is ordinary work and must
560573
// not red. A battery BELOW its floor means cases stopped running; the remedy is
561574
// to find what stopped registering.
575+
//
576+
// 30 = the cases that run in EVERY checkout, measured off a run (#15317), never
577+
// derived by subtracting the guarded cases by hand.
562578
const SELF_TEST_BATTERIES = Object.freeze({
563-
'release-rehearsal-clone self-test': 32,
579+
'release-rehearsal-clone self-test': 30,
564580
});
565581

566582
// DELETING an entry silences that battery's floor exactly as effectively as
@@ -591,15 +607,31 @@ function selfTest() {
591607
battery('release-rehearsal-clone self-test');
592608
const root = mkdtempSync(join(tmpdir(), 'rehearsal-clone-selftest-'));
593609
let failures = 0;
594-
const t = (name, cond, extra = '') => {
595-
registerCase();
610+
const report = (name, cond, extra = '') => {
596611
if (cond) {
597612
process.stdout.write(` ✓ ${name}\n`);
598613
} else {
599614
failures += 1;
600615
process.stdout.write(` ✗ ${name}${extra ? `\n ${extra.replace(/\n/g, '\n ')}` : ''}\n`);
601616
}
602617
};
618+
const t = (name, cond, extra = '') => {
619+
registerCase();
620+
report(name, cond, extra);
621+
};
622+
// The sink for C10's two `existsSync`-guarded cases: it asserts and reports
623+
// EXACTLY as `t` does — same condition, same message, same `failures` tally —
624+
// and registers nothing, because those cases run only where the two wiring
625+
// files are present. A floor over them would count what the CHECKOUT happens
626+
// to contain (a sparse or partial tree carries neither), so it would red for
627+
// the environment rather than for a case that stopped running, and the remedy
628+
// an author reaches for is editing the floor down — the one habit these floors
629+
// exist to prevent (#15317, applying #13797's ruling; the precedent is the
630+
// ALLOWLIST loops in check-whole-set-label-write.mjs). A skipped guard NAMES
631+
// ITSELF on the `cases skipped` line at C10 instead of going quiet.
632+
const tConditional = (name, cond, extra = '') => {
633+
report(name, cond, extra);
634+
};
603635

604636
try {
605637
const source = makeSource(root, 'source');
@@ -705,12 +737,24 @@ function selfTest() {
705737
// to end, so the two places that invoke it are pinned here.
706738
const doc = join(REPO_ROOT, 'docs', 'releases-maintenance.md');
707739
const lint = join(REPO_ROOT, '.github', 'workflows', 'lint.yml');
740+
// Both cases are ENVIRONMENT-CONDITIONAL and therefore outside the roster —
741+
// see `tConditional` above. They assert unchanged.
742+
const skippedWiring = [];
708743
if (existsSync(doc)) {
709-
t('C10 the rehearsal doc names this script', readFileSync(doc, 'utf8').includes(SELF));
744+
tConditional('C10 the rehearsal doc names this script', readFileSync(doc, 'utf8').includes(SELF));
745+
} else {
746+
skippedWiring.push(doc);
710747
}
711748
if (existsSync(lint)) {
712749
const body = readFileSync(lint, 'utf8');
713-
t('C10 lint.yml still runs this self-test', body.includes(SELF) && body.includes('--self-test'));
750+
tConditional('C10 lint.yml still runs this self-test', body.includes(SELF) && body.includes('--self-test'));
751+
} else {
752+
skippedWiring.push(lint);
753+
}
754+
if (skippedWiring.length) {
755+
process.stdout.write(
756+
` (C10 wiring cases skipped: ${skippedWiring.join(', ')} not present)\n`,
757+
);
714758
}
715759
} finally {
716760
rmSync(root, { recursive: true, force: true });

scripts/run-with-stall-guard.mjs

Lines changed: 63 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -228,14 +228,30 @@ const argv = process.argv.slice(2);
228228
// battery is the shape PR #14896, PR #15003 and PR #15217 landed for exactly
229229
// this case.
230230
//
231+
// ⚠️ What the floor deliberately does NOT count (#15317): the cases behind the
232+
// `const linux = existsSync('/proc')` guard — process classification, the
233+
// SIGTERM-trapping descendant, the source-side liveness probe and its cap. They
234+
// still assert exactly as they always did; they go through `checkConditional`,
235+
// which reports into the same `failures`/`results` sinks and registers nothing.
236+
// A floor over an ENVIRONMENT-CONDITIONAL battery counts what the host happens
237+
// to provide, so off Linux it would red for the ENVIRONMENT rather than for a
238+
// case that stopped running — and the remedy an author reaches for is editing
239+
// the floor down, the one habit these floors exist to prevent (#13797's ruling,
240+
// carried forward from the ALLOWLIST loops in check-whole-set-label-write.mjs).
241+
// The skipped block names ITSELF instead, on the `cases skipped` line printed
242+
// with the verdict. The floor pins the part that does not move with the host.
243+
//
231244
// ⛔ A pinned TOTAL is not the repair: a battery dropping from 9 cases to 3
232245
// keeps a total "right" the moment a sibling grows.
233246
//
234247
// The count is a FLOOR, not an equality — adding cases is ordinary work and must
235248
// not red. A battery BELOW its floor means cases stopped running; the remedy is
236249
// to find what stopped registering.
250+
//
251+
// 20 = the cases that run on EVERY host, measured off a run (#15317), never
252+
// derived by subtracting the conditional block by hand.
237253
const SELF_TEST_BATTERIES = Object.freeze({
238-
'run-with-stall-guard self-test': 41,
254+
'run-with-stall-guard self-test': 20,
239255
});
240256

241257
// DELETING an entry silences that battery's floor exactly as effectively as
@@ -822,6 +838,19 @@ async function selfTest() {
822838
};
823839
battery('run-with-stall-guard self-test');
824840
const dir = mkdtempSync(join(tmpdir(), 'stall-guard-selftest-'));
841+
// ⚠️ THE GUARD, and why the cases behind it are OUTSIDE the roster (#15317).
842+
// Every case below that needs `/proc` — process classification, the
843+
// SIGTERM-trapping descendant, the source-side liveness probe and its cap —
844+
// runs only where this is true, so on a host without `/proc` they do not run
845+
// and a floor that counted them would red for the ENVIRONMENT rather than for
846+
// a case that stopped running. That is a floor lying about what it measured,
847+
// and the remedy an author would reach for is editing the floor down — the
848+
// one habit these floors exist to prevent. So they go through
849+
// `checkConditional` below: they assert exactly as before, and they register
850+
// nothing. The floor pins the part that does not move with the environment.
851+
// The skipped block still NAMES ITSELF — the `cases skipped` line printed with
852+
// the verdict — so "did not run here" never reads as "ran and held".
853+
// Precedent: the ALLOWLIST loops in check-whole-set-label-write.mjs (#13797).
825854
const linux = existsSync('/proc');
826855
const failures = [];
827856
const results = [];
@@ -831,15 +860,25 @@ async function selfTest() {
831860
// a fixed 5s tick — the production 10-minute window still polls every 5s.
832861
const WINDOW = ['--stall-minutes', '0.05'];
833862

834-
const check = (label, cond, detail) => {
835-
registerCase();
863+
const record = (label, cond, detail) => {
836864
if (cond) {
837865
results.push(` ✓ ${label}`);
838866
} else {
839867
failures.push(label);
840868
results.push(` ✗ ${label}${detail ? ` — ${detail}` : ''}`);
841869
}
842870
};
871+
const check = (label, cond, detail) => {
872+
registerCase();
873+
record(label, cond, detail);
874+
};
875+
// The sink for the `if (linux)` cases: it asserts and reports EXACTLY as
876+
// `check` does — same condition, same message, same `failures` entry — and
877+
// registers nothing. See the `const linux` guard above for why those cases are
878+
// outside the roster.
879+
const checkConditional = (label, cond, detail) => {
880+
record(label, cond, detail);
881+
};
843882

844883
// Arms the SIGUSR2 stack harvest in the synthetic children, exactly as the
845884
// CI steps do. Without this the "no report = blocked loop" inference cannot
@@ -960,9 +999,9 @@ async function selfTest() {
960999
check('idle hang is not rescued by the liveness probe (plain verdict, no deferral)',
9611000
!out.includes('STALL-CAP') && !out.includes('deferring the kill'));
9621001
if (linux) {
963-
check('idle hang is classified idle, not on-CPU',
1002+
checkConditional('idle hang is classified idle, not on-CPU',
9641003
out.includes('idle -- waiting on something that never settles'));
965-
check('a live event loop answers SIGUSR2 with a report',
1004+
checkConditional('a live event loop answers SIGUSR2 with a report',
9661005
/SIGUSR2 -> \d+ node process\(es\), [1-9]\d* responded/.test(out));
9671006
}
9681007
}
@@ -982,20 +1021,20 @@ async function selfTest() {
9821021
reportEnv(spinDir), { marker: dir },
9831022
);
9841023
const { code, out } = res;
985-
check('sync-spinning hang: the guard exits on its own', !res.timedOut,
1024+
checkConditional('sync-spinning hang: the guard exits on its own', !res.timedOut,
9861025
'the guard never exited — detection is broken');
987-
check('sync-spinning hang is declared a stall', code === STALL_EXIT_CODE, `exit ${code}`);
988-
check('sync-spinning hang is classified ON-CPU',
1026+
checkConditional('sync-spinning hang is declared a stall', code === STALL_EXIT_CODE, `exit ${code}`);
1027+
checkConditional('sync-spinning hang is classified ON-CPU',
9891028
out.includes('ON-CPU -- sync-spinning or GC-thrashing'));
990-
check('a blocked event loop is diagnosed by its SILENCE',
1029+
checkConditional('a blocked event loop is diagnosed by its SILENCE',
9911030
out.includes('NO report -- its event loop is BLOCKED'));
9921031
// The inversion this card exists to design out. A spinning hang pegs a
9931032
// core, so any probe that reads CPU as liveness stops firing here -- and
9941033
// "never fires on spin hangs" is strictly worse than the defect it would
9951034
// be fixing, because no green run can tell it apart from success.
996-
check('sync-spinning hang is NOT rescued by the probe — burning CPU is not liveness',
1035+
checkConditional('sync-spinning hang is NOT rescued by the probe — burning CPU is not liveness',
9971036
!out.includes('STALL-CAP') && !out.includes('deferring the kill'));
998-
check('the verdict names the source-side probe as the reason it fired',
1037+
checkConditional('the verdict names the source-side probe as the reason it fired',
9991038
out.includes('FROZEN at the source'));
10001039
}
10011040

@@ -1037,9 +1076,9 @@ async function selfTest() {
10371076
'--log', join(dir, 'group.log'), ...WINDOW, '--', 'sh', script,
10381077
], {}, { marker: dir });
10391078
const { code, out } = res;
1040-
check('SIGTERM-trapping descendant: the guard exits on its own', !res.timedOut,
1079+
checkConditional('SIGTERM-trapping descendant: the guard exits on its own', !res.timedOut,
10411080
'the guard never exited — teardown is broken');
1042-
check('stall with a SIGTERM-trapping descendant still exits 75',
1081+
checkConditional('stall with a SIGTERM-trapping descendant still exits 75',
10431082
code === STALL_EXIT_CODE, `exit ${code}`);
10441083
// "Dead" means gone OR a zombie: SIGKILL leaves the entry in /proc until
10451084
// the (now reparented) process is reaped, and in a container PID 1 may be
@@ -1059,9 +1098,9 @@ async function selfTest() {
10591098
await sleep(100);
10601099
}
10611100
const alive = state !== null && state !== 'Z';
1062-
check('the descendant does not outlive the guard', !alive,
1101+
checkConditional('the descendant does not outlive the guard', !alive,
10631102
`pid ${pid} still running (state=${state}) after the guard exited`);
1064-
check('the SIGKILL escalation is reported, not silent',
1103+
checkConditional('the SIGKILL escalation is reported, not silent',
10651104
out.includes('ignored SIGTERM'));
10661105
if (alive) {
10671106
try { process.kill(pid, 'SIGKILL'); } catch { /* already gone */ }
@@ -1086,9 +1125,9 @@ async function selfTest() {
10861125
['--log', join(dir, 'buffered.log'), ...WINDOW, '--stall-cap-minutes', '0.5', '--', ...shape],
10871126
{}, { marker: dir },
10881127
);
1089-
check('a healthy silent-but-working run is NOT killed', saved.code === 0, `exit ${saved.code}`);
1090-
check('...and is never called a stall', !saved.out.includes('STALL'));
1091-
check('...and the deferral is announced, not silent',
1128+
checkConditional('a healthy silent-but-working run is NOT killed', saved.code === 0, `exit ${saved.code}`);
1129+
checkConditional('...and is never called a stall', !saved.out.includes('STALL'));
1130+
checkConditional('...and the deferral is announced, not silent',
10921131
saved.out.includes('deferring the kill'), saved.out.trim().split('\n')[0]);
10931132

10941133
// Positive control, and the reason case 8 is evidence rather than a
@@ -1098,9 +1137,9 @@ async function selfTest() {
10981137
['--log', join(dir, 'buffered-noprobe.log'), ...WINDOW, '--no-liveness-probe', '--', ...shape],
10991138
{}, { marker: dir },
11001139
);
1101-
check('positive control: the same run IS killed with --no-liveness-probe',
1140+
checkConditional('positive control: the same run IS killed with --no-liveness-probe',
11021141
killed.code === STALL_EXIT_CODE, `exit ${killed.code}`);
1103-
check('positive control: and the verdict says the probe was off',
1142+
checkConditional('positive control: and the verdict says the probe was off',
11041143
killed.out.includes('disabled (--no-liveness-probe)'));
11051144
}
11061145

@@ -1119,12 +1158,12 @@ async function selfTest() {
11191158
{}, { marker: dir },
11201159
);
11211160
const { code, out } = res;
1122-
check('a writing hang: the guard still exits on its own', !res.timedOut,
1161+
checkConditional('a writing hang: the guard still exits on its own', !res.timedOut,
11231162
'the guard never exited — the probe inverted the defect');
1124-
check('a hang that keeps writing is still killed', code === STALL_EXIT_CODE, `exit ${code}`);
1125-
check('...at the cap, under its own distinct STALL-CAP verdict',
1163+
checkConditional('a hang that keeps writing is still killed', code === STALL_EXIT_CODE, `exit ${code}`);
1164+
checkConditional('...at the cap, under its own distinct STALL-CAP verdict',
11261165
out.includes('STALL-CAP'), out.trim().split('\n').slice(-2).join(' | '));
1127-
check('...having first announced the deferral it was granted',
1166+
checkConditional('...having first announced the deferral it was granted',
11281167
out.includes('deferring the kill'));
11291168
}
11301169

0 commit comments

Comments
 (0)