Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,10 @@ export async function getStaticPaths() {
}
}

export async function getStaticProps({ params }) {
export async function getStaticProps({ params, revalidateReason }) {
console.log(
`getStaticProps({ revalidateReason: ${JSON.stringify(revalidateReason)} })`
)
return {
props: {
params,
Expand Down
76 changes: 0 additions & 76 deletions test/integration/root-optional-revalidate/test/index.test.js

This file was deleted.

94 changes: 94 additions & 0 deletions test/integration/root-optional-revalidate/test/index.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
import type { ChildProcess } from 'child_process'
import { join } from 'path'
import cheerio from 'cheerio'
import {
killApp,
findPort,
nextBuild,
nextStart,
renderViaHTTP,
waitFor,
retry,
} from 'next-test-utils'

const appDir = join(__dirname, '../')
let app: ChildProcess | undefined
let appPort: number | undefined

const getProps = async (path) => {
const html = await renderViaHTTP(appPort, path)
const $ = cheerio.load(html)
return JSON.parse($('#props').text())
}

const runTests = () => {
it('should render / correctly', async () => {
const props = await getProps('/')
expect(props.params).toEqual({})

let cliOutput = ''
app.stdout.on('data', (chunk) => (cliOutput += chunk.toString()))
await waitFor(1000)
await getProps('/')
expect(cliOutput).toEqual('getStaticProps({ revalidateReason: "stale" })\n')
Comment on lines +32 to +33
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test checks stdout immediately after triggering revalidation, but revalidation happens asynchronously in the background. This creates a race condition where the console.log output might not have arrived at the stdout listener yet when the expectation runs.

View Details
📝 Patch Details
diff --git a/test/integration/root-optional-revalidate/test/index.test.ts b/test/integration/root-optional-revalidate/test/index.test.ts
index 1f26e7ee82..e45f575bcd 100644
--- a/test/integration/root-optional-revalidate/test/index.test.ts
+++ b/test/integration/root-optional-revalidate/test/index.test.ts
@@ -30,7 +30,11 @@ const runTests = () => {
     app.stdout.on('data', (chunk) => (cliOutput += chunk.toString()))
     await waitFor(1000)
     await getProps('/')
-    expect(cliOutput).toEqual('getStaticProps({ revalidateReason: "stale" })\n')
+    
+    // Background revalidation is async, need to wait for console.log output
+    await retry(async () => {
+      expect(cliOutput).toEqual('getStaticProps({ revalidateReason: "stale" })\n')
+    })
 
     // Rendering takes time before the new cache entry is filled
     await retry(async () => {
@@ -48,7 +52,11 @@ const runTests = () => {
     app.stdout.on('data', (chunk) => (cliOutput += chunk.toString()))
     await waitFor(1000)
     await getProps('/a')
-    expect(cliOutput).toEqual('getStaticProps({ revalidateReason: "stale" })\n')
+    
+    // Background revalidation is async, need to wait for console.log output
+    await retry(async () => {
+      expect(cliOutput).toEqual('getStaticProps({ revalidateReason: "stale" })\n')
+    })
 
     // Rendering takes time before the new cache entry is filled
     await retry(async () => {
@@ -66,7 +74,11 @@ const runTests = () => {
     app.stdout.on('data', (chunk) => (cliOutput += chunk.toString()))
     await waitFor(1000)
     await getProps('/hello/world')
-    expect(cliOutput).toEqual('getStaticProps({ revalidateReason: "stale" })\n')
+    
+    // Background revalidation is async, need to wait for console.log output
+    await retry(async () => {
+      expect(cliOutput).toEqual('getStaticProps({ revalidateReason: "stale" })\n')
+    })
 
     // Rendering takes time before the new cache entry is filled
     await retry(async () => {

Analysis

Race condition in root-optional-revalidate test - stdout check without async handling

What fails: The test checks stdout output from getStaticProps immediately after triggering revalidation, but the output hasn't arrived yet due to Next.js ISR's asynchronous background revalidation pattern.

How to reproduce: Running the tests for root-optional-revalidate will intermittently fail because the stdout output from getStaticProps({ revalidateReason: "stale" }) isn't captured when checked immediately after the HTTP request completes.

npm test -- test/integration/root-optional-revalidate/test/index.test.ts

Run this multiple times to see intermittent failures on the stdout expectation at lines 34, 52, and 70.

Result: Test fails with:

Expected: 'getStaticProps({ revalidateReason: "stale" })\n'
Received: ''

Expected: The stdout expectation should use retry() to wait for the background revalidation to complete and execute getStaticProps, similar to how the test already correctly uses retry() for checking the new props after revalidation.

Root cause: Next.js's response-cache returns the old cached entry to the HTTP client immediately via resolve(), then schedules revalidation on process.nextTick(). When the test awaits the HTTP request and immediately checks the stdout, the process.nextTick() callback hasn't executed yet, so getStaticProps hasn't been called and its console output isn't captured.

Solution: Wrap the stdout expectation in await retry() to handle the asynchronous nature of background revalidation, matching the existing pattern used for checking the updated props.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure but we can't just retry since that would hide if we're suddenly revalidating after 5s instead. There's no way around that if you're observing another process.


// Rendering takes time before the new cache entry is filled
await retry(async () => {
const newProps = await getProps('/')
expect(newProps.params).toEqual({})
expect(props.random).not.toBe(newProps.random)
})
})

it('should render /a correctly', async () => {
const props = await getProps('/a')
expect(props.params).toEqual({ slug: ['a'] })

let cliOutput = ''
app.stdout.on('data', (chunk) => (cliOutput += chunk.toString()))
await waitFor(1000)
await getProps('/a')
expect(cliOutput).toEqual('getStaticProps({ revalidateReason: "stale" })\n')

// Rendering takes time before the new cache entry is filled
await retry(async () => {
const newProps = await getProps('/a')
expect(newProps.params).toEqual({ slug: ['a'] })
expect(props.random).not.toBe(newProps.random)
})
})

it('should render /hello/world correctly', async () => {
const props = await getProps('/hello/world')
expect(props.params).toEqual({ slug: ['hello', 'world'] })

let cliOutput = ''
app.stdout.on('data', (chunk) => (cliOutput += chunk.toString()))
await waitFor(1000)
await getProps('/hello/world')
expect(cliOutput).toEqual('getStaticProps({ revalidateReason: "stale" })\n')

// Rendering takes time before the new cache entry is filled
await retry(async () => {
const newProps = await getProps('/hello/world')
expect(newProps.params).toEqual({ slug: ['hello', 'world'] })
expect(props.random).not.toBe(newProps.random)
})
})
}

describe('Root Optional Catch-all Revalidate', () => {
;(process.env.TURBOPACK_DEV ? describe.skip : describe)(
'production mode',
() => {
beforeAll(async () => {
await nextBuild(appDir)
appPort = await findPort()
app = await nextStart(appDir, appPort)
})
afterAll(() => killApp(app))

runTests()
}
)
})
Loading