Skip to content

Commit 7c3ef83

Browse files
committed
Unflake parallel-routes-revalidation test
[flakiness metric](https://app.datadoghq.com/ci/test/runs?query=test_level%3Atest%20env%3Aci%20%40git.repository.id%3Agithub.com%2Fvercel%2Fnext.js%20%40test.service%3Anextjs%20%40test.status%3Afail%20%28-%40git.branch%3A%2A%3F%2A%20OR%20%40git.branch%3Acanary%29%20%40test.name%3A%22parallel-routes-revalidation%20should%20not%20trigger%20the%20intercepted%20route%20when%20lazy-fetching%20missing%20data%22&agg_m=count&agg_m_source=base&agg_t=count&citest_explorer_sort=timestamp%2Casc&cols=%40test.status%2Ctimestamp%2C%40test.suite%3A83%2C%40test.name%3A607%2C%40git.branch&currentTab=overview&eventStack=&fromUser=false&graphType=flamegraph&index=citest&start=1738068572273&end=1740660572273&paused=false) The latest canary release [failed](https://github.com/vercel/next.js/actions/runs/13555525831/job/37890389323) because the deploy test `parallel-routes-revalidation › should not trigger the intercepted route when lazy-fetching missing data` kept failing even with retries. According to my local testing using `pnpm test-deploy test/e2e/app-dir/parallel-routes-revalidation/parallel-routes-revalidation.test.ts -t 'should not trigger the intercepted route when lazy-fetching missing data'`, the forward navigation was sometimes triggered too early after the page was reloaded. This PR introduces an option to wait for hydration (same as we do for the initial load) when reloading a page, which fixed the test for me. Note: I don't like the option name `waitHydration`, but went with it anyways because we're already using it for the `next.browser` options.
1 parent 9d1c6fd commit 7c3ef83

File tree

4 files changed

+75
-51
lines changed

4 files changed

+75
-51
lines changed

test/e2e/app-dir/parallel-routes-revalidation/parallel-routes-revalidation.test.ts

+3-2
Original file line numberDiff line numberDiff line change
@@ -174,8 +174,9 @@ describe('parallel-routes-revalidation', () => {
174174
// go back to the previous page
175175
await browser.back()
176176

177-
// reload the page, which will cause the router to no longer have cache nodes
178-
await browser.refresh()
177+
// reload the page, which will cause the router to no longer have cache
178+
// nodes. make sure the page is hydrated before going forward.
179+
await browser.refresh({ waitHydration: true })
179180

180181
// go forward, this will trigger a lazy fetch for the missing data, and should restore the detail page
181182
await browser.forward()

test/lib/browsers/base.ts

+7-1
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,10 @@ export abstract class BrowserInterface<TCurrent = any> {
8585
* Use browsers `go forward` functionality. Inverse of back.
8686
*/
8787
abstract forward(options?: any): BrowserInterface<any> & Promise<any>
88-
abstract refresh(): BrowserInterface<any> & Promise<any>
88+
abstract refresh(options?: {
89+
waitHydration?: boolean
90+
retryWaitHydration?: boolean
91+
}): BrowserInterface<any> & Promise<any>
8992
abstract setDimensions(opts: {
9093
height: number
9194
width: number
@@ -129,4 +132,7 @@ export abstract class BrowserInterface<TCurrent = any> {
129132
abstract websocketFrames(): Promise<any[]>
130133
abstract url(): Promise<string>
131134
abstract waitForIdleNetwork(): Promise<void>
135+
abstract waitForHydration(options?: {
136+
retryWaitHydration?: boolean
137+
}): Promise<void>
132138
}

test/lib/browsers/playwright.ts

+64-1
Original file line numberDiff line numberDiff line change
@@ -302,9 +302,19 @@ export class Playwright extends BrowserInterface {
302302
await page.goForward(options)
303303
})
304304
}
305-
refresh() {
305+
refresh({
306+
waitHydration,
307+
retryWaitHydration,
308+
}: {
309+
waitHydration?: boolean
310+
retryWaitHydration?: boolean
311+
} = {}) {
306312
return this.chain(async () => {
307313
await page.reload()
314+
315+
if (waitHydration) {
316+
this.waitForHydration({ retryWaitHydration })
317+
}
308318
})
309319
}
310320
setDimensions({ width, height }: { height: number; width: number }) {
@@ -518,6 +528,59 @@ export class Playwright extends BrowserInterface {
518528
})
519529
}
520530

531+
async waitForHydration({
532+
retryWaitHydration = false,
533+
}: { retryWaitHydration?: boolean } = {}): Promise<void> {
534+
const fullUrl = page.url()
535+
console.log(`\n> Waiting hydration for ${fullUrl}\n`)
536+
537+
const checkHydrated = async () => {
538+
await this.evalAsync(function () {
539+
var callback = arguments[arguments.length - 1]
540+
541+
// if it's not a Next.js app return
542+
if (
543+
!document.documentElement.innerHTML.includes('__NEXT_DATA__') &&
544+
// @ts-ignore next exists on window if it's a Next.js page.
545+
typeof ((window as any).next && (window as any).next.version) ===
546+
'undefined'
547+
) {
548+
console.log('Not a next.js page, resolving hydrate check')
549+
callback()
550+
}
551+
552+
// TODO: should we also ensure router.isReady is true
553+
// by default before resolving?
554+
if ((window as any).__NEXT_HYDRATED) {
555+
console.log('Next.js page already hydrated')
556+
callback()
557+
} else {
558+
var timeout = setTimeout(callback, 10 * 1000)
559+
;(window as any).__NEXT_HYDRATED_CB = function () {
560+
clearTimeout(timeout)
561+
console.log('Next.js hydrate callback fired')
562+
callback()
563+
}
564+
}
565+
})
566+
}
567+
568+
try {
569+
await checkHydrated()
570+
} catch (err) {
571+
if (retryWaitHydration) {
572+
// re-try in case the page reloaded during check
573+
await new Promise((resolve) => setTimeout(resolve, 2000))
574+
await checkHydrated()
575+
} else {
576+
console.error('failed to check hydration')
577+
throw err
578+
}
579+
}
580+
581+
console.log(`\n> Hydration complete for ${fullUrl}\n`)
582+
}
583+
521584
locateRedbox(): Locator {
522585
return page.locator(
523586
'nextjs-portal [aria-labelledby="nextjs__container_errors_label"]'

test/lib/next-webdriver.ts

+1-47
Original file line numberDiff line numberDiff line change
@@ -145,53 +145,7 @@ export default async function webdriver(
145145

146146
// Wait for application to hydrate
147147
if (waitHydration) {
148-
console.log(`\n> Waiting hydration for ${fullUrl}\n`)
149-
150-
const checkHydrated = async () => {
151-
await browser.evalAsync(function () {
152-
var callback = arguments[arguments.length - 1]
153-
154-
// if it's not a Next.js app return
155-
if (
156-
!document.documentElement.innerHTML.includes('__NEXT_DATA__') &&
157-
// @ts-ignore next exists on window if it's a Next.js page.
158-
typeof ((window as any).next && (window as any).next.version) ===
159-
'undefined'
160-
) {
161-
console.log('Not a next.js page, resolving hydrate check')
162-
callback()
163-
}
164-
165-
// TODO: should we also ensure router.isReady is true
166-
// by default before resolving?
167-
if ((window as any).__NEXT_HYDRATED) {
168-
console.log('Next.js page already hydrated')
169-
callback()
170-
} else {
171-
var timeout = setTimeout(callback, 10 * 1000)
172-
;(window as any).__NEXT_HYDRATED_CB = function () {
173-
clearTimeout(timeout)
174-
console.log('Next.js hydrate callback fired')
175-
callback()
176-
}
177-
}
178-
})
179-
}
180-
181-
try {
182-
await checkHydrated()
183-
} catch (err) {
184-
if (retryWaitHydration) {
185-
// re-try in case the page reloaded during check
186-
await new Promise((resolve) => setTimeout(resolve, 2000))
187-
await checkHydrated()
188-
} else {
189-
console.error('failed to check hydration')
190-
throw err
191-
}
192-
}
193-
194-
console.log(`\n> Hydration complete for ${fullUrl}\n`)
148+
await browser.waitForHydration({ retryWaitHydration })
195149
}
196150

197151
// This is a temporary workaround for turbopack starting watching too late.

0 commit comments

Comments
 (0)