Skip to content

Commit 0ff26ac

Browse files
committed
test(cli): pay the oclif cold load at module scope, not in a 10s hook nobody chose
`envelope-unwrap.test.ts` loaded oclif's `Config.load({ root: CLI_ROOT })` inside a `beforeAll`, where vitest's DEFAULT `hookTimeout` of 10000ms judged it. `packages/cli/vitest.config.ts` sets no timeout key at all — by the declared design in its own header — so the budget is vitest's default and nobody in this package chose it. Measured on a 4-vCPU container, n=5 per row, the call itself: idle 3934 / 3962 / 4272 / 4451 / 4469 ms 4 spinners on 4 vCPU 7843 / 7981 / 8585 / 8782 / 9011 ms 39-45% of the budget on an IDLE box, and as little as 989 ms of margin on a box that cannot even reach the load a merge-queue shard applies. A budget a real cost approaches to within a second is not a budget — it is a load sensor, and what it senses is reported as "this file failed". The remedy is not a bigger number: widening the window around the cost relocates the cliff to the next heavier shard. It is the repo's own stated convention — "clocked windows measure behaviour, never loading" (AGENTS.md, Build & Test) — so the load moves OUT of every clocked window and is paid by a module-scope `await`, during COLLECTION. Verified against the runner this tree installs, not recalled: in `@vitest/runner@4.1.11` `withTimeout(...)` wraps exactly the hooks and the test bodies, while `collectTests()` awaits `runner.importFile(filepath, 'collect')` bare. Same file, same box, before -> after: before Duration 4.59s (import 481ms, tests 3.95s) 11 tests after Duration 4.40s (import 4.20s, tests 36ms) 12 tests The cost did not shrink and was never meant to; it left the clocked region. A pin holds the placement, because the identical prose warning one package over ("do not add one back") is a comment nobody asserts. It is a SOURCE assertion on purpose: the behavioural instrument the prior art used — `vitest run --hookTimeout=1`, green iff no hook time is left to clock — is INERT in this package. Measured: a probe `beforeAll` sleeping 500ms passes under `--hookTimeout=1` in `packages/cli` with and without `--project`, while the identical probe under `@objectstack/plugin-dev` (no `test.projects`) fails with `Hook timed out in 1ms`. Tier unchanged, measured both ways: `unit` 11 -> 12 entries for this file, `integration` 0 -> 0, against a lit integration list of 413 entries. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DvvamiacK328idtBYJBxV3
1 parent e2050ce commit 0ff26ac

1 file changed

Lines changed: 85 additions & 6 deletions

File tree

‎packages/cli/src/commands/datasource/envelope-unwrap.test.ts‎

Lines changed: 85 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27,15 +27,57 @@
2727
* construction. Typing this file's payloads out by hand would repeat the exact
2828
* mistake under repair: a copy of a server shape that stays self-consistent
2929
* while the server moves.
30+
*
31+
* ## Why the oclif `Config` is loaded at MODULE SCOPE (#18748)
32+
*
33+
* `Config.load({ root: CLI_ROOT })` used to sit in a `beforeAll`, where it was
34+
* judged by vitest's DEFAULT `hookTimeout` of 10000ms -- a budget nobody in
35+
* this package chose (`packages/cli/vitest.config.ts` sets no timeout key at
36+
* all, by the declared design in its own header). Measured on the 4-vCPU
37+
* container this change was made on, n=5 per row, the call itself:
38+
*
39+
* idle 3934 / 3962 / 4272 / 4451 / 4469 ms
40+
* 4 spinners on 4 vCPU 7843 / 7981 / 8585 / 8782 / 9011 ms
41+
*
42+
* So an ordinary idle run already spends 39-45% of that budget, and a box that
43+
* cannot even reach the load a merge-queue shard applies leaves as little as
44+
* **989 ms** of margin. A budget a real cost approaches to within a second is
45+
* not a budget -- it is a LOAD SENSOR, and what it senses is how busy the
46+
* runner is, reported as "this file failed".
47+
*
48+
* ⛔ The answer is NOT a bigger number. Widening the window around the cost
49+
* relocates the cliff to the next heavier shard; the merge queue runs the FULL
50+
* suite where PR-side CI runs only the affected subset, so the queue shard is
51+
* heavier than anything a PR check measures, and it is where this class has
52+
* already ejected green PRs belonging to other people.
53+
*
54+
* The answer is to take the cost OUT of every clocked window, which is the
55+
* repo's own stated convention -- "clocked windows measure behaviour, never
56+
* loading" (AGENTS.md, Build & Test), the same move `check:test-source-alias`
57+
* prescribes for a cold dependency load and the same one
58+
* `plugins/plugin-dev/src/dev-plugin-security-enforcement-warning.test.ts`
59+
* records paying twice. A module-scope `await` is paid during COLLECTION, and
60+
* collection is clocked against NOTHING. Verified against the runner this tree
61+
* installs rather than recalled: in `@vitest/runner@4.1.11`, `withTimeout(...)`
62+
* wraps exactly the hooks and the test bodies, while `collectTests()` awaits
63+
* `runner.importFile(filepath, 'collect')` bare; and `vitest --help` on 4.1.11
64+
* offers exactly three timeout knobs (`testTimeout`, `hookTimeout`,
65+
* `teardownTimeout`), none of which covers module loading.
66+
*
67+
* ⛔ Do not move this back into a hook, and do not answer a recurrence by
68+
* raising a timeout. The last section of this file pins the placement so that
69+
* "do not" is an assertion rather than a sentence nobody reads.
3070
*/
3171

32-
import { beforeAll, afterEach, describe, expect, it, vi } from 'vitest';
72+
import { afterEach, describe, expect, it, vi } from 'vitest';
3373
import { Config } from '@oclif/core';
3474
import type { Command } from '@oclif/core';
3575
import { sendError, sendOk } from '@objectstack/types';
3676
import type { RemoteTable, SchemaValidationResult } from '@objectstack/spec/contracts';
77+
import { readFileSync } from 'node:fs';
3778
import { dirname, resolve } from 'node:path';
3879
import { fileURLToPath } from 'node:url';
80+
import { maskCommentsAndLiterals } from '../../../../../scripts/js-comment-mask.mjs';
3981
import { serverBody } from '../../utils/__tests__/server-body.js';
4082
import DatasourceIntrospect from './introspect.js';
4183
import DatasourceListTables from './list-tables.js';
@@ -75,11 +117,12 @@ const CLEAN_RESULT: SchemaValidationResult = {
75117
/** The silent pass this card exists to make impossible. */
76118
const SILENT_PASS = 'No federated objects to validate.';
77119

78-
let config: Config;
79-
80-
beforeAll(async () => {
81-
config = await Config.load({ root: CLI_ROOT });
82-
});
120+
/**
121+
* Paid HERE, at module scope, and not in a hook -- see "Why the oclif `Config`
122+
* is loaded at MODULE SCOPE" in this file's header for the measured legs and
123+
* for the runner reading that says collection is the one unclocked phase.
124+
*/
125+
const config = await Config.load({ root: CLI_ROOT });
83126

84127
afterEach(() => {
85128
vi.unstubAllGlobals();
@@ -292,3 +335,39 @@ describe('os datasource introspect', () => {
292335
expect(run.failure?.message).not.toContain('first argument must be a string');
293336
});
294337
});
338+
339+
/**
340+
* The placement above, pinned (#18748).
341+
*
342+
* ⚠️ This is a SOURCE assertion on purpose, and it is the only shape available
343+
* here. The behavioural instrument the sibling prior art used --
344+
* `vitest run --hookTimeout=1`, green iff no hook time is left to clock -- is
345+
* INERT in this package, measured rather than assumed: a probe `beforeAll`
346+
* sleeping 500ms passes under `--hookTimeout=1` in `packages/cli` (with and
347+
* without `--project`), while the identical probe under `@objectstack/plugin-
348+
* dev` -- which declares no `test.projects` -- fails with `Hook timed out in
349+
* 1ms`. A CLI timeout override does not reach a project-level config on
350+
* vitest 4.1.11, so in this package that flag cannot witness anything.
351+
*
352+
* What is left to assert is the structural fact the measurement stands on: the
353+
* cold load has no clocked window around it. A regression puts `Config.load`
354+
* back inside a hook or a test body, and both halves of that show up here.
355+
*/
356+
describe('#18748 the oclif cold load stays outside every clocked window', () => {
357+
it('pays `Config.load` at module scope, leaving no hook to clock it', () => {
358+
// Comment AND literal spans blanked: this file's prose discusses the very
359+
// spellings being searched for, and so do the regex bodies just below, so
360+
// a bare-text scan would match itself and pass on its own commentary.
361+
const code = maskCommentsAndLiterals(readFileSync(fileURLToPath(import.meta.url), 'utf8'));
362+
363+
// Exactly one call site, and it opens its own line -- i.e. it is nested in
364+
// no function body, which is what "paid during collection" reduces to.
365+
expect(code.match(/Config\.load\s*\(/g) ?? []).toHaveLength(1);
366+
expect(code).toMatch(/^const\s+config\s*=\s*await\s+Config\.load\s*\(/m);
367+
368+
// ⛔ No hook may come back to carry it. The `afterEach` this file does keep
369+
// is a synchronous `vi.unstubAllGlobals()` and loads nothing.
370+
expect(code).not.toMatch(/\bbeforeAll\s*\(/);
371+
expect(code).not.toMatch(/\bbeforeEach\s*\(/);
372+
});
373+
});

0 commit comments

Comments
 (0)