From 5a318abb2bd31b3404d4c162c0dabff9de84110f Mon Sep 17 00:00:00 2001 From: Philippe Serhal Date: Thu, 12 Sep 2024 09:36:45 -0400 Subject: [PATCH 1/3] test(e2e): fix failures due to latest and canary releases [v14.2.10](https://github.com/vercel/next.js/releases/tag/v14.2.10) and [v15.0.0-canary.147](https://github.com/vercel/next.js/releases/tag/v15.0.0-canary.147) included https://github.com/vercel/next.js/pull/69802 which added `private` back into `no-cache,no-store` `cache-control` headers in some cases. --- .../cli-before-regional-blobs-support.test.ts | 14 ++++++++-- tests/e2e/page-router.test.ts | 28 ++++++++++++++++--- tests/e2e/simple-app.test.ts | 8 ++++-- 3 files changed, 42 insertions(+), 8 deletions(-) diff --git a/tests/e2e/cli-before-regional-blobs-support.test.ts b/tests/e2e/cli-before-regional-blobs-support.test.ts index 0e3b310d4e..e5b72b203b 100644 --- a/tests/e2e/cli-before-regional-blobs-support.test.ts +++ b/tests/e2e/cli-before-regional-blobs-support.test.ts @@ -1,5 +1,6 @@ import { expect } from '@playwright/test' import { test } from '../utils/playwright-helpers.js' +import { nextVersionSatisfies } from '../utils/next-version-helpers.mjs' test('should serve 404 page when requesting non existing page (no matching route) if site is deployed with CLI not supporting regional blobs', async ({ page, @@ -18,8 +19,17 @@ test('should serve 404 page when requesting non existing page (no matching route expect(await page.textContent('h1')).toBe('404') + // https://github.com/vercel/next.js/pull/69802 made changes to returned cache-control header, + // after that (14.2.10 and canary.147) 404 pages would have `private` directive, before that it + // would not + const shouldHavePrivateDirective = nextVersionSatisfies( + '>=14.2.10 <15.0.0 || >=15.0.0-canary.147', + ) expect(headers['netlify-cdn-cache-control']).toBe( - 'no-cache, no-store, max-age=0, must-revalidate, durable', + (shouldHavePrivateDirective ? 'private, ' : '') + + 'private, no-cache, no-store, max-age=0, must-revalidate, durable', + ) + expect(headers['cache-control']).toBe( + (shouldHavePrivateDirective ? 'private,' : '') + 'no-cache,no-store,max-age=0,must-revalidate', ) - expect(headers['cache-control']).toBe('no-cache,no-store,max-age=0,must-revalidate') }) diff --git a/tests/e2e/page-router.test.ts b/tests/e2e/page-router.test.ts index 895a2025fb..10a3e778b8 100644 --- a/tests/e2e/page-router.test.ts +++ b/tests/e2e/page-router.test.ts @@ -1,5 +1,6 @@ import { expect } from '@playwright/test' import { test } from '../utils/playwright-helpers.js' +import { nextVersionSatisfies } from '../utils/next-version-helpers.mjs' export function waitFor(millis: number) { return new Promise((resolve) => setTimeout(resolve, millis)) @@ -340,10 +341,20 @@ test.describe('Simple Page Router (no basePath, no i18n)', () => { expect(await page.textContent('h1')).toBe('404') + // https://github.com/vercel/next.js/pull/69802 made changes to returned cache-control header, + // after that (14.2.10 and canary.147) 404 pages would have `private` directive, before that + // it would not + const shouldHavePrivateDirective = nextVersionSatisfies( + '>=14.2.10 <15.0.0 || >=15.0.0-canary.147', + ) expect(headers['netlify-cdn-cache-control']).toBe( - 'no-cache, no-store, max-age=0, must-revalidate, durable', + (shouldHavePrivateDirective ? 'private, ' : '') + + 'no-cache, no-store, max-age=0, must-revalidate, durable', + ) + expect(headers['cache-control']).toBe( + (shouldHavePrivateDirective ? 'private,' : '') + + 'no-cache,no-store,max-age=0,must-revalidate', ) - expect(headers['cache-control']).toBe('no-cache,no-store,max-age=0,must-revalidate') }) test('should serve 404 page when requesting non existing page (marked with notFound: true in getStaticProps)', async ({ @@ -1039,10 +1050,19 @@ test.describe('Page Router with basePath and i18n', () => { expect(await page.textContent('h1')).toBe('404') + // https://github.com/vercel/next.js/pull/69802 made changes to returned cache-control header, + // after that 404 pages would have `private` directive, before that it would not + const shouldHavePrivateDirective = nextVersionSatisfies( + '>=14.2.10 <15.0.0 || >=15.0.0-canary.147', + ) expect(headers['netlify-cdn-cache-control']).toBe( - 'no-cache, no-store, max-age=0, must-revalidate, durable', + (shouldHavePrivateDirective ? 'private, ' : '') + + 'no-cache, no-store, max-age=0, must-revalidate, durable', + ) + expect(headers['cache-control']).toBe( + (shouldHavePrivateDirective ? 'private,' : '') + + 'no-cache,no-store,max-age=0,must-revalidate', ) - expect(headers['cache-control']).toBe('no-cache,no-store,max-age=0,must-revalidate') }) test('requesting a non existing page route that needs to be fetched from the blob store like 404.html (notFound: true)', async ({ diff --git a/tests/e2e/simple-app.test.ts b/tests/e2e/simple-app.test.ts index 7fc38bb5a7..af1c6f172a 100644 --- a/tests/e2e/simple-app.test.ts +++ b/tests/e2e/simple-app.test.ts @@ -220,8 +220,12 @@ test('requesting a non existing page route that needs to be fetched from the blo expect(await page.textContent('h1')).toBe('404 Not Found') // https://github.com/vercel/next.js/pull/66674 made changes to returned cache-control header, - // before that 404 page would have `private` directive, after that it would not - const shouldHavePrivateDirective = !nextVersionSatisfies('>=14.2.4 <15.0.0 || >=15.0.0-canary.24') + // before that 404 page would have `private` directive, after that (14.2.4 and canary.24) it + // would not ... and then https://github.com/vercel/next.js/pull/69802 changed it back again + // (14.2.10 and canary.147) + const shouldHavePrivateDirective = nextVersionSatisfies( + '<14.2.4 || >=14.2.10 < 15 || <15.0.0-canary.24 || >= 15.0.0-canary.147', + ) expect(headers['netlify-cdn-cache-control']).toBe( (shouldHavePrivateDirective ? 'private, ' : '') + From d8f0bbaf8f763b9bc1b8db7cdf69a7823bc97194 Mon Sep 17 00:00:00 2001 From: Philippe Serhal Date: Tue, 17 Sep 2024 12:41:03 -0400 Subject: [PATCH 2/3] fix: fix private directive logic in tests --- tests/e2e/cli-before-regional-blobs-support.test.ts | 6 ++---- tests/e2e/page-router.test.ts | 8 ++------ tests/e2e/simple-app.test.ts | 2 +- 3 files changed, 5 insertions(+), 11 deletions(-) diff --git a/tests/e2e/cli-before-regional-blobs-support.test.ts b/tests/e2e/cli-before-regional-blobs-support.test.ts index e5b72b203b..ce2e5f4b5a 100644 --- a/tests/e2e/cli-before-regional-blobs-support.test.ts +++ b/tests/e2e/cli-before-regional-blobs-support.test.ts @@ -22,12 +22,10 @@ test('should serve 404 page when requesting non existing page (no matching route // https://github.com/vercel/next.js/pull/69802 made changes to returned cache-control header, // after that (14.2.10 and canary.147) 404 pages would have `private` directive, before that it // would not - const shouldHavePrivateDirective = nextVersionSatisfies( - '>=14.2.10 <15.0.0 || >=15.0.0-canary.147', - ) + const shouldHavePrivateDirective = nextVersionSatisfies('^14.2.10 || >=15.0.0-canary.147') expect(headers['netlify-cdn-cache-control']).toBe( (shouldHavePrivateDirective ? 'private, ' : '') + - 'private, no-cache, no-store, max-age=0, must-revalidate, durable', + 'no-cache, no-store, max-age=0, must-revalidate, durable', ) expect(headers['cache-control']).toBe( (shouldHavePrivateDirective ? 'private,' : '') + 'no-cache,no-store,max-age=0,must-revalidate', diff --git a/tests/e2e/page-router.test.ts b/tests/e2e/page-router.test.ts index 10a3e778b8..790eb19ac3 100644 --- a/tests/e2e/page-router.test.ts +++ b/tests/e2e/page-router.test.ts @@ -344,9 +344,7 @@ test.describe('Simple Page Router (no basePath, no i18n)', () => { // https://github.com/vercel/next.js/pull/69802 made changes to returned cache-control header, // after that (14.2.10 and canary.147) 404 pages would have `private` directive, before that // it would not - const shouldHavePrivateDirective = nextVersionSatisfies( - '>=14.2.10 <15.0.0 || >=15.0.0-canary.147', - ) + const shouldHavePrivateDirective = nextVersionSatisfies('^14.2.10 || >=15.0.0-canary.147') expect(headers['netlify-cdn-cache-control']).toBe( (shouldHavePrivateDirective ? 'private, ' : '') + 'no-cache, no-store, max-age=0, must-revalidate, durable', @@ -1052,9 +1050,7 @@ test.describe('Page Router with basePath and i18n', () => { // https://github.com/vercel/next.js/pull/69802 made changes to returned cache-control header, // after that 404 pages would have `private` directive, before that it would not - const shouldHavePrivateDirective = nextVersionSatisfies( - '>=14.2.10 <15.0.0 || >=15.0.0-canary.147', - ) + const shouldHavePrivateDirective = nextVersionSatisfies('^14.2.10 || >=15.0.0-canary.147') expect(headers['netlify-cdn-cache-control']).toBe( (shouldHavePrivateDirective ? 'private, ' : '') + 'no-cache, no-store, max-age=0, must-revalidate, durable', diff --git a/tests/e2e/simple-app.test.ts b/tests/e2e/simple-app.test.ts index af1c6f172a..98058eb3af 100644 --- a/tests/e2e/simple-app.test.ts +++ b/tests/e2e/simple-app.test.ts @@ -224,7 +224,7 @@ test('requesting a non existing page route that needs to be fetched from the blo // would not ... and then https://github.com/vercel/next.js/pull/69802 changed it back again // (14.2.10 and canary.147) const shouldHavePrivateDirective = nextVersionSatisfies( - '<14.2.4 || >=14.2.10 < 15 || <15.0.0-canary.24 || >= 15.0.0-canary.147', + '<14.2.4 || >=14.2.10 <15.0.0-canary.24 || ^15.0.0-canary.147', ) expect(headers['netlify-cdn-cache-control']).toBe( From b8110b13299fab334279327d3ddcac8e475ba943 Mon Sep 17 00:00:00 2001 From: Philippe Serhal Date: Tue, 17 Sep 2024 12:56:09 -0400 Subject: [PATCH 3/3] test: update one more no-cache test --- tests/integration/static.test.ts | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/tests/integration/static.test.ts b/tests/integration/static.test.ts index b4f6d6ade4..a312745630 100644 --- a/tests/integration/static.test.ts +++ b/tests/integration/static.test.ts @@ -13,6 +13,7 @@ import { getBlobEntries, startMockBlobStore, } from '../utils/helpers.js' +import { nextVersionSatisfies } from '../utils/next-version-helpers.mjs' // Disable the verbose logging of the lambda-local runtime getLogger().level = 'alert' @@ -50,9 +51,16 @@ test('requesting a non existing page route that needs to be const call1 = await invokeFunction(ctx, { url: 'static/revalidate-not-existing' }) expect(call1.statusCode).toBe(404) expect(load(call1.body)('h1').text()).toBe('404') + + // https://github.com/vercel/next.js/pull/69802 made changes to returned cache-control header, + // after that (14.2.10 and canary.147) 404 pages would have `private` directive, before that it + // would not + const shouldHavePrivateDirective = nextVersionSatisfies('^14.2.10 || >=15.0.0-canary.147') expect(call1.headers, 'a cache hit on the first invocation of a prerendered page').toEqual( expect.objectContaining({ - 'netlify-cdn-cache-control': 'no-cache, no-store, max-age=0, must-revalidate, durable', + 'netlify-cdn-cache-control': + (shouldHavePrivateDirective ? 'private, ' : '') + + 'no-cache, no-store, max-age=0, must-revalidate, durable', }), ) })