Skip to content

Commit c199895

Browse files
os-zhuangclaude
andauthored
test: fix five load-sensitive flaky tests by moving module loads out of timed windows (#3010)
* test(previews): stop dom-heavy preview tests racing the lazy renderer chunk DatasetPreview and ReportPreview render their renderers behind `React.lazy(() => import('@object-ui/plugin-charts' | '@object-ui/plugin-report'))`, so everything the assertions wait on lives *behind* a Suspense boundary and does not exist until that chunk resolves. Loading it is unbounded work: under full-suite parallelism Vite's transform pipeline is saturated (the `dom-heavy` project alone spends ~60s in transform), and the first import of the package graph can outlast RTL's 1000ms `waitFor`/`findBy` window. Instrumenting the import under a real full `dom-heavy` run measured `import('@object-ui/plugin-report')` at 312ms / 424ms / 976ms across three runs — that last one consumes 97.6% of the entire assertion budget before a single render or `queryDataset` call can happen. A captured failure confirms it: the DOM at timeout still showed the "Loading report renderer…" Suspense fallback, and `queryDataset` had 0 calls because `ReportRenderer` — the thing that calls it — had not loaded yet. That explains exactly which tests flaked: - ReportPreview.dataset: the *first two* tests, the ones running while the very first import is still in flight; tests 3-5 find the module warm. - DatasetPreview: only the ratio-measure test, because its "use the right axis" caption is the one assertion in the file that sits *inside* the Suspense boundary. Earlier tests assert on the table, which renders outside it — so they start the import but never wait for it, leaving the last test to race a load already in flight. Fix: resolve each chunk once in `beforeAll`, taking the module load out of every assertion window rather than widening the windows. ESM caches by resolved specifier, so warming the same specifier makes the component's own `React.lazy` factory resolve immediately. No timeout was raised and no file was moved off the `dom-heavy` list. Verified: `--project dom-heavy` failed 1 run in 4 before the change (the captured failure above) and is green 14 consecutive runs after it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test: move unbounded module loads out of timed windows (unit + dom-heavy flakes) Follow-up to the previous commit, which fixed the two `dom-heavy` preview files with a `beforeAll` warm-up. Two changes here: 1. Switch that warm-up to a module-scope import. `beforeAll` is bounded by `hookTimeout` (10s) — *narrower* than the 15s `testTimeout` it replaced. The import phase has no such bound (a full run logs `import 1523s` cumulative across 716 files without an import-phase failure), and it is where module loading naturally belongs. Same idiom now used for every fix below. 2. Extend the same treatment to three more files that share the root cause — unbounded work billed against a bounded window — found while verifying: - `anchors.page-seed.test.ts`: `createSeed` reaches the page synthesizer via `await import('@object-ui/plugin-detail')` (anchors.ts:207), so the cold transform of that graph is billed to the test body. With `isolate: false` the `unit` project's shared worker graph makes "who pays the cold transform" scheduling-dependent, so it blew the 15s budget only sometimes. Verified a timeout, not a wrong result: `createSeed` swallows failures and returns `{}`, so a genuine synth failure would read as `expected false to be true`, which was never observed. - `plugin-kanban/src/index.test.ts`: was `beforeAll(async () => { await import('./index') }, 15000)` — already "fixed" once by raising its timeout, and still timed out under full load (observed 15021ms, surfacing as 8 skipped tests rather than a failed assertion). The raised timeout is deleted, not re-raised. Tests: 15021ms hook -> 2ms. - `all-locales-key-parity.test.ts`: a genuine quadratic, not a load effect. Line 64 called `keysOf(builtInLocales[lang])` *inside* the `.filter()` predicate, re-walking the whole locale tree once per `en` key (~2.5k keys x a full recursive walk). The sibling assertion below it always hoisted the call; this one didn't. Hoisting it: tests 7.51s -> 25ms (~300x), same 20 assertions passing. Parity itself is clean — all 9 non-`en` packs have 0 missing and 0 extra keys — so this was never a missing-translation failure. No timeout was raised and no file was moved to an allowlist. Verified: - `--project dom-heavy`: failed 1 run in 4 before; green 20/20 runs after (14 on the `beforeAll` form, 6 on the final static-import form). - `--project unit`: the four files above pass; the 7 files that still fail are a pre-existing broken baseline (real AssertionErrors in spec-subschema-parity, onMutation, filter-operator-ast-parity x2, view-config-utils, view-operator-builder-parity, clientValidation.fieldRules) — unchanged by this commit, and unrelated to timing. - Full suite before vs after: identical 19 genuine test failures, no new failures, and no cross-file interference from the module-scope imports despite `isolate: false`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent f1c04b6 commit c199895

5 files changed

Lines changed: 63 additions & 7 deletions

File tree

packages/app-shell/src/views/metadata-admin/anchors.page-seed.test.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,20 @@
11
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
22

33
import { describe, it, expect } from 'vitest';
4+
// `createSeed` reaches the page synthesizer through a dynamic
5+
// `await import('@object-ui/plugin-detail')` (anchors.ts), so the cold transform
6+
// of that whole package graph is billed to whichever test first awaits the hook
7+
// — inside the test body, against the 15s `testTimeout`. The `unit` project runs
8+
// `isolate: false`, so whether the module is already warm depends on what else
9+
// the worker happened to run first; when it wasn't, the seeding test blew the
10+
// budget. (It surfaced as a timeout rather than a wrong result because
11+
// `createSeed` swallows failures and returns `{}`.)
12+
//
13+
// Importing it here moves that cost into the file's import phase, which no test
14+
// or hook timeout applies to, so it can never land inside a timed window again.
15+
// Keep the specifier identical to the one in anchors.ts — ESM caches by resolved
16+
// specifier, so this makes the hook's own dynamic import resolve immediately.
17+
import '@object-ui/plugin-detail';
418
import { registerBuiltinAnchors } from './anchors';
519
import { resolveResourceConfig } from './registry';
620

packages/app-shell/src/views/metadata-admin/previews/DatasetPreview.test.tsx

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,21 @@
22

33
import { describe, it, expect, vi, afterEach } from 'vitest';
44
import { render, screen, cleanup, waitFor } from '@testing-library/react';
5+
// DatasetPreview renders its chart behind
6+
// `React.lazy(() => import('@object-ui/plugin-charts'))`, and the "use the right
7+
// axis" caption asserted on below lives *inside* that Suspense boundary — so it
8+
// only exists once the (recharts-backed) chunk resolves. Loading it is unbounded
9+
// work: under full-suite parallelism Vite's transform pipeline is saturated and
10+
// the first import of the package graph can outlast RTL's 1000ms
11+
// `waitFor`/`findBy` window. The earlier tests in this file assert on the table,
12+
// which renders *outside* the boundary, so they start the import but never wait
13+
// for it — leaving the ratio-measure test to race a load already in flight.
14+
//
15+
// Importing it here moves that cost into the file's import phase, which no test
16+
// or hook timeout applies to, instead of widening any assertion window. Keep the
17+
// specifier identical to DatasetPreview.tsx's — ESM caches by resolved specifier,
18+
// so this makes the component's own `React.lazy` factory resolve immediately.
19+
import '@object-ui/plugin-charts';
520
import { DatasetPreview } from './DatasetPreview';
621

722
// Mock the data adapter the preview pulls from AdapterProvider.

packages/app-shell/src/views/metadata-admin/previews/ReportPreview.dataset.test.tsx

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,23 @@
22

33
import { describe, it, expect, vi, afterEach } from 'vitest';
44
import { render, screen, cleanup, waitFor } from '@testing-library/react';
5+
// ReportPreview renders the runtime report renderer behind
6+
// `React.lazy(() => import('@object-ui/plugin-report'))`, so nothing below the
7+
// Suspense boundary — including the `queryDataset` call the assertions wait on —
8+
// exists until that chunk resolves. Loading it is unbounded work: under
9+
// full-suite parallelism Vite's transform pipeline is saturated (the `dom-heavy`
10+
// project alone spends ~60s in transform) and the first import of the package
11+
// graph can outlast RTL's 1000ms `waitFor`/`findBy` window. Measured under a real
12+
// full run, this import alone took 312/424/976ms across three runs — that last
13+
// one is 97.6% of the entire assertion budget. The assertions then raced the
14+
// module loader and failed against the "Loading report renderer…" fallback —
15+
// only in the first tests of the file, because later ones find the module warm.
16+
//
17+
// Importing it here moves that cost into the file's import phase, which no test
18+
// or hook timeout applies to, instead of widening any assertion window. Keep the
19+
// specifier identical to ReportPreview.tsx's — ESM caches by resolved specifier,
20+
// so this makes the component's own `React.lazy` factory resolve immediately.
21+
import '@object-ui/plugin-report';
522
import { ReportPreview } from './ReportPreview';
623

724
// Mock the data adapter the preview pulls from AdapterProvider.

packages/i18n/src/__tests__/all-locales-key-parity.test.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,14 @@ describe('all locale packs are at full key parity with en (objectui#2872)', () =
6161
});
6262

6363
it.each(OTHER_LOCALES)('%s defines every en key', (lang) => {
64-
const missing = [...EN].filter((k) => !keysOf(builtInLocales[lang]).has(k)).sort();
64+
// Build the pack's key set ONCE. Calling `keysOf` inside the predicate
65+
// re-walks the whole locale tree per `en` key (~2.5k keys x a full
66+
// recursive walk), which made this quadratic: ~850-2200ms per locale
67+
// isolated, and >15s — a timeout, not a parity failure — once full-suite
68+
// contention slowed each walk down. The sibling assertion below always
69+
// hoisted it; this one didn't.
70+
const packKeys = keysOf(builtInLocales[lang]);
71+
const missing = [...EN].filter((k) => !packKeys.has(k)).sort();
6572
expect(missing, `${lang} is missing ${missing.length} key(s)`).toEqual([]);
6673
});
6774

packages/plugin-kanban/src/index.test.ts

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,18 @@
66
* LICENSE file in the root directory of this source tree.
77
*/
88

9-
import { describe, it, expect, beforeAll } from 'vitest';
9+
import { describe, it, expect } from 'vitest';
1010
import { ComponentRegistry } from '@object-ui/core';
11+
// Import all renderers to register them. This was a `beforeAll(async () => {
12+
// await import('./index') }, 15000)` — the cold transform of the renderer graph
13+
// was billed to a hook, so it needed its timeout raised to 15s, and under full
14+
// parallel load it blew even that (observed: 15021ms, reported as 8 skipped tests
15+
// rather than a failed assertion). A static import puts the same cost in the
16+
// file's import phase, which no test or hook timeout applies to — so the raised
17+
// timeout is no longer needed at all.
18+
import './index';
1119

1220
describe('Plugin Kanban', () => {
13-
// Import all renderers to register them
14-
beforeAll(async () => {
15-
await import('./index');
16-
}, 15000); // Increase timeout to 15 seconds for async import
17-
1821
describe('kanban component', () => {
1922
it('should be registered in ComponentRegistry', () => {
2023
const kanbanRenderer = ComponentRegistry.get('kanban-ui');

0 commit comments

Comments
 (0)