Skip to content

Commit 7097859

Browse files
test: load package barrels at module top so the collect phase pays for them (#21480)
Fixes #21457 Clause-②: no ## What this changes Five test files, test-only. Every case that loaded a package barrel with a dynamic import inside its body now gets that barrel from a module-top load, so the load is charged to vitest's collect phase instead of the case's 5000ms window (AGENTS.md: 「Clocked windows measure behaviour, never loading」). The shape and the reasoning are the core precedent's, `packages/core/src/service-resolution-discriminator.contract.test.ts`; no helper was added. No timeout, retry, skip, `vitest.config.ts`, source or gate change. | site (line numbers at `7e7e64b1`) | change | |---|---| | `packages/objectql/src/system-write-organization.test.ts:506` | `import * as barrel from './index.js'` at module top; the case is now synchronous and still asserts, by identity, that both names come from the barrel | | `packages/core/src/security/platform-admin.test.ts:279` | `import * as securityEntry from './index.js'` at module top; the case still probes presence and absence on the security entry | | `packages/lint/src/validate-predicate-path-refs.test.ts:376` | `import * as barrel from './index.js'` at module top | | `packages/plugins/plugin-sharing/src/translations/serving-seam.test.ts:86` and `:100` | one static import plus one module-top re-evaluation under a mock, see below | | `packages/lint/src/lazy-deps.test.ts:190` | the triage's named exception: keeps its in-case import, and one comment line now says why | ### The one site where the hoist was not mechanical: `serving-seam.test.ts` Hoisting changed what this file observes, so it was not a plain move. The `:86` case evaluated the barrel against a MOVED source: a `beforeEach` ran `vi.resetModules()`, then the case ran `vi.doMock('./en.objects.generated.js', ...)` and imported the barrel. A static import cannot be mocked per case. The resolution keeps each assertion's meaning: - `:100` ("substitutes nothing while the source has not moved") reads a static `import { SharingTranslations } from './index.js'`, which is the barrel as committed. - `:86` ("SERVES the current source when the source moves under the recorded leaf") reads `servedAfterSourceMoved`. That value is produced at module top by the same four steps the `beforeEach` and the case used to perform: `vi.resetModules()`, `vi.doMock(...)`, `await import('./index.js')`, `vi.doUnmock(...)`. A top-level await in a test module runs during collection; the repo already uses that shape in `packages/cli/test/score-lint-crash.test.ts` and `packages/mcp/src/__tests__/plugin-execution-context.test.ts`. - The `beforeEach` is removed. It existed only to isolate the two in-case imports, and no case imports anything now. So one `await import('./index.js')` remains in that file, at column 0 at module top and not inside a case. The triage pin is about loads inside a case. **Ablation** (one-time, on the committed tree, `scripts/ablation-replace.mjs` in WRAP mode with its own restore proof): deleting the module-top `vi.resetModules();` turns exactly the `:86` case red, `AssertionError: expected 'Token' to be 'Share token'`, with 1 failed and 4 passed. Without the reset, the import returns the copy the static import cached. The restore was proven: blob `9d8e14d9fe22` equals HEAD's, and `git diff HEAD` is empty. ## Measured Every reading was taken on this container's shared box. Other agents' work ran alongside, and the verify lock excluded only other locked runs, so the absolute numbers are shared-box seconds. The signal is the per-case drop. **Per-case duration, run alone** (one file plus `-t`, vitest JSON reporter; the number in brackets is the same case in a whole-file run): | case | before (`7e7e64b1`) | after (`8e8b3bb6`) | |---|---|---| | objectql barrel case | 1669 ms (1556) | 2.4 ms (0.1) | | core security-entry case | 1695 ms (1837) | 2.1 ms (0.1) | | lint predicate barrel case | 57 ms (68) | 5.2 ms (0.4) | | sharing `:86` | 107 ms (134) | 1.5 ms (0.3) | | sharing `:100` | 114 ms (57) | 1.4 ms (0.2) | | lint lazy-deps in-process case (the exception, unchanged) | 8372 ms | 8296 ms | **In the package's whole suite.** CI's Test Core runs each of these four packages whole on its shard; only `@objectstack/cli` takes file-level slices. So the whole-suite run is their shard. The readings are on `2d2204d764`; for the before leg, the edited file(s) were restored tree-only from `7e7e64b1`. | case | before | after | suite, both legs | |---|---|---|---| | objectql (`--project local`) | 614 ms | 0.1 ms | 366 files, 7383 tests passed | | core (both projects) | 313 ms | 0.3 ms | 79 files, 2204 tests passed | | lint predicate | 34 ms | 0.2 ms | 119 files, 5619 tests passed | | lint lazy-deps (the exception) | 5476 ms | 5544 ms | same run | | sharing `:86` / `:100` | 99 / 56 ms | 0.2 / 0.2 ms | 38 files, 954 tests passed | The load did not disappear. It moved into the collect phase, which vitest 4's summary calls "import". For example, objectql's single-case run went from import 7.84s and tests 1.67s to import 10.02s and tests 4ms. Nobody had measured the other five sites before this. At these readings, only the objectql and core cases carried a load large enough to matter (1.5 to 1.8 s alone). The lazy-deps case is not under the default window at all: it already runs under `COLD_LOAD_TIMEOUT_MS = 30_000`, and it measures 5.5 to 8.4 s. ## Verification - `pnpm test` per package on `2d2204d764`: core 76 files / 2156 tests, lint 119 / 5619, plugin-sharing 38 / 954, all passed. objectql `vitest run --project local`, which is its `test` script: 366 / 7383 passed. - `pnpm typecheck` for all four packages: exit 0, and `check:test-typecheck` OK in each. All five files are in their package's `tsconfig.test.json` program (`tsc --listFiles`), and none of them carries an error. - Gates: `dispatch-gates --commands` derived 63 commands from this tree, the same list as the PM's derivation. All 63 were run and all exit 0. `check:i18n` and `check:dual-build-cjs-loads` first answered PREREQUISITE NOT MET (exit 3); after a turbo cache replay of their build closure, both re-ran green. The `--ran` reconciliation: 63 derived, 63 run, 0 NOT-MEASURED, 0 UNRUN. - ESLint, narrowed with the narrowing proven: `eslint --no-inline-config --format json` over the five files on `2d2204d764` linted 5 files (none reported as ignored), with 0 errors and 0 warnings. `eslint.config.mjs` enables no type-aware linting (no `parserOptions.project`, no `projectService`), so this diff cannot change a verdict on any file it does not touch. The repo-wide `pnpm lint` is CI's. - Pin grep, `git grep -n "await import('\./index" -- 'packages/**/*.test.ts' ':!packages/spec'`, gives 3 hits: the core precedent's own comment, `lazy-deps.test.ts:191` (the named exception) and `serving-seam.test.ts:84` (at module top). The spec side, as the control, gives 45 hits. - `check:test-source-alias` does not flag this shape. Per the triage ruling, that is recorded here and the gate is not widened. It is green on this tree. ## Acceptance notes - **Same family, other spellings.** These sites are outside this card's ruled pattern and outside its claimed files. Each is an in-case barrel load, measured alone on `2d2204d764`: - `packages/services/service-analytics/src/__tests__/dataset-compare-kind-refusal.test.ts:220`, `../index.js`: 568 ms; - `packages/plugins/plugin-email/src/send-template.test.ts:336` and `:342`, `./transports/index.js`: 37 ms and 1 ms; - `packages/metadata/src/loaders/overlay-index-single-producer.test.ts:150`, `../migrations/index.js`: 24 ms. None is near the window. They are noted here and not filed. - **Base:** the branch was cut at `7e7e64b1` and merged with `origin/main` at `49524f6906`. --- _Generated by [Claude Code](https://claude.ai/code/session_01DDZNkDVwPQnevTFcYE47H3)_ Co-authored-by: Claude <noreply@anthropic.com>
1 parent cc07862 commit 7097859

5 files changed

Lines changed: 64 additions & 19 deletions

File tree

‎packages/core/src/security/platform-admin.test.ts‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,17 @@ import {
2323
setPlatformAdminConfigSink,
2424
type PlatformAdminConfigSink,
2525
} from './platform-admin.js';
26+
// The security entry is loaded HERE, at module top, and not by a dynamic import
27+
// inside the case that probes it below. That import loads the entry's whole
28+
// re-export graph (signatures, sandbox, permission enforcement), and inside a
29+
// case the load is charged to vitest's per-case budget (5000ms by default), so
30+
// the pin's verdict would turn on how loaded the machine is. At module top the
31+
// same load is charged to the COLLECT phase, where no per-case budget applies.
32+
// The case loses nothing: it asserts export presence and absence only, and the
33+
// entry only re-exports, so loading it earlier observes no env this file's
34+
// hooks set or clear. Same reasoning as the core precedent
35+
// `packages/core/src/service-resolution-discriminator.contract.test.ts`.
36+
import * as securityEntry from './index.js';
2637

2738
const ENV = 'OS_PLATFORM_OWNER_EMAIL';
2839

@@ -275,8 +286,8 @@ describe('[#11663 L5] the legacy-grant deprecation pointer is retired', () => {
275286
expect('resolvePlatformAdminEmails' in mod).toBe(true);
276287
});
277288

278-
it("⛔ nor by `@objectstack/core`'s security entry — the published surface lost them", async () => {
279-
const entry: Record<string, unknown> = await import('./index.js');
289+
it("⛔ nor by `@objectstack/core`'s security entry — the published surface lost them", () => {
290+
const entry: Record<string, unknown> = securityEntry;
280291
expect('reportLegacyPlatformAdminGrant' in entry).toBe(false);
281292
expect('resetLegacyPlatformAdminGrantReport' in entry).toBe(false);
282293
// Positive control on the same entry: its sibling config exports are still

‎packages/lint/src/lazy-deps.test.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,7 @@ describe('lazy dependency loading (kernel boot-path contract)', () => {
180180

181181
it('loads each dep lazily in-process and the gates still work', async () => {
182182
const req = createRequire(import.meta.url);
183+
// Kept in-case on purpose: loading the barrel IS this case's subject, budgeted by COLD_LOAD_TIMEOUT_MS.
183184
const {
184185
validateReactPages,
185186
validateReactPageProps,

‎packages/lint/src/validate-predicate-path-refs.test.ts‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,17 @@ import {
2626
PREDICATE_RHS_PATH_SHAPED,
2727
} from './validate-predicate-path-refs.js';
2828
import { AUTHORING_RULES } from './authoring-rules.js';
29+
// The published barrel is loaded HERE, at module top, and not by a dynamic
30+
// import inside the case that reads the id off it. That import loads the whole
31+
// `@objectstack/lint` index graph, and inside a case the load is charged to
32+
// vitest's per-case budget (5000ms by default), so the case's verdict would
33+
// turn on how loaded the machine is. At module top the same load is charged to
34+
// the COLLECT phase, where no per-case budget applies. The case loses nothing:
35+
// it asserts an id's value and identity only. `lazy-deps.test.ts` is the one
36+
// file here that keeps its barrel load inside a case, because that load is its
37+
// subject. Same reasoning as the core precedent
38+
// `packages/core/src/service-resolution-discriminator.contract.test.ts`.
39+
import * as barrel from './index.js';
2940

3041
// ── A miniature target schema, so the traversal is pinned against a shape the
3142
// test fully controls rather than against whatever `FieldSchema` happens to
@@ -372,8 +383,7 @@ describe('validatePredicatePathRefs — path-shaped right-hand side (#7659)', ()
372383
expect(rhs('data.name == $b')).toEqual([]);
373384
});
374385

375-
it('emits the id the published barrel exports', async () => {
376-
const barrel = await import('./index.js');
386+
it('emits the id the published barrel exports', () => {
377387
expect(barrel.PREDICATE_RHS_PATH_SHAPED).toBe('predicate-rhs-path-shaped');
378388
expect(rhs('data.name == data.type')[0].rule).toBe(barrel.PREDICATE_RHS_PATH_SHAPED);
379389
});

‎packages/objectql/src/system-write-organization.test.ts‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,17 @@ import {
4444
SystemWriteOrganizationRequiredError,
4545
SYSTEM_WRITE_ORGANIZATION_REQUIRED_CODE,
4646
} from './tenancy/system-write-organization.js';
47+
// The package barrel is loaded HERE, at module top, and not by a dynamic import
48+
// inside the barrel case at the foot of this file. That import loads the whole
49+
// `@objectstack/objectql` index graph, and inside a case the load is charged to
50+
// vitest's per-case budget (5000ms by default): the case was measured timing
51+
// out on a loaded box while its assertions held, so its verdict was a function
52+
// of the machine rather than of the code. At module top the same load is
53+
// charged to the COLLECT phase, where no per-case budget applies. The case
54+
// loses nothing: it asserts symbol identity only, and the barrel itself only
55+
// re-exports. Same reasoning, and the same shape, as the core precedent
56+
// `packages/core/src/service-resolution-discriminator.contract.test.ts`.
57+
import * as barrel from './index.js';
4758

4859
const ORG_ID = 'org_msokm9oaz0cal87q';
4960
const SECOND_ORG_ID = 'org_second';
@@ -499,11 +510,11 @@ describe('#14936 the published recognizer for the org-less system-write refusal'
499510
expect(pinned).toBe(SYSTEM_WRITE_ORGANIZATION_REQUIRED_CODE);
500511
});
501512

502-
it('publishes both names from the package BARREL, not only from the module', async () => {
513+
it('publishes both names from the package BARREL, not only from the module', () => {
503514
// The card's landing surface is "the module plus that package's index.ts
504515
// export" - a consumer reaches these by bare specifier, so an export that
505516
// exists only on the deep module is not the affordance that was asked for.
506-
const barrel = await import('./index.js');
517+
// `barrel` is the module-top import (see the imports for why it is there).
507518
expect(barrel.SYSTEM_WRITE_ORGANIZATION_REQUIRED_CODE).toBe(SYSTEM_WRITE_ORGANIZATION_REQUIRED_CODE);
508519
expect(barrel.isSystemWriteOrganizationRequiredError).toBe(isSystemWriteOrganizationRequiredError);
509520
});

‎packages/plugins/plugin-sharing/src/translations/serving-seam.test.ts‎

Lines changed: 25 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -35,11 +35,20 @@
3535
// the seam is wired. The source has to be MOVED for the two to differ, which is
3636
// what the mock below does.
3737

38-
import { describe, it, expect, vi, beforeEach } from 'vitest';
38+
import { describe, it, expect, vi } from 'vitest';
3939
import { withSourceFallback, findStaleFills } from '@objectstack/platform-objects/apps';
4040
import { enObjects } from './en.objects.generated.js';
4141
import { esESObjects } from './es-ES.objects.generated.js';
4242
import { esESGeneratedSourceHashes } from './es-ES.source-hashes.generated.js';
43+
// The barrel as committed, loaded HERE, at module top. Both of this file's
44+
// barrel loads happen at module top and neither inside a case: a fresh load of
45+
// the barrel and its bundles inside a case is charged to vitest's per-case
46+
// budget (5000ms by default), so the verdict would turn on how loaded the
47+
// machine is, while at module top the same load is charged to the COLLECT
48+
// phase, where no per-case budget applies. The second load, against a moved
49+
// source, is below `revisedSource`. Same reasoning as the core precedent
50+
// `packages/core/src/service-resolution-discriminator.contract.test.ts`.
51+
import { SharingTranslations } from './index.js';
4352

4453
/** The leaf the gap was measured on: recorded in es-ES only. */
4554
const PATH = ['objects', 'sys_share_link', 'fields', 'token', 'label'] as const;
@@ -64,12 +73,18 @@ function revisedObjects() {
6473
/** The same thing as a `TranslationData` — what the barrel passes as `source`. */
6574
const revisedSource = () => ({ objects: revisedObjects() });
6675

67-
describe('SharingTranslations — the provenance companion is read at serving time', () => {
68-
beforeEach(() => {
69-
vi.resetModules();
70-
vi.doUnmock('./en.objects.generated.js');
71-
});
76+
// The SAME barrel, evaluated a second time with the source string behind the
77+
// recorded leaf revised. This setup has to run before the load it shapes, which
78+
// is why it sits here at module top rather than in a hook: `resetModules` drops
79+
// the copy the static import above cached, so the import below evaluates the
80+
// barrel afresh against the mock, and `doUnmock` takes the mock straight back
81+
// out, so the static bindings and every case see the committed bundles.
82+
vi.resetModules();
83+
vi.doMock('./en.objects.generated.js', () => ({ enObjects: revisedObjects() }));
84+
const { SharingTranslations: servedAfterSourceMoved } = await import('./index.js');
85+
vi.doUnmock('./en.objects.generated.js');
7286

87+
describe('SharingTranslations — the provenance companion is read at serving time', () => {
7388
it('the leaf under test is recorded in es-ES and is a byte copy of the current source', () => {
7489
const path = PATH.join('.');
7590
expect(esESGeneratedSourceHashes[path]).toBeTypeOf('string');
@@ -81,23 +96,20 @@ describe('SharingTranslations — the provenance companion is read at serving ti
8196
expect(stale.map((s) => s.path)).toEqual([PATH.join('.')]);
8297
});
8398

84-
it('SERVES the current source when the source moves under the recorded leaf', async () => {
85-
vi.doMock('./en.objects.generated.js', () => ({ enObjects: revisedObjects() }));
86-
const { SharingTranslations } = await import('./index.js');
87-
expect(read(SharingTranslations['es-ES'])).toBe(REVISED);
99+
it('SERVES the current source when the source moves under the recorded leaf', () => {
100+
expect(read(servedAfterSourceMoved['es-ES'])).toBe(REVISED);
88101
// The locales with no record for this path are legacy-trusted and untouched —
89102
// recovery is per-locale, which is the half of ruling #8765 Option B that a
90103
// blanket "fall back to source" would have destroyed.
91-
expect(read(SharingTranslations['zh-CN'])).toBe('令牌');
104+
expect(read(servedAfterSourceMoved['zh-CN'])).toBe('令牌');
92105
});
93106

94107
it('NEGATIVE CONTROL: the same bundle with no companion serves the superseded draft', () => {
95108
const unserved = withSourceFallback({ objects: esESObjects }, revisedSource(), undefined, undefined);
96109
expect(read(unserved)).toBe('Token');
97110
});
98111

99-
it('substitutes nothing while the source has not moved', async () => {
100-
const { SharingTranslations } = await import('./index.js');
112+
it('substitutes nothing while the source has not moved', () => {
101113
expect(read(SharingTranslations['es-ES'])).toBe(read({ objects: enObjects }));
102114
});
103115
});

0 commit comments

Comments
 (0)