Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Attempt to fix flakiness of prerender e2e tests #67965

Merged
merged 2 commits into from
Jul 20, 2024
Merged

Attempt to fix flakiness of prerender e2e tests #67965

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8b929ae
Attempt to fix flakiness of prerender e2e tests
unstubbable Jul 19, 2024
34bd146
Retry server readiness after `next.config` changes
unstubbable Jul 19, 2024
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
13 changes: 7 additions & 6 deletions test/e2e/app-dir/logging/fetch-logging.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,12 +235,13 @@ describe('app-dir - logging', () => {
await next.patchFile(
'app/default-cache/page.js',
(content) => content.replace('Default Cache', 'Hello!'),
async () => {
headline = await browser.waitForElementByCss('h1').text()
expect(headline).toBe('Hello!')
const logs = stripAnsi(next.cliOutput.slice(outputIndex))
expect(logs).not.toInclude(` │ GET `)
}
async () =>
retry(async () => {
headline = await browser.waitForElementByCss('h1').text()
expect(headline).toBe('Hello!')
const logs = stripAnsi(next.cliOutput.slice(outputIndex))
expect(logs).not.toInclude(` │ GET `)
})
)
})
}
Expand Down
216 changes: 100 additions & 116 deletions test/e2e/prerender.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1005,9 +1005,8 @@ describe('Prerender', () => {

if ((global as any).isNextDev) {
it('should not show warning from url prop being returned', async () => {
const urlPropPage = 'pages/url-prop.js'
await next.patchFile(
urlPropPage,
'pages/url-prop.js',
`
export async function getStaticProps() {
return {
Expand All @@ -1018,15 +1017,16 @@ describe('Prerender', () => {
}

export default ({ url }) => <p>url: {url}</p>
`
)

const html = await renderViaHTTP(next.url, '/url-prop')
await next.deleteFile(urlPropPage)
expect(next.cliOutput).not.toMatch(
/The prop `url` is a reserved prop in Next.js for legacy reasons and will be overridden on page \/url-prop/
`,
async () =>
retry(async () => {
const html = await renderViaHTTP(next.url, '/url-prop')
expect(next.cliOutput).not.toMatch(
/The prop `url` is a reserved prop in Next.js for legacy reasons and will be overridden on page \/url-prop/
)
expect(html).toMatch(/url:.*?something/)
})
)
expect(html).toMatch(/url:.*?something/)
})

it('should always show fallback for page not in getStaticPaths', async () => {
Expand Down Expand Up @@ -1080,33 +1080,30 @@ describe('Prerender', () => {
})

it('should log error in console and browser in development mode', async () => {
const indexPage = 'pages/index.js'
const origContent = await next.readFile(indexPage)

const browser = await webdriver(next.url, '/')
expect(await browser.elementByCss('p').text()).toMatch(/hello.*?world/)

await next.patchFile(
indexPage,
origContent
.replace('// throw new', 'throw new')
.replace('{/* <div', '<div')
.replace('</div> */}', '</div>')
'pages/index.js',
(content) =>
content
.replace('// throw new', 'throw new')
.replace('{/* <div', '<div')
.replace('</div> */}', '</div>'),
async () => {
await browser.waitForElementByCss('#after-change')
// we need to reload the page to trigger getStaticProps
await browser.refresh()

return retry(async () => {
await assertHasRedbox(browser)
const errOverlayContent = await getRedboxHeader(browser)
const errorMsg = /oops from getStaticProps/
expect(next.cliOutput).toMatch(errorMsg)
expect(errOverlayContent).toMatch(errorMsg)
})
}
)

try {
await browser.waitForElementByCss('#after-change')
// we need to reload the page to trigger getStaticProps
await browser.refresh()

await assertHasRedbox(browser)
const errOverlayContent = await getRedboxHeader(browser)
const errorMsg = /oops from getStaticProps/
expect(next.cliOutput).toMatch(errorMsg)
expect(errOverlayContent).toMatch(errorMsg)
} finally {
await next.patchFile(indexPage, origContent)
}
})

it('should always call getStaticProps without caching in dev', async () => {
Expand All @@ -1133,25 +1130,20 @@ describe('Prerender', () => {
})

it('should error on bad object from getStaticProps', async () => {
const indexPage = 'pages/index.js'
const origContent = await next.readFile(indexPage)
await next.patchFile(
indexPage,
origContent.replace(/\/\/ bad-prop/, 'another: true,')
'pages/index.js',
(content) => content.replace(/\/\/ bad-prop/, 'another: true,'),
async () =>
retry(async () => {
const html = await renderViaHTTP(next.url, '/')
expect(html).toMatch(/Additional keys were returned/)
})
)
await waitFor(1000)
try {
const html = await renderViaHTTP(next.url, '/')
expect(html).toMatch(/Additional keys were returned/)
} finally {
await next.patchFile(indexPage, origContent)
}
})

it('should error on dynamic page without getStaticPaths', async () => {
const curPage = 'pages/temp/[slug].js'
await next.patchFile(
curPage,
'pages/temp/[slug].js',
`
export async function getStaticProps() {
return {
Expand All @@ -1161,23 +1153,20 @@ describe('Prerender', () => {
}
}
export default () => 'oops'
`
`,
async () =>
retry(async () => {
const html = await renderViaHTTP(next.url, '/temp/hello')
expect(html).toMatch(
/getStaticPaths is required for dynamic SSG pages and is missing for/
)
})
)
await waitFor(1000)
try {
const html = await renderViaHTTP(next.url, '/temp/hello')
expect(html).toMatch(
/getStaticPaths is required for dynamic SSG pages and is missing for/
)
} finally {
await next.deleteFile(curPage)
}
})

it('should error on dynamic page without getStaticPaths returning fallback property', async () => {
const curPage = 'pages/temp2/[slug].js'
await next.patchFile(
curPage,
'pages/temp2/[slug].js',
`
export async function getStaticPaths() {
return {
Expand All @@ -1192,15 +1181,13 @@ describe('Prerender', () => {
}
}
export default () => 'oops'
`
`,
async () =>
retry(async () => {
const html = await renderViaHTTP(next.url, '/temp2/hello')
expect(html).toMatch(/`fallback` key must be returned from/)
})
)
await waitFor(1000)
try {
const html = await renderViaHTTP(next.url, '/temp2/hello')
expect(html).toMatch(/`fallback` key must be returned from/)
} finally {
await next.deleteFile(curPage)
}
})

it('should not re-call getStaticProps when updating query', async () => {
Expand Down Expand Up @@ -2196,26 +2183,23 @@ describe('Prerender', () => {

if (!isDev && !isDeploy) {
it('should automatically reset cache TTL when an error occurs and build cache was available', async () => {
await next.patchFile('error.txt', 'yes')
await waitFor(2000)
await next.patchFile('error.txt', 'yes', async () => {
await waitFor(2000)

for (let i = 0; i < 5; i++) {
const res = await fetchViaHTTP(
next.url,
'/blocking-fallback/test-errors-1'
)
expect(res.status).toBe(200)
}
await next.deleteFile('error.txt')
await check(
() =>
next.cliOutput.match(
for (let i = 0; i < 5; i++) {
const res = await fetchViaHTTP(
next.url,
'/blocking-fallback/test-errors-1'
)
expect(res.status).toBe(200)
}

return retry(async () => {
expect(next.cliOutput).toMatch(
/throwing error for \/blocking-fallback\/test-errors-1/
).length === 1
? 'success'
: next.cliOutput,
'success'
)
)
})
})
})

it('should automatically reset cache TTL when an error occurs and runtime cache was available', async () => {
Expand All @@ -2226,26 +2210,22 @@ describe('Prerender', () => {

expect(res.status).toBe(200)
await waitFor(2000)
await next.patchFile('error.txt', 'yes')

for (let i = 0; i < 5; i++) {
const res = await fetchViaHTTP(
next.url,
'/blocking-fallback/test-errors-2'
)
expect(res.status).toBe(200)
}
await next.deleteFile('error.txt')
await next.patchFile('error.txt', 'yes', async () => {
for (let i = 0; i < 5; i++) {
const res = await fetchViaHTTP(
next.url,
'/blocking-fallback/test-errors-2'
)
expect(res.status).toBe(200)
}

await check(
() =>
next.cliOutput.match(
return retry(async () => {
expect(next.cliOutput).toMatch(
/throwing error for \/blocking-fallback\/test-errors-2/
).length === 1
? 'success'
: next.cliOutput,
'success'
)
)
})
})
})

it('should not on-demand revalidate for fallback: blocking with onlyGenerated if not generated', async () => {
Expand Down Expand Up @@ -2291,15 +2271,17 @@ describe('Prerender', () => {
expect($('p').text()).toMatch(/Post:.*?test-if-generated-2/)
expect(res.headers.get('x-nextjs-cache')).toMatch(/MISS/)

const res2 = await fetchViaHTTP(
next.url,
'/blocking-fallback/test-if-generated-2'
)
const html2 = await res2.text()
const $2 = cheerio.load(html2)
await retry(async () => {
const res2 = await fetchViaHTTP(
next.url,
'/blocking-fallback/test-if-generated-2'
)
const html2 = await res2.text()
const $2 = cheerio.load(html2)

expect(initialTime).toBe($2('#time').text())
expect(res2.headers.get('x-nextjs-cache')).toMatch(/(HIT|STALE)/)
expect(initialTime).toBe($2('#time').text())
expect(res2.headers.get('x-nextjs-cache')).toMatch(/(HIT|STALE)/)
})

const res3 = await fetchViaHTTP(
next.url,
Expand All @@ -2315,14 +2297,16 @@ describe('Prerender', () => {
const revalidateData = await res3.json()
expect(revalidateData.revalidated).toBe(true)

const res4 = await fetchViaHTTP(
next.url,
'/blocking-fallback/test-if-generated-2'
)
const html4 = await res4.text()
const $4 = cheerio.load(html4)
expect($4('#time').text()).not.toBe(initialTime)
expect(res4.headers.get('x-nextjs-cache')).toMatch(/(HIT|STALE)/)
await retry(async () => {
const res4 = await fetchViaHTTP(
next.url,
'/blocking-fallback/test-if-generated-2'
)
const html4 = await res4.text()
const $4 = cheerio.load(html4)
expect($4('#time').text()).not.toBe(initialTime)
expect(res4.headers.get('x-nextjs-cache')).toMatch(/(HIT|STALE)/)
})
})

it('should on-demand revalidate for revalidate: false', async () => {
Expand Down
10 changes: 5 additions & 5 deletions test/lib/next-modes/base.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { ChildProcess } from 'child_process'
import { createNextInstall } from '../create-next-install'
import { Span } from 'next/dist/trace'
import webdriver from '../next-webdriver'
import { renderViaHTTP, fetchViaHTTP, findPort, retry } from 'next-test-utils'
import { renderViaHTTP, fetchViaHTTP, findPort } from 'next-test-utils'
import cheerio from 'cheerio'
import { once } from 'events'
import { BrowserInterface } from '../browsers/base'
Expand Down Expand Up @@ -69,7 +69,7 @@ export class NextInstance {
public env: Record<string, string>
public forcedPort?: string
public dirSuffix: string = ''
public serverReadyPattern?: RegExp = /^\s* ✓ Ready in /
public serverReadyPattern?: RegExp = / ✓ Ready in /

constructor(opts: NextInstanceOpts) {
this.env = {}
Expand Down Expand Up @@ -491,7 +491,7 @@ export class NextInstance {
public async patchFile(
filename: string,
content: string | ((content: string) => string),
retryWithTempContent?: (context: { newFile: boolean }) => Promise<void>
runWithTempContent?: (context: { newFile: boolean }) => Promise<void>
): Promise<{ newFile: boolean }> {
const outputPath = path.join(this.testDir, filename)
const newFile = !existsSync(outputPath)
Expand All @@ -503,9 +503,9 @@ export class NextInstance {
typeof content === 'function' ? content(previousContent) : content
)

if (retryWithTempContent) {
if (runWithTempContent) {
try {
await retry(() => retryWithTempContent({ newFile }))
await runWithTempContent({ newFile })
} finally {
if (previousContent === undefined) {
await fs.rm(outputPath)
Expand Down
Loading
Loading