Skip to content

Commit 1fd0c32

Browse files
test(app-shell): unmount before unstubbing fetch in the ImpersonationBanner test (#30502)
<!-- claude-projects-attribution: {"github_login":"os-sales"} --> _Requested by **Sales** · [project thread](https://claude.ai/code/project/chan_01D5bWFY3rFzgJcytD5gD4Uu?thread=cmsg_01D5bWFY3rFzgJcytD5gD4UuXpPPZiBKyj1HzrdQ6La3ZH)_ domain:ui#6 · os-sales · session_01L6bhgUR8Nu4M7tbtgES7q2 Fixes #30434 Clause-②: no Metadata-protocol: none ## 维护者速读 - 改了什么:只改 `ImpersonationBanner.test.tsx` 一个测试文件:`afterEach` 先 `cleanup()` 卸载组件,再 `vi.unstubAllGlobals()` 撤掉 fetch 替身;另加一条用例,钉住测试体结束后才发出的 `sys_user` 读取仍落到替身。 - 为什么改:原顺序先撤替身、后由根 setup 卸载,卸载时补跑的读取打到真实 `localhost:3000`,网络逃逸守卫随机把无关 PR 的 `Test UI` 判红(PR #30417 就为此白查一轮)。 - 风险与回滚:仅测试代码,不进任何发布包;回滚即 revert 本提交。 - 先看:`ui/packages/app-shell/src/layout/__tests__/ImpersonationBanner.test.tsx` 的 `afterEach` 与文件末尾新增用例。 - 两分钟验证:`pnpm --dir ui exec vitest run packages/app-shell/src/layout/__tests__/ImpersonationBanner.test.tsx`,预期 14 条全绿。 - 需要你做的:无,席位落地(PR 需带 `skip-changeset` 标签)。 ## What changes - `afterEach` now calls RTL `cleanup()` before `vi.unstubAllGlobals()`. Vitest runs this file's hook before the root setup's `cleanup()` and before the network-escape guard. With the old order the tree was still mounted when the real `fetch` came back, so a banner `sys_user` read whose effect was still pending when the body returned went out during that cleanup's act-flush. That is the shape the guard's message names (objectui#7439). The banner issues `fetch` synchronously inside its effect, so once the tree is unmounted nothing in this file can issue a read. Cleanup-first is therefore enough, and it is the smaller of the two shapes the guard offers. - New case `a read that lands AFTER the test body still reaches the double, never the network`. A small probe issues the banner's own administrator read (same URL, same `createAuthenticatedFetch` lane) from its unmount. That is the exact moment the teardown order decides. An `onTestFinished` check, which runs after every `afterEach` including the guard, asserts the double served it. A probe stands in for the banner's effect because React's passive-effect timing cannot be held open on purpose. The probe lands a read in that window every time. - `ImpersonationBanner.tsx` is not touched. ## Tests - File on the fix: 14/14 passed (13 before plus the new case). - Positive control: the old teardown order restored through `scripts/ablation-replace.mjs`, which records the mutation on disk and restores from HEAD. The new case goes red twice, and the other 13 stay green. - Stability: 20 consecutive runs of the file, 20/20 green, with zero `Network escape` lines across all 20 logs. ## Acceptance notes - `skip-changeset`: test-only. The file is under `src/**/__tests__` and is not shipped in any package's published files. - Local `type-check` of `@object-ui/app-shell` (`tsc -p tsconfig.test.json`) was NOT MEASURED. In this container the sibling `@object-ui/*` packages are unbuilt, so every error is TS2307 module resolution, including those in untouched files. CI's `Type Check UI` lane covers it. ## Verification Head `b244cccb8d`. <details><summary>Positive control: old order (unstub, then the root setup's cleanup)</summary> ``` ablation-replace: ok mutation landed: anchor 1 -> 0, blob d3570d9 -> 58c6112e115d FAIL |dom| packages/app-shell/src/layout/__tests__/ImpersonationBanner.test.tsx > ImpersonationBanner (#4467) > a read that lands AFTER the test body still reaches the double, never the network Error: Network escape: this test reached a REAL socket at http://localhost:3000/api/v1/data/sys_user/usr_admin. FAIL |dom| ... > a read that lands AFTER the test body still reaches the double, never the network AssertionError: expected 1 to be 2 // Object.is equality Test Files 1 failed (1) Tests 1 failed | 13 passed (14) ablation-replace: ok restored: blob == HEAD (d3570d9) and `git diff HEAD` is empty ``` The URL is the one the CI flake reported (PR #30417, job 127192655114). </details> <details><summary>Fix: one run, then 20 in a row</summary> ``` Test Files 1 passed (1) Tests 14 passed (14) run 1..20: Tests 14 passed (14) (each) LOOP pass=20 fail=0 os-verify-lock: VERDICT command-exit 0 ``` </details> <details><summary>Derived gates (dispatch-gates --commands, 26 families)</summary> All 26 exited 0, including `pnpm --dir ui exec vitest run scripts/` (Test Files 170 passed, 2 skipped; Tests 4985 passed, 2 skipped) and `pnpm check:type-check-debt`. ``` Run reconciliation — 26 derived, 26 run, 0 NOT-MEASURED, 0 UNRUN. ✓ dispatch-gates --ran: 26 derived famil(ies) accounted for — 26 run, 0 NOT-MEASURED ``` </details> 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01L6bhgUR8Nu4M7tbtgES7q2 <!-- signed: domain:ui#6 · os-sales · session_01L6bhgUR8Nu4M7tbtgES7q2 --> Co-authored-by: Claude <noreply@anthropic.com>
1 parent 5174603 commit 1fd0c32

1 file changed

Lines changed: 47 additions & 3 deletions

File tree

‎ui/packages/app-shell/src/layout/__tests__/ImpersonationBanner.test.tsx‎

Lines changed: 47 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,11 @@
3030
* resolved, and what the console renders for it.
3131
*/
3232

33-
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
34-
import { render, screen, waitFor } from '@testing-library/react';
33+
import { describe, it, expect, vi, beforeEach, afterEach, onTestFinished } from 'vitest';
34+
import { cleanup, render, screen, waitFor } from '@testing-library/react';
3535
import userEvent from '@testing-library/user-event';
36-
import { AuthProvider, TokenStorage } from '@object-ui/auth';
36+
import { useEffect } from 'react';
37+
import { AuthProvider, TokenStorage, createAuthenticatedFetch } from '@object-ui/auth';
3738
import type { AuthClient } from '@object-ui/auth';
3839
import { I18nProvider } from '@object-ui/i18n';
3940
import { MemoryRouter, useLocation } from 'react-router-dom';
@@ -131,12 +132,36 @@ function renderBanner() {
131132
);
132133
}
133134

135+
/**
136+
* Issues the banner's own administrator read — same URL, same authenticated
137+
* lane — at the one moment the teardown order decides: when `cleanup()`
138+
* unmounts the tree. That is where the banner's read lands when its effect is
139+
* still pending as the test body returns (cleanup's act-flush runs it), which
140+
* is timing and cannot be held open on purpose; this lands one there every time.
141+
*/
142+
function LateAdministratorRead() {
143+
useEffect(
144+
() => () => {
145+
void createAuthenticatedFetch()(ADMIN_READ_PATH, { method: 'GET' }).catch(() => {});
146+
},
147+
[],
148+
);
149+
return null;
150+
}
151+
134152
describe('ImpersonationBanner (#4467)', () => {
135153
beforeEach(() => {
136154
TokenStorage.clear();
137155
});
138156

139157
afterEach(() => {
158+
// Unmount BEFORE restoring the real `fetch` (objectui#7439). Vitest runs
159+
// this hook before the root setup's RTL cleanup() and the network-escape
160+
// guard, so unstubbing first left the tree mounted with the real global
161+
// back: the banner's `sys_user` read, when its effect was still pending as
162+
// the body returned, then went out to localhost:3000 during that cleanup.
163+
// Pinned by 'a read that lands AFTER the test body …' below.
164+
cleanup();
140165
vi.unstubAllGlobals();
141166
vi.restoreAllMocks();
142167
TokenStorage.clear();
@@ -307,6 +332,25 @@ describe('ImpersonationBanner (#4467)', () => {
307332
expect(screen.queryByTestId('impersonation-not-restored')).toBeNull();
308333
expect(screen.queryByTestId('impersonation-stop-error')).toBeNull();
309334
});
335+
336+
it('a read that lands AFTER the test body still reaches the double, never the network', async () => {
337+
TokenStorage.set(IMPERSONATED_TOKEN);
338+
const seen = stubStopEndpoint((url) =>
339+
url.endsWith(ADMIN_READ_PATH) ? adminRow({ name: 'Dev Admin' }) : new Response('{}'),
340+
);
341+
renderBanner();
342+
await screen.findByTestId('impersonation-started-by');
343+
const reads = () => seen.urls.filter((u) => u.endsWith(ADMIN_READ_PATH)).length;
344+
const readsInBody = reads();
345+
render(<LateAdministratorRead />);
346+
347+
// Runs after every afterEach, the network-escape guard's included. With
348+
// the teardown unstubbing before cleanup(), the late read went to the real
349+
// socket instead: this count stays put and the guard reds the test.
350+
onTestFinished(() => {
351+
expect(reads()).toBe(readsInBody + 1);
352+
});
353+
});
310354
});
311355

312356
/**

0 commit comments

Comments
 (0)