Skip to content

Commit 7ef8159

Browse files
committed
test(plugin-dev): pay the plugin-security transform at module load, not in a clocked window
The `beforeAll` warm-up only moved the one-off `@objectstack/plugin-security` transform from the 5000ms `testTimeout` onto the 10000ms `hookTimeout`. The merge queue runs the FULL suite where PR-side CI runs only the affected subset, and on that heavier shard the hook itself blew its budget — ejecting the PR four times with `Error: Hook timed out in 10000ms.` at this file's `beforeAll`. Replace the hook with a top-level side-effect import. Collection is the one phase vitest 4.1.10 clocks against nothing: `@vitest/runner` wraps only hooks and test bodies in `withTimeout(...)`, `collectTests()` awaits `runner.importFile(filepath, 'collect')` bare and merely records `collectDuration`, and the runner exposes exactly three timeout knobs (`testTimeout`, `hookTimeout`, `teardownTimeout`), none covering module load. Under an identical single-core load the file's `tests` phase drops from 7.22s to 67ms and the suite stays green even with `--hookTimeout=1`. All four #10036 assertions are untouched, and the real plugin is still imported for real. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PnJHU45vPJj5UQrxe946Bx
1 parent e0ac7ef commit 7ef8159

1 file changed

Lines changed: 50 additions & 33 deletions

File tree

packages/plugins/plugin-dev/src/dev-plugin-security-enforcement-warning.test.ts

Lines changed: 50 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,55 @@
1-
import { describe, it, expect, vi, beforeAll } from 'vitest';
1+
import { describe, it, expect, vi } from 'vitest';
22
import { DevPlugin } from './dev-plugin';
33

4+
// [#10115] Pay the one-off `@objectstack/plugin-security` module-graph cost at
5+
// MODULE LOAD, not inside any hook and not inside any test.
6+
//
7+
// `DevPlugin.start()` reaches the plugin through a dynamic `await import()`, and
8+
// this file deliberately leaves that chain unmocked (see the header below: the
9+
// real plugin's `init()`/`start()` phase split IS the subject). Something has to
10+
// pay its cold vite transform; the only question is which clock is running when
11+
// it does. This file has now answered that question wrongly twice:
12+
//
13+
// * paid inside whichever `it` ran first -> `Test timed out in 5000ms`
14+
// (idle 3110/3351/3360 ms; red on every run under four concurrent builds).
15+
// * moved into a `beforeAll` -> `Hook timed out in 10000ms`,
16+
// which ejected this file from the merge queue four times in one night
17+
// (runs 32334616926 / 32334642055 / 32334745861 / 32335141663, shard
18+
// `Test Core (3/3)`, file duration 10024ms) while PR-side CI stayed green --
19+
// the queue runs the FULL suite, PR-side CI only the affected subset, so the
20+
// queue shard is far heavier than anything the PR checks measure.
21+
//
22+
// Neither move took the cost OUT of a clocked window; each only widened or
23+
// swapped the window around it, which relocates the cliff to the next heavier
24+
// shard instead of removing it. A top-level import is paid during collection,
25+
// and in vitest 4.1.10 collection is clocked against NOTHING. Verified against
26+
// the installed runner, not recalled: `@vitest/runner` wraps exactly hooks and
27+
// test bodies in `withTimeout(...)`, while `collectTests()` awaits
28+
// `runner.importFile(filepath, 'collect')` bare and merely RECORDS
29+
// `file.collectDuration` for reporters; and `vitest --help` on 4.1.10 offers
30+
// exactly three timeout knobs -- `testTimeout`, `hookTimeout`, `teardownTimeout`
31+
// -- none of which covers module loading.
32+
//
33+
// Measured on a 4-vCPU container with this run confined to a single core and a
34+
// spinner beside it (idle -> loaded): the old `beforeAll` cost 3.3s -> 7.2-7.4s,
35+
// i.e. 74% of its 10000ms budget on a machine that could not even reach the
36+
// load the queue applies. After this change the file has NO hook at all, its
37+
// four tests cost 2-70 ms each against the 5000ms `testTimeout`, and the run
38+
// stays green even under `--hookTimeout=1` -- there is no hook time left to clock.
39+
//
40+
// `vi.mock` is hoisted above every import in this file, this one included, so
41+
// the ten mocks below still register before this module is evaluated.
42+
//
43+
// This must stay a REAL, STATIC, side-effect import of the REAL plugin:
44+
// - replacing it with a stub, here or via `vi.mock`, deletes the subject
45+
// exactly as the header warns;
46+
// - turning it back into a hook or a dynamic `await import()` puts the cost
47+
// back inside a clocked window and re-arms the ejection;
48+
// - answering a recurrence by raising `testTimeout` / `hookTimeout` widens the
49+
// window around the cost rather than moving the cost out of it, and re-hides
50+
// the next transform that lands in this file.
51+
import '@objectstack/plugin-security';
52+
453
// [#10036] The state under test is "SecurityPlugin LOADED but its start()
554
// bailed", so `@objectstack/plugin-security` is deliberately NOT mocked here —
655
// the real plugin's real `init()`/`start()` phase split is what constructs the
@@ -66,38 +115,6 @@ async function boot(ctx: any, options: Record<string, unknown> = {}) {
66115
return plugin;
67116
}
68117

69-
// [#10115] Pay the one-off module-graph cost HERE, before the first `it`.
70-
//
71-
// `DevPlugin.start()` reaches `@objectstack/plugin-security` through a dynamic
72-
// `await import()`, and this file deliberately leaves that chain unmocked (see
73-
// the header: the real plugin's `init()`/`start()` phase split IS the subject).
74-
// So whichever test ran first paid the plugin's cold vite transform inside its
75-
// own measured window. Measured on an idle 4-vCPU container, bail #1 cost
76-
// 3110 / 3351 / 3360 ms — ~70% of vitest's 5000ms default `testTimeout` — while
77-
// the other three tests cost 3-5 ms each. Under four concurrent tsup DTS builds
78-
// of plugin-dev dependents it crossed the budget on every run
79-
// (5006 / 5007 / 5006 ms, `Test timed out in 5000ms`), and bail #2 then
80-
// inherited the still-cold import (2203-2931 ms) and went red with it on the
81-
// busier CI shard. Because a PR that dirties a plugin-dev dependency both makes
82-
// this file re-run AND makes it run beside a cold rebuild, the failure was
83-
// intermittent and named neither the real cause nor the offending PR — it read
84-
// as "your change broke security enforcement" when nothing of the sort happened.
85-
//
86-
// Warming the import here moves that one-off transform off every test's clock
87-
// and onto vitest's separate `hookTimeout` (default 10000ms — twice the test
88-
// budget), so each `it` measures only the behaviour it is about. Same machine,
89-
// same load, after: bail #1 22-26 ms idle and 29-54 ms under the same four-build
90-
// load, all four tests green.
91-
//
92-
// ⛔ This must stay a REAL import of the REAL plugin — replacing it with a stub,
93-
// here or via `vi.mock`, deletes the subject exactly as the header warns.
94-
// ⛔ Do not answer a future recurrence by raising `testTimeout` instead: that
95-
// widens the window around the cost rather than moving the cost out of it, and
96-
// it re-hides the next in-test transform that lands in this file.
97-
beforeAll(async () => {
98-
await import('@objectstack/plugin-security');
99-
});
100-
101118
describe('[#10036] the "nothing is enforced" warning must fire when SecurityPlugin.start() bailed', () => {
102119
// ── The state the warning describes, constructed for real ───────────────
103120
//

0 commit comments

Comments
 (0)