From 5e984745df8f06c78e8daf625b07742b1995f51d Mon Sep 17 00:00:00 2001 From: Francesco Gringl-Novy Date: Mon, 4 May 2026 10:15:06 +0200 Subject: [PATCH] test(react-router): Fix flaky E2E tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit What was going wrong Issue #20532 is about a flaky Playwright test: should send pageload transaction in the react-router-7-framework-spa app. That app uses client-only hydration (hydrateRoot + HydratedRouter). The pageload transaction and React Router’s instrumentHydratedRouter updates (route name, sentry.origin, etc.) can land in a race with page.goto finishing: sometimes CI loses that race, so the test times out or assertions fail. This matches how other E2E apps avoid the same class of bug—for example the SvelteKit helpers wait for hydration / UI together with the pageload transaction so routing and tracing line up. What we changed After each page.goto for the performance routes, the tests now wait for the route’s

before awaiting waitForTransaction: /performance → Performance Page /performance/with/sentry → Dynamic Parameter Page So the HydratedRouter has rendered the matched route before the test blocks on the envelope, which stabilizes timing relative to router instrumentation. --- .../tests/performance/pageload.client.test.ts | 2 ++ .../tests/performance/pageload.client.test.ts | 2 ++ .../test/reactrouter-cross-usage.test.tsx | 28 ++++++++++++++++--- 3 files changed, 28 insertions(+), 4 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-framework-spa-node-20-18/tests/performance/pageload.client.test.ts b/dev-packages/e2e-tests/test-applications/react-router-7-framework-spa-node-20-18/tests/performance/pageload.client.test.ts index d32fd24c75a6..1f41355a8ebd 100644 --- a/dev-packages/e2e-tests/test-applications/react-router-7-framework-spa-node-20-18/tests/performance/pageload.client.test.ts +++ b/dev-packages/e2e-tests/test-applications/react-router-7-framework-spa-node-20-18/tests/performance/pageload.client.test.ts @@ -9,6 +9,7 @@ test.describe('client - pageload performance', () => { }); await page.goto(`/performance`); + await page.getByRole('heading', { name: 'Performance Page' }).waitFor(); const transaction = await txPromise; @@ -62,6 +63,7 @@ test.describe('client - pageload performance', () => { }); await page.goto(`/performance/with/sentry`); + await page.getByRole('heading', { name: 'Dynamic Parameter Page' }).waitFor(); const transaction = await txPromise; diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-framework-spa/tests/performance/pageload.client.test.ts b/dev-packages/e2e-tests/test-applications/react-router-7-framework-spa/tests/performance/pageload.client.test.ts index d32fd24c75a6..1f41355a8ebd 100644 --- a/dev-packages/e2e-tests/test-applications/react-router-7-framework-spa/tests/performance/pageload.client.test.ts +++ b/dev-packages/e2e-tests/test-applications/react-router-7-framework-spa/tests/performance/pageload.client.test.ts @@ -9,6 +9,7 @@ test.describe('client - pageload performance', () => { }); await page.goto(`/performance`); + await page.getByRole('heading', { name: 'Performance Page' }).waitFor(); const transaction = await txPromise; @@ -62,6 +63,7 @@ test.describe('client - pageload performance', () => { }); await page.goto(`/performance/with/sentry`); + await page.getByRole('heading', { name: 'Dynamic Parameter Page' }).waitFor(); const transaction = await txPromise; diff --git a/packages/react/test/reactrouter-cross-usage.test.tsx b/packages/react/test/reactrouter-cross-usage.test.tsx index 424821a9ad98..c158f831c381 100644 --- a/packages/react/test/reactrouter-cross-usage.test.tsx +++ b/packages/react/test/reactrouter-cross-usage.test.tsx @@ -627,9 +627,14 @@ describe('React Router cross usage of wrappers', () => { await act(async () => { router.navigate('/settings'); - await waitFor(() => expect(mockStartBrowserTracingNavigationSpan).toHaveBeenCalledTimes(1)); }); + await waitFor(() => { + expect(router.state.navigation.state).toBe('idle'); + expect(router.state.location.pathname).toBe('/settings'); + }); + await waitFor(() => expect(mockStartBrowserTracingNavigationSpan).toHaveBeenCalledTimes(1)); + expect(mockStartBrowserTracingNavigationSpan).toHaveBeenLastCalledWith(expect.any(BrowserClient), { name: '/settings', attributes: { @@ -641,9 +646,14 @@ describe('React Router cross usage of wrappers', () => { await act(async () => { router.navigate('/profile'); - await waitFor(() => expect(mockStartBrowserTracingNavigationSpan).toHaveBeenCalledTimes(2)); }); + await waitFor(() => { + expect(router.state.navigation.state).toBe('idle'); + expect(router.state.location.pathname).toBe('/profile'); + }); + await waitFor(() => expect(mockStartBrowserTracingNavigationSpan).toHaveBeenCalledTimes(2)); + expect(mockStartBrowserTracingNavigationSpan).toHaveBeenCalledTimes(2); const calls = mockStartBrowserTracingNavigationSpan.mock.calls; @@ -734,9 +744,14 @@ describe('React Router cross usage of wrappers', () => { await act(async () => { router.navigate('/user/2'); - await waitFor(() => expect(mockStartBrowserTracingNavigationSpan).toHaveBeenCalledTimes(1)); }); + await waitFor(() => { + expect(router.state.navigation.state).toBe('idle'); + expect(router.state.location.pathname).toBe('/user/2'); + }); + await waitFor(() => expect(mockStartBrowserTracingNavigationSpan).toHaveBeenCalledTimes(1)); + expect(mockStartBrowserTracingNavigationSpan).toHaveBeenCalledTimes(1); expect(mockStartBrowserTracingNavigationSpan).toHaveBeenCalledWith(expect.any(BrowserClient), { name: '/user/:id', @@ -749,9 +764,14 @@ describe('React Router cross usage of wrappers', () => { await act(async () => { router.navigate('/user/3'); - await waitFor(() => expect(mockStartBrowserTracingNavigationSpan).toHaveBeenCalledTimes(2)); }); + await waitFor(() => { + expect(router.state.navigation.state).toBe('idle'); + expect(router.state.location.pathname).toBe('/user/3'); + }); + await waitFor(() => expect(mockStartBrowserTracingNavigationSpan).toHaveBeenCalledTimes(2)); + // Should create 2 spans - different concrete paths are different user actions expect(mockStartBrowserTracingNavigationSpan).toHaveBeenCalledTimes(2); expect(mockStartBrowserTracingNavigationSpan).toHaveBeenNthCalledWith(2, expect.any(BrowserClient), {