From c483576b668420b8e12ffb9456440c10e83a9622 Mon Sep 17 00:00:00 2001 From: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Date: Thu, 30 Jul 2026 18:55:22 +0800 Subject: [PATCH] test: retire four raised hook timeouts by importing at module scope MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to #3010 and the §9 rule added in #3015. A sweep for the same anti-pattern — `beforeAll(async () => { await import(…) })`, which bills a cold module transform to `hookTimeout` — found 36 more test files, every single one already carrying a raised timeout, on an escalating ladder: 15s -> 30s -> 60s. That ladder is the evidence that raising the timeout never fixed anything. This takes the four highest-risk ones. Each drops the raised timeout instead of raising it again; the import phase has no test or hook timeout. - `plugin-markdown/src/index.test.ts` (was 15s) — the twin of the file fixed in #3010: same shape, same budget, same comment verbatim ("Increase timeout to 15 seconds for async import"). plugin-kanban proved that budget insufficient by blowing it at 15021ms, and this file's timed portion was already 5.83s of the same 15s. This was a live flake, not a theoretical one. - `plugin-charts/src/index.test.ts` (was 60s) — recharts-backed graph. - `plugin-editor/src/index.test.ts` (was 30s) — Monaco. - `apps/console/.../public-contract.test.ts` (was 60s) — also folds the `let contract` + hook assignment into a module-scope `const`, now that the two side-effect imports are evaluated before the module body. The console file was the delicate one: its assertions pin the eager/lazy registration split (objectui#2953), so the concern was whether moving the imports perturbs what is loaded when. It does not — static imports are evaluated in order before the module body, and the shared setup still runs before this module is imported at all. Its order-sensitive "reaches the lazily-registered blocks before their chunks load" test passes unchanged. Timed portion of each file, isolated (before -> after): plugin-markdown 5.83s -> 2ms plugin-charts 2.22s -> 2ms plugin-editor 1.78s -> 4ms public-contract 345ms -> 3ms Verified: full suite 100% green — 731 files passed / 1 skipped, 8521 tests passed / 24 skipped, zero failures. The `unit` project alone is 297/297 files and 4084/4084 tests, which also confirms no cross-file interference from the new module-scope registrations despite that project running `isolate: false`. Note for anyone baselining locally: the ~19 failures this repo can show on a long-lived worktree are a STALE INSTALL (`node_modules` @objectstack/spec 14.6.0 vs ^17.0.0-rc.0 declared), not a regression. `pnpm install --prefer-offline` clears all of them — hence the all-green run above. Already documented in §9. Co-Authored-By: Claude Opus 5 --- .../src/__tests__/public-contract.test.ts | 31 +++++++++++++------ packages/plugin-charts/src/index.test.ts | 14 +++++---- packages/plugin-editor/src/index.test.ts | 15 ++++----- packages/plugin-markdown/src/index.test.ts | 15 +++++---- 4 files changed, 46 insertions(+), 29 deletions(-) diff --git a/apps/console/src/__tests__/public-contract.test.ts b/apps/console/src/__tests__/public-contract.test.ts index dab4da7a80..4b54b0f0cf 100644 --- a/apps/console/src/__tests__/public-contract.test.ts +++ b/apps/console/src/__tests__/public-contract.test.ts @@ -25,8 +25,23 @@ * silently disappeared; an exact list makes both directions a deliberate edit. */ -import { describe, it, expect, beforeAll } from 'vitest'; +import { describe, it, expect } from 'vitest'; import { ComponentRegistry, PUBLIC_BLOCKS, type PublicComponentConfig } from '@object-ui/core'; +// The two graphs whose registrations this file reads. At module scope, NOT +// awaited inside a `beforeAll` — there their cold transform is billed to the +// hook, against `hookTimeout`, which is why this needed a 60s budget. The import +// phase has no test/hook timeout, so the raised timeout goes away rather than +// getting raised again (objectui#3010). See AGENTS.md §9 (test discipline). +// +// Loading them here rather than in a hook does not change WHAT is loaded, so the +// eager/lazy split the assertions below pin is untouched: static imports are +// evaluated in order before the module body, and the shared setup still runs +// before this module is imported at all. +// +// The layout/content primitives (Tier B) … +import '@object-ui/components'; +// … and the console's own plugin layer, from the module main.tsx boots from. +import '../register-plugins'; /** * Every curated tag the console makes available, in `PUBLIC_BLOCKS` order. @@ -109,15 +124,11 @@ const EXPECTED_LAZY = [ 'markdown', ]; -let contract: Map; - -beforeAll(async () => { - // The layout/content primitives (Tier B) … - await import('@object-ui/components'); - // … and the console's own plugin layer, from the module main.tsx boots from. - await import('../register-plugins'); - contract = new Map(ComponentRegistry.getPublicConfigs().map((c) => [c.type, c])); -}, 60000); +// Safe at module scope: the two side-effect imports at the top of this file are +// evaluated before the module body, so every registration is already in place. +const contract: Map = new Map( + ComponentRegistry.getPublicConfigs().map((c) => [c.type, c]), +); describe('console ↔ PUBLIC_BLOCKS coverage', () => { it('exposes every curated block the console ships, eager or lazy', () => { diff --git a/packages/plugin-charts/src/index.test.ts b/packages/plugin-charts/src/index.test.ts index b683c4d65b..62b76e6306 100644 --- a/packages/plugin-charts/src/index.test.ts +++ b/packages/plugin-charts/src/index.test.ts @@ -6,15 +6,17 @@ * LICENSE file in the root directory of this source tree. */ -import { describe, it, expect, beforeAll } from 'vitest'; +import { describe, it, expect } from 'vitest'; import { ComponentRegistry } from '@object-ui/core'; +// Imports all renderers to register them. Module scope, NOT awaited inside a +// `beforeAll` — there the cold transform of the (recharts-backed) renderer graph +// is billed to the hook, which is why this needed a 60s `hookTimeout` to begin +// with. The import phase has no test/hook timeout, so the raised timeout goes +// away rather than getting raised again (objectui#3010). +// See AGENTS.md §9 (test discipline). +import './index'; describe('Plugin Charts', () => { - // Import all renderers to register them - beforeAll(async () => { - await import('./index'); - }, 60000); - describe('bar-chart component', () => { it('should be registered in ComponentRegistry', () => { const chartBarRenderer = ComponentRegistry.get('bar-chart'); diff --git a/packages/plugin-editor/src/index.test.ts b/packages/plugin-editor/src/index.test.ts index c30c4a913d..b936519dfe 100644 --- a/packages/plugin-editor/src/index.test.ts +++ b/packages/plugin-editor/src/index.test.ts @@ -6,16 +6,17 @@ * LICENSE file in the root directory of this source tree. */ -import { describe, it, expect, beforeAll } from 'vitest'; +import { describe, it, expect } from 'vitest'; import { ComponentRegistry } from '@object-ui/core'; +// Imports all renderers to register them. Module scope, NOT awaited inside a +// `beforeAll` — Monaco is a heavy library, and loading it inside the hook billed +// that cost against `hookTimeout`, which is why this needed a 30s budget. The +// import phase has no test/hook timeout, so the raised timeout goes away rather +// than getting raised again (objectui#3010). +// See AGENTS.md §9 (test discipline). +import './index'; describe('Plugin Editor', () => { - // Import all renderers to register them - // Note: Monaco Editor is a heavy library that takes time to load - beforeAll(async () => { - await import('./index'); - }, 30000); // 30 second timeout for Monaco Editor initialization - describe('code-editor component', () => { it('should be registered in ComponentRegistry', () => { const editorRenderer = ComponentRegistry.get('code-editor'); diff --git a/packages/plugin-markdown/src/index.test.ts b/packages/plugin-markdown/src/index.test.ts index 70b0ae970a..381b188ae8 100644 --- a/packages/plugin-markdown/src/index.test.ts +++ b/packages/plugin-markdown/src/index.test.ts @@ -6,15 +6,18 @@ * LICENSE file in the root directory of this source tree. */ -import { describe, it, expect, beforeAll } from 'vitest'; +import { describe, it, expect } from 'vitest'; import { ComponentRegistry } from '@object-ui/core'; +// Imports all renderers to register them. Module scope, NOT awaited inside a +// `beforeAll` — there the cold transform of the renderer graph is billed to the +// hook, against `hookTimeout`. That is what made the sibling plugin-kanban test +// blow its raised 15s budget at 15021ms under full parallel load +// (objectui#3010); this file's timed portion was already at 5.83s of the same +// 15s. The import phase has no test/hook timeout, so no raised timeout is +// needed. See AGENTS.md §9 (test discipline). +import './index'; describe('Plugin Markdown', () => { - // Import all renderers to register them - beforeAll(async () => { - await import('./index'); - }, 15000); // Increase timeout to 15 seconds for async import - describe('markdown component', () => { it('should be registered in ComponentRegistry', () => { const markdownRenderer = ComponentRegistry.get('markdown');