From 199499bd482c9c3c59a9918776902ff50388645f Mon Sep 17 00:00:00 2001 From: Brad Hefta-Gaub Date: Wed, 3 Jun 2026 17:02:27 -0700 Subject: [PATCH 1/2] fix(test): strengthen unmount and Tab-trap coverage assertions Address Copilot review on #401: use userEvent.tab() to verify focus advances when Tab is not trapped, and assert no React unmounted warnings in post-unmount async tests for billing, waitlist, and admin packages. --- .../components/AdminDetailDrawer.web.test.tsx | 11 ++--- packages/billing/src/BillingProvider.test.tsx | 44 +++++++++++++++++-- .../waitlist/src/hooks/useSignupMode.test.tsx | 8 ++++ 3 files changed, 54 insertions(+), 9 deletions(-) diff --git a/packages/admin/src/components/AdminDetailDrawer.web.test.tsx b/packages/admin/src/components/AdminDetailDrawer.web.test.tsx index faadee70..59bec11f 100644 --- a/packages/admin/src/components/AdminDetailDrawer.web.test.tsx +++ b/packages/admin/src/components/AdminDetailDrawer.web.test.tsx @@ -308,7 +308,8 @@ describe('AdminDetailDrawer', () => { expect(onClose).toHaveBeenCalledTimes(1); }); - it('does not trap Tab when focus is not on the last element', () => { + it('does not trap Tab when focus is not on the last element', async () => { + const user = userEvent.setup(); render( @@ -316,10 +317,10 @@ describe('AdminDetailDrawer', () => { ); const notes = screen.getByLabelText('Notes'); - notes.focus(); - document.dispatchEvent( - new KeyboardEvent('keydown', { key: 'Tab', bubbles: true }) - ); + const save = screen.getByRole('button', { name: 'Save' }); + await user.click(notes); expect(document.activeElement).toBe(notes); + await user.tab(); + expect(document.activeElement).toBe(save); }); }); diff --git a/packages/billing/src/BillingProvider.test.tsx b/packages/billing/src/BillingProvider.test.tsx index 41eed02b..efc8c3d9 100644 --- a/packages/billing/src/BillingProvider.test.tsx +++ b/packages/billing/src/BillingProvider.test.tsx @@ -591,6 +591,7 @@ describe('BillingProvider', () => { data: ReturnType | null; error: null; }) => void = () => {}; + const consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); auth.state.session = { user: { id: 'u-plan-unmount' } }; const row = { ...testSubscription(), @@ -612,11 +613,20 @@ describe('BillingProvider', () => { await waitFor(() => expect(db.planMaybeSingle).toHaveBeenCalled()); unmount(); resolvePlan({ data: testPlan({ display_name: 'Late plan' }), error: null }); - await Promise.resolve(); + await act(async () => { + await Promise.resolve(); + }); + + const unmountedWarnings = consoleSpy.mock.calls.filter(([message]) => + String(message).toLowerCase().includes('unmounted') + ); + expect(unmountedWarnings).toHaveLength(0); + consoleSpy.mockRestore(); }); it('ignores ensure_billing_subscription result after unmount', async () => { let resolveRpc: (value: { error: null }) => void = () => {}; + const consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); auth.state.session = { user: { id: 'u-rpc-unmount' } }; db.rpc.mockImplementationOnce( () => @@ -635,11 +645,20 @@ describe('BillingProvider', () => { await waitFor(() => expect(db.rpc).toHaveBeenCalled()); unmount(); resolveRpc({ error: null }); - await Promise.resolve(); + await act(async () => { + await Promise.resolve(); + }); + + const unmountedWarnings = consoleSpy.mock.calls.filter(([message]) => + String(message).toLowerCase().includes('unmounted') + ); + expect(unmountedWarnings).toHaveLength(0); + consoleSpy.mockRestore(); }); it('ignores plan query errors after unmount', async () => { let rejectPlan: (error: Error) => void = () => {}; + const consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); auth.state.session = { user: { id: 'u-plan-err-unmount' } }; const row = { ...testSubscription(), @@ -661,11 +680,20 @@ describe('BillingProvider', () => { await waitFor(() => expect(db.planMaybeSingle).toHaveBeenCalled()); unmount(); rejectPlan(new Error('late plan fail')); - await Promise.resolve(); + await act(async () => { + await Promise.resolve(); + }); + + const unmountedWarnings = consoleSpy.mock.calls.filter(([message]) => + String(message).toLowerCase().includes('unmounted') + ); + expect(unmountedWarnings).toHaveLength(0); + consoleSpy.mockRestore(); }); it('ignores ensure_billing_subscription errors after unmount', async () => { let rejectRpc: (error: Error) => void = () => {}; + const consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); auth.state.session = { user: { id: 'u-rpc-err-unmount' } }; db.rpc.mockImplementationOnce( () => @@ -684,6 +712,14 @@ describe('BillingProvider', () => { await waitFor(() => expect(db.rpc).toHaveBeenCalled()); unmount(); rejectRpc(new Error('late rpc fail')); - await Promise.resolve(); + await act(async () => { + await Promise.resolve(); + }); + + const unmountedWarnings = consoleSpy.mock.calls.filter(([message]) => + String(message).toLowerCase().includes('unmounted') + ); + expect(unmountedWarnings).toHaveLength(0); + consoleSpy.mockRestore(); }); }); diff --git a/packages/waitlist/src/hooks/useSignupMode.test.tsx b/packages/waitlist/src/hooks/useSignupMode.test.tsx index 76640cf8..1a0eb10e 100644 --- a/packages/waitlist/src/hooks/useSignupMode.test.tsx +++ b/packages/waitlist/src/hooks/useSignupMode.test.tsx @@ -31,6 +31,7 @@ describe('useSignupMode', () => { resolveSettings = resolve; }) ); + const consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); const supabase = {} as never; const { unmount } = renderHook(() => useSignupMode(supabase)); await waitFor(() => @@ -39,6 +40,13 @@ describe('useSignupMode', () => { unmount(); resolveSettings({ signup_mode: 'open', copy: {}, metadata_schema: [] }); await Promise.resolve(); + + const unmountedWarnings = consoleSpy.mock.calls.filter(([message]) => + String(message).toLowerCase().includes('unmounted') + ); + expect(unmountedWarnings).toHaveLength(0); + + consoleSpy.mockRestore(); vi.restoreAllMocks(); }); }); From f7e27149f0ed1ddf7f4dc576180caa892e378f6b Mon Sep 17 00:00:00 2001 From: Brad Hefta-Gaub Date: Wed, 3 Jun 2026 17:10:44 -0700 Subject: [PATCH 2/2] fix(test): harden console.error unmount warning assertions Wrap console.error spies in try/finally so failed assertions cannot leak mocks into later tests, and scan all console.error arguments when detecting React unmounted-component warnings. --- packages/billing/src/BillingProvider.test.tsx | 232 +++++++++--------- .../waitlist/src/hooks/useSignupMode.test.tsx | 38 +-- 2 files changed, 145 insertions(+), 125 deletions(-) diff --git a/packages/billing/src/BillingProvider.test.tsx b/packages/billing/src/BillingProvider.test.tsx index efc8c3d9..51f15e8e 100644 --- a/packages/billing/src/BillingProvider.test.tsx +++ b/packages/billing/src/BillingProvider.test.tsx @@ -11,6 +11,15 @@ import { testSubscription, } from './test/billingFixtures.js'; +type ConsoleErrorSpy = ReturnType>; + +function expectNoUnmountedConsoleWarnings(consoleSpy: ConsoleErrorSpy) { + const unmountedWarnings = consoleSpy.mock.calls.filter(args => + args.some(arg => String(arg).toLowerCase().includes('unmounted')) + ); + expect(unmountedWarnings).toHaveLength(0); +} + function Reader() { const { userId, subscription } = useBillingContext(); return ( @@ -592,134 +601,137 @@ describe('BillingProvider', () => { error: null; }) => void = () => {}; const consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); - auth.state.session = { user: { id: 'u-plan-unmount' } }; - const row = { - ...testSubscription(), - user_id: 'u-plan-unmount', - plan_id: 'plan_free', - }; - db.maybeSingle.mockResolvedValue({ data: row, error: null }); - db.planMaybeSingle.mockImplementationOnce( - () => - new Promise(resolve => { - resolvePlan = resolve; - }) - ); - const { unmount } = render( - - - - ); - await waitFor(() => expect(db.planMaybeSingle).toHaveBeenCalled()); - unmount(); - resolvePlan({ data: testPlan({ display_name: 'Late plan' }), error: null }); - await act(async () => { - await Promise.resolve(); - }); + try { + auth.state.session = { user: { id: 'u-plan-unmount' } }; + const row = { + ...testSubscription(), + user_id: 'u-plan-unmount', + plan_id: 'plan_free', + }; + db.maybeSingle.mockResolvedValue({ data: row, error: null }); + db.planMaybeSingle.mockImplementationOnce( + () => + new Promise(resolve => { + resolvePlan = resolve; + }) + ); + const { unmount } = render( + + + + ); + await waitFor(() => expect(db.planMaybeSingle).toHaveBeenCalled()); + unmount(); + resolvePlan({ + data: testPlan({ display_name: 'Late plan' }), + error: null, + }); + await act(async () => { + await Promise.resolve(); + }); - const unmountedWarnings = consoleSpy.mock.calls.filter(([message]) => - String(message).toLowerCase().includes('unmounted') - ); - expect(unmountedWarnings).toHaveLength(0); - consoleSpy.mockRestore(); + expectNoUnmountedConsoleWarnings(consoleSpy); + } finally { + consoleSpy.mockRestore(); + } }); it('ignores ensure_billing_subscription result after unmount', async () => { let resolveRpc: (value: { error: null }) => void = () => {}; const consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); - auth.state.session = { user: { id: 'u-rpc-unmount' } }; - db.rpc.mockImplementationOnce( - () => - new Promise(resolve => { - resolveRpc = resolve; - }) - ); - const { unmount } = render( - - - - ); - await waitFor(() => - expect(screen.getByTestId('uid').textContent).toBe('u-rpc-unmount') - ); - await waitFor(() => expect(db.rpc).toHaveBeenCalled()); - unmount(); - resolveRpc({ error: null }); - await act(async () => { - await Promise.resolve(); - }); + try { + auth.state.session = { user: { id: 'u-rpc-unmount' } }; + db.rpc.mockImplementationOnce( + () => + new Promise(resolve => { + resolveRpc = resolve; + }) + ); + const { unmount } = render( + + + + ); + await waitFor(() => + expect(screen.getByTestId('uid').textContent).toBe('u-rpc-unmount') + ); + await waitFor(() => expect(db.rpc).toHaveBeenCalled()); + unmount(); + resolveRpc({ error: null }); + await act(async () => { + await Promise.resolve(); + }); - const unmountedWarnings = consoleSpy.mock.calls.filter(([message]) => - String(message).toLowerCase().includes('unmounted') - ); - expect(unmountedWarnings).toHaveLength(0); - consoleSpy.mockRestore(); + expectNoUnmountedConsoleWarnings(consoleSpy); + } finally { + consoleSpy.mockRestore(); + } }); it('ignores plan query errors after unmount', async () => { let rejectPlan: (error: Error) => void = () => {}; const consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); - auth.state.session = { user: { id: 'u-plan-err-unmount' } }; - const row = { - ...testSubscription(), - user_id: 'u-plan-err-unmount', - plan_id: 'plan_free', - }; - db.maybeSingle.mockResolvedValue({ data: row, error: null }); - db.planMaybeSingle.mockImplementationOnce( - () => - new Promise((_resolve, reject) => { - rejectPlan = reject; - }) - ); - const { unmount } = render( - - - - ); - await waitFor(() => expect(db.planMaybeSingle).toHaveBeenCalled()); - unmount(); - rejectPlan(new Error('late plan fail')); - await act(async () => { - await Promise.resolve(); - }); + try { + auth.state.session = { user: { id: 'u-plan-err-unmount' } }; + const row = { + ...testSubscription(), + user_id: 'u-plan-err-unmount', + plan_id: 'plan_free', + }; + db.maybeSingle.mockResolvedValue({ data: row, error: null }); + db.planMaybeSingle.mockImplementationOnce( + () => + new Promise((_resolve, reject) => { + rejectPlan = reject; + }) + ); + const { unmount } = render( + + + + ); + await waitFor(() => expect(db.planMaybeSingle).toHaveBeenCalled()); + unmount(); + rejectPlan(new Error('late plan fail')); + await act(async () => { + await Promise.resolve(); + }); - const unmountedWarnings = consoleSpy.mock.calls.filter(([message]) => - String(message).toLowerCase().includes('unmounted') - ); - expect(unmountedWarnings).toHaveLength(0); - consoleSpy.mockRestore(); + expectNoUnmountedConsoleWarnings(consoleSpy); + } finally { + consoleSpy.mockRestore(); + } }); it('ignores ensure_billing_subscription errors after unmount', async () => { let rejectRpc: (error: Error) => void = () => {}; const consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); - auth.state.session = { user: { id: 'u-rpc-err-unmount' } }; - db.rpc.mockImplementationOnce( - () => - new Promise((_resolve, reject) => { - rejectRpc = reject; - }) - ); - const { unmount } = render( - - - - ); - await waitFor(() => - expect(screen.getByTestId('uid').textContent).toBe('u-rpc-err-unmount') - ); - await waitFor(() => expect(db.rpc).toHaveBeenCalled()); - unmount(); - rejectRpc(new Error('late rpc fail')); - await act(async () => { - await Promise.resolve(); - }); + try { + auth.state.session = { user: { id: 'u-rpc-err-unmount' } }; + db.rpc.mockImplementationOnce( + () => + new Promise((_resolve, reject) => { + rejectRpc = reject; + }) + ); + const { unmount } = render( + + + + ); + await waitFor(() => + expect(screen.getByTestId('uid').textContent).toBe('u-rpc-err-unmount') + ); + await waitFor(() => expect(db.rpc).toHaveBeenCalled()); + unmount(); + rejectRpc(new Error('late rpc fail')); + await act(async () => { + await Promise.resolve(); + }); - const unmountedWarnings = consoleSpy.mock.calls.filter(([message]) => - String(message).toLowerCase().includes('unmounted') - ); - expect(unmountedWarnings).toHaveLength(0); - consoleSpy.mockRestore(); + expectNoUnmountedConsoleWarnings(consoleSpy); + } finally { + consoleSpy.mockRestore(); + } }); }); diff --git a/packages/waitlist/src/hooks/useSignupMode.test.tsx b/packages/waitlist/src/hooks/useSignupMode.test.tsx index 1a0eb10e..3c53bc90 100644 --- a/packages/waitlist/src/hooks/useSignupMode.test.tsx +++ b/packages/waitlist/src/hooks/useSignupMode.test.tsx @@ -3,6 +3,15 @@ import { renderHook, waitFor } from '@testing-library/react'; import { useSignupMode } from './useSignupMode.js'; import * as client from '../waitlistClient.js'; +type ConsoleErrorSpy = ReturnType>; + +function expectNoUnmountedConsoleWarnings(consoleSpy: ConsoleErrorSpy) { + const unmountedWarnings = consoleSpy.mock.calls.filter(args => + args.some(arg => String(arg).toLowerCase().includes('unmounted')) + ); + expect(unmountedWarnings).toHaveLength(0); +} + describe('useSignupMode', () => { it('loads public settings and exposes mode flags', async () => { vi.spyOn(client, 'getPublicWaitlistSettings').mockResolvedValue({ @@ -32,21 +41,20 @@ describe('useSignupMode', () => { }) ); const consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); - const supabase = {} as never; - const { unmount } = renderHook(() => useSignupMode(supabase)); - await waitFor(() => - expect(client.getPublicWaitlistSettings).toHaveBeenCalled() - ); - unmount(); - resolveSettings({ signup_mode: 'open', copy: {}, metadata_schema: [] }); - await Promise.resolve(); + try { + const supabase = {} as never; + const { unmount } = renderHook(() => useSignupMode(supabase)); + await waitFor(() => + expect(client.getPublicWaitlistSettings).toHaveBeenCalled() + ); + unmount(); + resolveSettings({ signup_mode: 'open', copy: {}, metadata_schema: [] }); + await Promise.resolve(); - const unmountedWarnings = consoleSpy.mock.calls.filter(([message]) => - String(message).toLowerCase().includes('unmounted') - ); - expect(unmountedWarnings).toHaveLength(0); - - consoleSpy.mockRestore(); - vi.restoreAllMocks(); + expectNoUnmountedConsoleWarnings(consoleSpy); + } finally { + consoleSpy.mockRestore(); + vi.restoreAllMocks(); + } }); });