From 8f9a2406c814e50d503489205256d8592ac99444 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 5 Sep 2026 16:01:48 +0000 Subject: [PATCH] test(app-shell): keep the approvals fetch double alive for the whole file MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `RecordDetailView.approvalDeclaredActions.test.tsx` installed its `fetch` double inside `stubApprovalsApi` and tore it down in an unconditional `afterEach`. A decision dispatched through `DeclaredActionsBar` carries `refreshAfter: true`, so on success the record page re-reads the approval state — `handleApprovalActionDone` calls `void approvals.refresh()` and the `notifyDataChanged` beside it runs the same read again through the record-invalidation effect. Neither is awaited by the console or by the test, which asserts on the POST and returns, so a `GET /api/v1/approvals/requests?object=…` was still in flight at teardown. Vitest runs `afterEach` in reverse registration order, so this file's teardown ran first — before RTL `cleanup()` and before the network-escape guard's assertion — and restored the real `fetch` while that read was pending. Whether the read landed before or after the restore was pure timing. Install one double at module scope instead and never remove it: the per-test router still swaps, so call counts keep meaning "this test's reads", but the window where a late probe can reach a real socket no longer exists. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01KbJQ1y1J12nZxYzFWhP8Q3 --- .changeset/7439-approvals-teardown-race.md | 9 +++ ...etailView.approvalDeclaredActions.test.tsx | 58 +++++++++++++++---- 2 files changed, 57 insertions(+), 10 deletions(-) create mode 100644 .changeset/7439-approvals-teardown-race.md diff --git a/.changeset/7439-approvals-teardown-race.md b/.changeset/7439-approvals-teardown-race.md new file mode 100644 index 0000000000..b0bc5aad13 --- /dev/null +++ b/.changeset/7439-approvals-teardown-race.md @@ -0,0 +1,9 @@ +--- +--- + +Close the network-escape window in +`RecordDetailView.approvalDeclaredActions.test.tsx` (objectui#7439): the `fetch` +double is now installed once for the whole file instead of being torn down by an +unconditional `afterEach`, which used to race the record page's own +`refreshAfter: true` approvals re-read. Test only; no package is released by +this change. diff --git a/packages/app-shell/src/views/RecordDetailView.approvalDeclaredActions.test.tsx b/packages/app-shell/src/views/RecordDetailView.approvalDeclaredActions.test.tsx index 5d703af2bf..c704e91924 100644 --- a/packages/app-shell/src/views/RecordDetailView.approvalDeclaredActions.test.tsx +++ b/packages/app-shell/src/views/RecordDetailView.approvalDeclaredActions.test.tsx @@ -31,7 +31,7 @@ */ import * as React from 'react'; -import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { describe, it, expect, vi, beforeEach } from 'vitest'; import { render, screen, waitFor, fireEvent, cleanup } from '@testing-library/react'; import { MemoryRouter } from 'react-router-dom'; @@ -254,10 +254,16 @@ const pendingRequest = (viewer: Record | undefined) => ({ ...(viewer ? { viewer } : {}), }); -let approvalsFetch: ReturnType; +/** + * The approvals router the current test is being served by. Swapped per test + * (`stubApprovalsApi`) so a call count still means "this test's reads"; the + * GLOBAL `fetch` double that forwards to it is installed once, below. + */ +type ApprovalsRouter = (url: string, init?: RequestInit) => Promise; +let approvalsFetch: ReturnType>; -function stubApprovalsApi(row: Record) { - approvalsFetch = vi.fn(async (url: string) => { +function makeApprovalsApi(row: Record) { + return vi.fn(async (url: string) => { const u = String(url); if (u.includes(`/approvals/requests/${REQUEST_ID}/actions`)) { return { ok: true, json: async () => ({ data: [] }) } as any; @@ -271,9 +277,46 @@ function stubApprovalsApi(row: Record) { } return { ok: true, json: async () => ({ data: [] }) } as any; }); - vi.stubGlobal('fetch', approvalsFetch); } +function stubApprovalsApi(row: Record) { + approvalsFetch = makeApprovalsApi(row); +} + +/** + * The `fetch` double is installed ONCE, here at module scope, and is + * deliberately NEVER torn down — there is no `vi.unstubAllGlobals()` in this + * file's `afterEach`, and adding one back re-opens objectui#7439. + * + * ⛔ Why the usual per-test install/teardown pair does not work here. A decision + * dispatched by `DeclaredActionsBar` carries `refreshAfter: true`, so on success + * the record page re-reads the approval state — `RecordDetailView`'s + * `handleApprovalActionDone` calls `void approvals.refresh()`, and the + * `notifyDataChanged` it fires alongside runs the same read again through the + * record-invalidation effect. Both are fire-and-forget by design: nothing in the + * console awaits them, and no test barrier here waits for them either — the + * decision cases assert on the POST and return. That leaves a + * `GET /api/v1/approvals/requests?object=…` in flight when the test ends. + * + * Vitest runs `afterEach` hooks in reverse registration order, so a teardown in + * THIS file runs FIRST — before the root setup's RTL `cleanup()` and before the + * network-escape guard's assertion. A `vi.unstubAllGlobals()` there restored the + * real `fetch` while that read was still pending, and whether the read landed + * before or after that restore was pure timing: green on a fast worker, a + * `Network escape` red under load. Keeping ONE double installed for the whole + * file removes the window instead of widening the stub — the late read is served + * by the same router either way, and no test can ever end with the real `fetch` + * back in place. + * + * The default below answers "no requests on this record", so the double is + * honest even before the first `stubApprovalsApi` call and after the last test. + */ +approvalsFetch = vi.fn(async () => ({ ok: true, json: async () => ({ data: [] }) })); +vi.stubGlobal( + 'fetch', + ((url: string, init?: RequestInit) => approvalsFetch(url, init)) as unknown as typeof globalThis.fetch, +); + const METADATA = { objects: [...OBJECTS, SYS_APPROVAL_REQUEST_DEF], pages: [], @@ -321,10 +364,6 @@ beforeEach(() => { authFetchSpy.mockClear(); }); -afterEach(() => { - vi.unstubAllGlobals(); -}); - describe('record page decision actions — a GROUP approver (objectui#3055)', () => { /** * The acceptance case: the signed-in user holds the slot through a position, @@ -473,7 +512,6 @@ describe('record page decision actions — the submitter (objectui#3055)', () => describe('record page decision actions — no bar without a pending request', () => { it('renders no decision chrome when the record has no requests', async () => { approvalsFetch = vi.fn(async () => ({ ok: true, json: async () => ({ data: [] }) }) as any); - vi.stubGlobal('fetch', approvalsFetch); renderRecordPage(); await waitFor(() => expect(approvalsFetch).toHaveBeenCalled());