Skip to content

Commit ff88a5f

Browse files
committed
docs(cli): correct the issue citation and reconcile the flake counts
Review findings on #327, both about the comment rather than the test. The comment said this race was asserted "until #262 work looked at it", which cites the wrong issue for the work this PR is doing. #262 reports two subprocess-spawning tests in error-envelope.test.ts and verify-test-commands.test.ts, a different failure mode, and never names this one — the PR body says as much, so the comment contradicted it. A future reader would have opened #262 and found nothing connecting it to this fixture. The citation is kept rather than dropped, because the causal chain is real: #262 is where the search started, not what it found. The comment also said the sibling failed "twice across 13 full-suite runs" while the sibling's own comment, unchanged, says "once across four concurrent full-suite runs". Both are true and neither was wrong: they are two measurement passes, the first while #323 was open and the second counting captured logs recovered later. Nothing said so, which left two comments appearing to disagree about the same event.
1 parent d9db60a commit ff88a5f

1 file changed

Lines changed: 21 additions & 9 deletions

File tree

packages/cli/test/vale-run.test.ts

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -259,17 +259,29 @@ withVale("runVale against the real binary", () => {
259259

260260
it("terminates and reports a timeout rather than hanging", async () => {
261261
// THE BUDGET AND THE INPUT ARE BOTH LOAD-BEARING. This asserted the winner
262-
// of a race until taskless/cli#262 work looked at it: a 1ms budget against
263-
// a one-line document, on the stated grounds that "1ms cannot survive
264-
// process startup". That is not something the test controls. Vale runs in
265-
// its OWN process and does not care whether our event loop is free, so
266-
// under load the timer's callback is delayed while the child keeps going,
267-
// and the run completes cleanly where the test demanded a timeout.
262+
// of a race until taskless/cli#327: a 1ms budget against a one-line
263+
// document, on the stated grounds that "1ms cannot survive process
264+
// startup". That is not something the test controls. Vale runs in its OWN
265+
// process and does not care whether our event loop is free, so under load
266+
// the timer's callback is delayed while the child keeps going, and the run
267+
// completes cleanly where the test demanded a timeout.
268+
//
269+
// NO ISSUE EVER FLAGGED THIS TEST. It was found while investigating
270+
// taskless/cli#262, which reports a different flake entirely — two
271+
// SUBPROCESS-SPAWNING tests in `error-envelope.test.ts` and
272+
// `verify-test-commands.test.ts` — and does not name this one. #262 is
273+
// where the search started, not what it found, and it remains open.
268274
//
269275
// Its sibling in `ValeRunOutcome.blocking` had the identical shape and was
270-
// MEASURED failing that way — twice across 13 full-suite runs, reporting
271-
// `status: "ok"` — before it was given a real margin. This test survived
272-
// only because its window was narrower, not because it was safe.
276+
// MEASURED failing that way, reporting `status: "ok"`, before it was given
277+
// a real margin. This test survived only because its window was narrower,
278+
// not because it was safe.
279+
//
280+
// TWO SEPARATE MEASUREMENT PASSES COUNTED THAT SIBLING, which is why the
281+
// numbers here and in its own comment below differ and neither is wrong.
282+
// The first, while #323 was open, saw it lose ONCE ACROSS FOUR concurrent
283+
// full-suite runs. The second, counting a set of captured logs recovered
284+
// later, saw TWICE ACROSS 13. Same test, same failure, different samples.
273285
//
274286
// The metric that matters is the ABSOLUTE margin (duration minus budget),
275287
// not a ratio: what has to happen is the child finishing before a delayed

0 commit comments

Comments
 (0)