Skip to content

Commit 208d3e2

Browse files
test(ui): two flaky Test UI cases wait on the effect they read (#29532)
<!-- claude-projects-attribution: {"github_login":"zhuangjianguo"} --> _Requested by **Steedos** · [project thread](https://claude.ai/code/project/chan_018ETKaNwMZCNRYD7D8Q9Ubb?thread=cmsg_018ETKaNwMZCNRYD7D8Q9Ubb11FQ2fAk48Dv5duBionc2x)_ Fixes #29468 Clause-②: no Metadata-protocol: none ## 维护者速读 - 改了什么:两个今天在 main 上各红过一次的 UI 测试,断言改为等待它真正读取的那个 effect 产出的结果,不再在 DOM 出现后立即同步读取。只改两个测试文件,产品代码不动。 - 为什么改:两处读到的值(Studio 的 beforeunload 守卫、Console 的 /me/apps 请求)都由 React 的 passive effect 写出,而 effect 可能晚于测试等待的那次提交执行;负载高的分片上测试先读到旧值就红了。 - 风险与回滚:仅测试改动,不发布任何包;回滚即 revert 本 PR。消融验证证明两条断言仍能钉住原卡的行为。 - 先看:`StudioDesignSurface.refusedLeafUnsaved-29113.test.tsx` 新增的 `untilReloadAsks` 及其三处调用;`App.permissionsWave-29114.test.tsx` 第一条用例中新增的 `waitFor(launcherCalls)` 加 `settle()`。 - 两分钟验证:在 `ui/` 下运行 `pnpm exec vitest run packages/app-shell/src/views/studio-design/StudioDesignSurface.refusedLeafUnsaved-29113.test.tsx apps/console/src/__tests__/App.permissionsWave-29114.test.tsx`,预期 2 个文件、8 条用例全绿。 - 需要你做的:无,席位落地。 ## What changes Two `ui/` test cases that each went red once on `main` today now wait on the outcome they read instead of reading it in the same tick as the DOM marker they awaited. Test-only: no product source changes, no changeset (neither touched file ships: `@object-ui/app-shell` is private and `@object-ui/console` publishes `dist` and its plugin entry only), so this PR carries `skip-changeset`. - `StudioDesignSurface.refusedLeafUnsaved-29113.test.tsx`: a new helper `untilReloadAsks(expected)` wraps the `beforeunload` read in `waitFor`. It replaces the three bare reads that follow an autosave answer or a leaf load (the CI red was the first, line 279). - `App.permissionsWave-29114.test.tsx`: in the first case, after the home page shows, the test waits for the launcher's `/me/apps` read, then runs the file's own `settle()` before the "exactly once, no duplicate URL" assertions, so a second request would also have time to show. ## Why The race is the same in both files: **a value written by a React passive effect, read in the tick after the commit the test waited for.** - Studio: the refusal banner shows in one commit. The pillar reports `refusedUnsent` from an effect (`onDirtyChange`), and the surface installs its `beforeunload` listener from an effect of the render that report causes. `findByTestId('studio-refusal')` resolves on the commit, so `reloadAsks()` can run before either effect has. - Console: `LauncherAppsProvider` asks `/me/apps` from its effect, which can run after the commit that shows `home-page`. Testing Library's `waitFor` resolves on the DOM mutation and drains with a 1 ms `setTimeout`. When React's scheduler yields after the commit (render work over its 5 ms frame), the passive effects run from a `setImmediate` continuation. On an idle box that continuation wins; on a loaded box the event loop takes over 1 ms to get back, the drain timer fires first, and the test reads the value before the effect writes it. This is a test race, not a product race: the guard and the request both happen, just one effect pass after the marker. ## Tests Reproduced first, at base `8ca9dd7b`, with each test file copied N times into one vitest run (`--maxWorkers=4`, `--sequence.shuffle`) beside node CPU hogs on a 4-core box: | File · case | Base: red / runs | Fixed: red / runs | Red line (same as CI) | |:--|:--|:--|:--| | refusedLeafUnsaved-29113 · "a refused report save makes a reload and the pillar link ask…" | 1 / 80 | 0 / 32 | 279 `expect(reloadAsks()).toBe(true)` | | permissionsWave-29114 · "the session names the organization…" | 1 / 36 | 0 / 36 | 230 `expect(launcherCalls()).toHaveLength(1)` | To name the race rather than only count it, a probe preload (`--import`, scratch only, not committed) made React's scheduler continuation (`performWorkUntilDeadline`) run from a timer 2 ms or 3 ms later instead of `setImmediate`, which is what a loaded event loop does to it: | File | Base: red / runs | Fixed: red / runs | |:--|:--|:--| | refusedLeafUnsaved-29113 | 14 / 20 (all at line 279) | 0 / 20 | | permissionsWave-29114 | 15 / 20 (all at line 230) | 0 / 20 | Ablations, each through `scripts/ablation-replace.mjs` (anchor 1 → 0, restored to the HEAD blob, `git diff HEAD` empty): - `StudioDesignSurface.tsx`: `onDirtyChangeRef.current?.(navDirty || refusedUnsent)` → `(navDirty)`. The fixed file goes red: the target case at `untilReloadAsks(true)`, plus the list-view case. 2 failed, 2 passed. - `apps/console/src/App.tsx`: `ConsolePermissionsRequest` removed. The fixed file goes red in both early-request cases at `waitFor(permissionsCalls … 1)`. 2 failed, 2 passed. Whole files on the merged head `3d6225a8`: 2 files, 8 tests, all passed. `pnpm --filter @object-ui/app-shell lint` and `pnpm --filter @object-ui/console lint` exit 0, with no warning on either touched file. `turbo run type-check` for `@object-ui/app-shell` and `@object-ui/console`: 72 tasks, exit 0. ## Acceptance notes - The shard-sized run used the `--maxWorkers` load beside CPU hogs, not the real `planShards` file set. The scheduler probe shows the mechanism directly. - The second case of the 29113 file (the rail) did not go red under the ablation. It reads the pillar's own `leafUnsent` during render, not the surface report, so it is synchronous and not part of this race. It is unchanged here. ## Verification <details><summary>Derived gates (dispatch-gates --commands, diff 392359f, 2 files, 23 changed lines)</summary> All exit 0 at `392359f6`: `check-closing-keyword-parity --self-test`, `check-comment-mask-corpus`, `check-sdui-manifest` and its `--self-test`, spec `check:duration-unit-keys`, `check:cross-package-test-inputs`, `check:dispatcher-error-vocabulary`, `check:driver-memory-census`, `check:engine-double-contract`, `check:gitlink-declared`, `check:nul-bytes`, `check:published-files`, `check:refd-timer-probe`, `check:skill-identifier-liveness`, `check:test-source-alias`, `check:tier-file-adoption`, `check:type-check-coverage`, `check:undeclared-dep-imports`, `check:watch-hint-literal`, `check:where-matcher`, `check:query-options-erasure`, `check:type-check-debt`, and `pnpm --dir ui exec vitest run scripts/` (169 files passed, 2 skipped; 4921 tests). Rerun on the merged head `3d6225a8`, exit 0: `check:type-check-coverage`, `check:query-options-erasure`, `check:type-check-debt`, `check-sdui-manifest`, `check:nul-bytes`. `check-closing-keyword-parity` was run on this body before the PR opened. </details> ## Human Minimum - [x] 1. Schemas start as Zod; `packages/spec` holds no runtime logic. (not touched) - [x] 2. Every name I added uses the casing for its kind. - [x] 3. No formatter ran; `pnpm lint` passes. - [x] 4. One issue, one branch off an up-to-date `main`, one pull request. - [x] 5. A change that publishes carries a changeset. (publishes nothing: `skip-changeset`) - [x] 6. `packages/spec` touched: its generated artifacts are regenerated and committed. (not touched) - [x] 7. No hand edits to `content/docs/references/`, `packages/*/CHANGELOG.md` or `content/docs/releases/`. - [x] 8. Title, body and comments are in English. --- _Generated by [Claude Code](https://claude.ai/code/session_01QsR2jRAnis8Swqnj1Aw3B8)_ Co-authored-by: Claude <noreply@anthropic.com>
1 parent 0e75081 commit 208d3e2

2 files changed

Lines changed: 20 additions & 3 deletions

File tree

‎ui/apps/console/src/__tests__/App.permissionsWave-29114.test.tsx‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,11 @@ describe('/auth/me/permissions leaves in the organization/list wave when the ses
225225

226226
organizationsAnswer(ORG);
227227
await waitFor(() => expect(screen.getByTestId('home-page')).toBeInTheDocument());
228+
// The launcher's `/me/apps` read leaves from its provider's effect, which can
229+
// run after the commit that shows the page (objectstack#29468: red once on a
230+
// loaded Test UI shard). Wait for it, then let any second request show.
231+
await waitFor(() => expect(launcherCalls()).toHaveLength(1));
232+
await settle();
228233
// No second request and no duplicate URL: the answer asked early is the one used.
229234
expect(permissionsCalls()).toHaveLength(1);
230235
expect(launcherCalls()).toHaveLength(1);

‎ui/packages/app-shell/src/views/studio-design/StudioDesignSurface.refusedLeafUnsaved-29113.test.tsx‎

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -266,6 +266,18 @@ function reloadAsks(): boolean {
266266
return e.defaultPrevented;
267267
}
268268

269+
/**
270+
* Wait until a reload asks (`true`) or no longer asks (`false`). The guard is
271+
* not part of the commit that shows or clears the refusal: the pillar reports
272+
* the change from an effect, and the surface installs or removes its
273+
* `beforeunload` listener from an effect of the render that report causes. So
274+
* a read straight after the refusal shows can run before either effect has
275+
* (objectstack#29468: red once on a loaded Test UI shard).
276+
*/
277+
async function untilReloadAsks(expected: boolean): Promise<void> {
278+
await waitFor(() => expect(reloadAsks()).toBe(expected));
279+
}
280+
269281
const pillarLink = (name: string) => within(screen.getByTestId('studio-pillar-nav')).getByRole('link', { name });
270282
const railRow = (name: string) => screen.getByTitle(`report · ${name}`);
271283

@@ -276,7 +288,7 @@ describe('a leaf edit whose save was refused guards every departure (objectstack
276288
await editAndSave();
277289

278290
// Red on base: the pillar reported only its nav edits, so neither asked.
279-
expect(reloadAsks()).toBe(true);
291+
await untilReloadAsks(true);
280292
fireEvent.click(pillarLink('Data'));
281293
expect(confirmSpy).toHaveBeenCalledTimes(1);
282294
expect(confirmSpy).toHaveBeenCalledWith(LEAVE_QUESTION);
@@ -285,7 +297,7 @@ describe('a leaf edit whose save was refused guards every departure (objectstack
285297
// The next edit is sent again, and this time it lands.
286298
state.refuse = false;
287299
await editAndSave();
288-
expect(reloadAsks()).toBe(false);
300+
await untilReloadAsks(false);
289301
fireEvent.click(pillarLink('Data'));
290302
await screen.findByTestId('studio-rail-data');
291303
expect(confirmSpy).toHaveBeenCalledTimes(1);
@@ -311,7 +323,7 @@ describe('a leaf edit whose save was refused guards every departure (objectstack
311323
await waitFor(() => expect(screen.getByTestId('stub-label')).toHaveTextContent('Ops report'), { timeout: 8000 });
312324
// The other leaf's buffer is installed clean: the refusal went with the edit.
313325
expect(screen.queryByTestId('studio-refusal')).toBeNull();
314-
expect(reloadAsks()).toBe(false);
326+
await untilReloadAsks(false);
315327
});
316328

317329
// CONTROL: an untouched leaf, and an edit whose save landed, never ask.

0 commit comments

Comments
 (0)