Skip to content

test(cli): stop asserting the winner of a 1ms race in the Vale timeout test #320

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
thecodedrift merged 1 commit into from
Sep 9, 2026
Merged

test(cli): stop asserting the winner of a 1ms race in the Vale timeout test #320

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 20 additions & 2 deletions packages/cli/test/vale-run.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -431,14 +431,32 @@ withVale("ValeRunOutcome.blocking against the real binary", () => {
// Vale was present and asked to work. Reporting this as a skip would let a
// broken rule file read as "no Vale findings" — indistinguishable from a
// clean run, and how a silently disabled engine ships.
//
// THE BUDGET AND THE INPUT ARE BOTH LOAD-BEARING, and an earlier version
// of this test got it wrong. It gave a 1ms budget to a one-line document,
// which asserts the winner of a race: the timer has to fire before a child
// that runs in its OWN process and does not care whether our event loop is
// free. Measured, that document takes Vale about 46ms, so 1ms normally
// wins — but under load the timer's callback is delayed while the child
// keeps going, and it was seen losing once across four concurrent
// full-suite runs, reporting a clean "ok" where the test demanded a
// "timeout".
//
// The race is removed by making the work outlast the budget by a margin
// nothing plausible closes. Vale is QUADRATIC in the size of a single
// file — measured on the pinned binary at 80KB 0.3s, 160KB 0.9s, 320KB
// 3.5s, 640KB 14s — so roughly 320KB of prose takes about 3.5 SECONDS
// against a 100ms budget. That is a 35x margin the right way round, where
// the old one was a 46x margin the wrong way. The run is killed at 100ms,
// so the test costs about that rather than 3.5s.
const cwd = makeProject(
`${header}\n[*.md]\nno-simply.no-simply = YES\n`,
{ "no-simply": existenceRule("simply", "Avoid 'simply'") },
{ "doc.md": "Just simply do it.\n" }
{ "doc.md": `${"Just simply do it. ".repeat(17_000)}\n` }
);

expect(
await runVale({ cwd, paths: ["doc.md"], timeoutMs: 1 })
await runVale({ cwd, paths: ["doc.md"], timeoutMs: 100 })

@claude claude Bot Sep 8, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[New] Minor, not blocking: the fix's margin (100ms budget vs. ~3.5s measured real runtime) is what makes this deterministic, but that margin is measured on one pinned binary/host. It only has to survive down to roughly 3x for the timeout to still win reliably; anything that shrinks the ~35x margin toward that — a materially faster CI runner, a future Vale binary bump that improves the quadratic constant, or running this in a lower-resource sandbox where process startup itself eats a chunk of the 100ms — narrows the safety margin rather than eliminating the race outright. Given the PR's own verification (mutation check + 8 concurrent copies) already stress-tested this, I'd treat this as informational rather than something to act on now — just flagging it as the one axis that could reintroduce flakiness later if the constants drift.

).toMatchObject({ status: "timeout", blocking: true });
});

Expand Down
Loading