From 37c5fbd7eaa665654387dc240e3d8e1cad97c611 Mon Sep 17 00:00:00 2001 From: Michal Piechowiak Date: Mon, 27 Jan 2025 18:26:15 +0100 Subject: [PATCH 1/7] test: fail prepare fixtures script if fixtures fail to install deps or build a fixture --- tests/prepare.mjs | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/tests/prepare.mjs b/tests/prepare.mjs index 5bc35c6f1f..ebae699af6 100644 --- a/tests/prepare.mjs +++ b/tests/prepare.mjs @@ -62,6 +62,13 @@ const promises = fixtures.map((fixture) => this.push(chunk.toString().replace(/\n/gm, `\n[${fixture}] `)) callback() }, + flush(callback) { + // final transform might create non-terminated line with a prefix + // so this is just to make sure we end with a newline so further writes + // to same destination stream start on a new line for better readability + this.push('\n') + callback() + }, }) console.log(`[${fixture}] Running \`${cmd}\`...`) const output = execaCommand(cmd, { @@ -80,6 +87,11 @@ const promises = fixtures.map((fixture) => operation: 'revert', }) } + if (output.exitCode !== 0) { + const errorMessage = `[${fixture}] ๐Ÿšจ Failed to install dependencies or build a fixture` + console.error(errorMessage) + throw new Error(errorMessage) + } fixtureList.delete(fixture) }) }).finally(() => { @@ -91,5 +103,22 @@ const promises = fixtures.map((fixture) => } }), ) -await Promise.allSettled(promises) +const prepareFixturesResults = await Promise.allSettled(promises) +const failedFixturesErrors = prepareFixturesResults + .map((promise) => { + if (promise.status === 'rejected') { + return promise.reason + } + return null + }) + .filter(Boolean) + +if (failedFixturesErrors.length > 0) { + console.error('Some fixtures failed to prepare:') + for (const error of failedFixturesErrors) { + console.error(error.message) + } + process.exit(1) +} + console.log('๐ŸŽ‰ All fixtures prepared') From 0d9c1f4befd804d37b15a889c8f4899f615daec9 Mon Sep 17 00:00:00 2001 From: Michal Piechowiak Date: Mon, 27 Jan 2025 19:44:38 +0100 Subject: [PATCH 2/7] test: intentionally break deps installation / fixture build to test prepare script changes --- tests/fixtures/wasm-src/package.json | 3 ++- tests/fixtures/wasm/middleware.js | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/fixtures/wasm-src/package.json b/tests/fixtures/wasm-src/package.json index d5a533479f..30d51268f6 100644 --- a/tests/fixtures/wasm-src/package.json +++ b/tests/fixtures/wasm-src/package.json @@ -11,6 +11,7 @@ "@vercel/og": "latest", "next": "latest", "react": "18.2.0", - "react-dom": "18.2.0" + "react-dom": "18.2.0", + "package-that-does-not-exist": "^1.0.0" } } diff --git a/tests/fixtures/wasm/middleware.js b/tests/fixtures/wasm/middleware.js index 3173be4c63..11a0637f35 100644 --- a/tests/fixtures/wasm/middleware.js +++ b/tests/fixtures/wasm/middleware.js @@ -10,7 +10,7 @@ export default async function middleware(request) { const value = await increment(input) return new Response(null, { headers: { data: JSON.stringify({ input, value }) } }) } - +blah blah - this should not build export const config = { matcher: '/wasm', } From 8294b81eb725701105620c4f85b3168fe18ad4fd Mon Sep 17 00:00:00 2001 From: Michal Piechowiak Date: Mon, 27 Jan 2025 19:57:10 +0100 Subject: [PATCH 3/7] Revert "test: intentionally break deps installation / fixture build to test prepare script changes" This reverts commit 0d9c1f4befd804d37b15a889c8f4899f615daec9. --- tests/fixtures/wasm-src/package.json | 3 +-- tests/fixtures/wasm/middleware.js | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/fixtures/wasm-src/package.json b/tests/fixtures/wasm-src/package.json index 30d51268f6..d5a533479f 100644 --- a/tests/fixtures/wasm-src/package.json +++ b/tests/fixtures/wasm-src/package.json @@ -11,7 +11,6 @@ "@vercel/og": "latest", "next": "latest", "react": "18.2.0", - "react-dom": "18.2.0", - "package-that-does-not-exist": "^1.0.0" + "react-dom": "18.2.0" } } diff --git a/tests/fixtures/wasm/middleware.js b/tests/fixtures/wasm/middleware.js index 11a0637f35..3173be4c63 100644 --- a/tests/fixtures/wasm/middleware.js +++ b/tests/fixtures/wasm/middleware.js @@ -10,7 +10,7 @@ export default async function middleware(request) { const value = await increment(input) return new Response(null, { headers: { data: JSON.stringify({ input, value }) } }) } -blah blah - this should not build + export const config = { matcher: '/wasm', } From e68d57502bf78cd21e549fb6dc12be1155f0e698 Mon Sep 17 00:00:00 2001 From: Michal Piechowiak Date: Tue, 11 Feb 2025 12:14:18 +0100 Subject: [PATCH 4/7] ci: upgrade corepack --- .github/workflows/run-tests.yml | 15 +++++++++++++-- tests/prepare.mjs | 1 + 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/.github/workflows/run-tests.yml b/.github/workflows/run-tests.yml index 07f3eb0693..3163da9514 100644 --- a/.github/workflows/run-tests.yml +++ b/.github/workflows/run-tests.yml @@ -60,7 +60,9 @@ jobs: cache-dependency-path: '**/package-lock.json' - uses: oven-sh/setup-bun@v1 - name: setup pnpm/yarn - run: corepack enable + run: | + npm install -g corepack@latest + corepack enable shell: bash - name: Install Deno uses: denoland/setup-deno@v1 @@ -127,8 +129,17 @@ jobs: node-version: '18.x' cache: 'npm' cache-dependency-path: '**/package-lock.json' + - name: Prefer npm global on windows + if: runner.os == 'Windows' + # On Windows by default PATH prefers corepack bundled with Node.js + # This prepends npm global to PATH to ensure that npm installed global corepack is used instead + run: | + echo "$(npm config get prefix)" >> "$GITHUB_PATH" + shell: bash - name: setup pnpm/yarn - run: corepack enable + run: | + npm install -g corepack --force + corepack enable shell: bash - name: Install Deno uses: denoland/setup-deno@v1 diff --git a/tests/prepare.mjs b/tests/prepare.mjs index ebae699af6..2f611cd4d7 100644 --- a/tests/prepare.mjs +++ b/tests/prepare.mjs @@ -70,6 +70,7 @@ const promises = fixtures.map((fixture) => callback() }, }) + console.log(`[${fixture}] Running \`${cmd}\`...`) const output = execaCommand(cmd, { cwd, From 948fd2b42c76443e5e50c7887fc163f05c3f6e9c Mon Sep 17 00:00:00 2001 From: Michal Piechowiak Date: Wed, 12 Feb 2025 08:44:20 +0100 Subject: [PATCH 5/7] test: don't prepare e2e-only fixtures ahead of time --- tests/integration/turborepo.test.ts | 2 ++ tests/prepare.mjs | 19 +++++++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/tests/integration/turborepo.test.ts b/tests/integration/turborepo.test.ts index 442a5f0016..0e5d8d9032 100644 --- a/tests/integration/turborepo.test.ts +++ b/tests/integration/turborepo.test.ts @@ -25,6 +25,8 @@ beforeEach(async (ctx) => { // monorepo test uses process.chdir which is not working inside vite workers // so I'm disabling that test for now will revisit later in a follow up PR. // we have at least a e2e test that tests the monorepo functionality +// NOTE: turborepo-npm fixture is currently skipped in tests/prepare.mjs +// be sure to unskip it there if you would be working on making this integration test work test.skip('should create the files in the correct directories', async (ctx) => { await createFixture('turborepo-npm', ctx) await runPlugin(ctx, { PACKAGE_PATH: 'apps/web' }) diff --git a/tests/prepare.mjs b/tests/prepare.mjs index 2f611cd4d7..2c7b34d380 100644 --- a/tests/prepare.mjs +++ b/tests/prepare.mjs @@ -18,12 +18,31 @@ const NEXT_VERSION = process.env.NEXT_VERSION ?? 'latest' const fixturesDir = fileURLToPath(new URL(`./fixtures`, import.meta.url)) const fixtureFilter = argv[2] ?? '' +// E2E tests run next builds, so we don't need to prepare those ahead of time for integration tests +const e2eOnlyFixtures = new Set([ + 'after', + 'cli-before-regional-blobs-support', + 'dist-dir', + // There is also a bug on Windows on Node.js 18.20.6, that cause build failures on this fixture + // see https://github.com/opennextjs/opennextjs-netlify/actions/runs/13268839161/job/37043172448?pr=2749#step:12:78 + 'middleware-og', + 'middleware-single-matcher', + 'nx-integrated', + 'turborepo', + // We do have a skipped test using this fixture (due to integration test setup not working for monorepos) + 'turborepo-npm', + 'unstable-cache', +]) + const limit = pLimit(Math.max(2, cpus().length / 2)) const fixtures = readdirSync(fixturesDir) // Ignoring things like `.DS_Store`. .filter((fixture) => !fixture.startsWith('.')) // Applying the filter, if one is set. .filter((fixture) => !fixtureFilter || fixture.startsWith(fixtureFilter)) + // Filter out fixtures that are only needed for E2E tests + .filter((fixture) => !e2eOnlyFixtures.has(fixture)) + console.log(`๐Ÿงช Preparing fixtures: ${fixtures.join(', ')}`) const fixtureList = new Set(fixtures) const fixtureCount = fixtures.length From 49126e0127ba1900b398899efd04e30d21a72641 Mon Sep 17 00:00:00 2001 From: Michal Piechowiak Date: Wed, 12 Feb 2025 17:19:50 +0100 Subject: [PATCH 6/7] test: make global corepack update/installation commands as similar as possible --- .github/workflows/run-tests.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/run-tests.yml b/.github/workflows/run-tests.yml index 3163da9514..74d8ab61fd 100644 --- a/.github/workflows/run-tests.yml +++ b/.github/workflows/run-tests.yml @@ -61,7 +61,7 @@ jobs: - uses: oven-sh/setup-bun@v1 - name: setup pnpm/yarn run: | - npm install -g corepack@latest + npm install -g corepack corepack enable shell: bash - name: Install Deno @@ -138,6 +138,7 @@ jobs: shell: bash - name: setup pnpm/yarn run: | + # global corepack installation requires --force on Windows, otherwise EEXIST errors occur npm install -g corepack --force corepack enable shell: bash From de940d9f687f4248cb294a5ff9d66e56483dc7f1 Mon Sep 17 00:00:00 2001 From: Michal Piechowiak Date: Wed, 12 Feb 2025 17:21:10 +0100 Subject: [PATCH 7/7] test: remove integration test that was skipped for over a year, we do have e2e that assert it is working --- tests/integration/turborepo.test.ts | 48 ----------------------------- tests/prepare.mjs | 1 - 2 files changed, 49 deletions(-) delete mode 100644 tests/integration/turborepo.test.ts diff --git a/tests/integration/turborepo.test.ts b/tests/integration/turborepo.test.ts deleted file mode 100644 index 0e5d8d9032..0000000000 --- a/tests/integration/turborepo.test.ts +++ /dev/null @@ -1,48 +0,0 @@ -import { getLogger } from 'lambda-local' -import { existsSync } from 'node:fs' -import { rm } from 'node:fs/promises' -import { join } from 'node:path' -import { v4 } from 'uuid' -import { beforeEach, expect, test, vi } from 'vitest' -import { type FixtureTestContext } from '../utils/contexts.js' -import { createFixture, runPlugin } from '../utils/fixture.js' -import { generateRandomObjectID, startMockBlobStore } from '../utils/helpers.js' - -// Disable the verbose logging of the lambda-local runtime -getLogger().level = 'alert' - -beforeEach(async (ctx) => { - // set for each test a new deployID and siteID - ctx.deployID = generateRandomObjectID() - ctx.siteID = v4() - vi.stubEnv('SITE_ID', ctx.siteID) - vi.stubEnv('DEPLOY_ID', ctx.deployID) - vi.stubEnv('NETLIFY_PURGE_API_TOKEN', 'fake-token') - - await startMockBlobStore(ctx) -}) - -// monorepo test uses process.chdir which is not working inside vite workers -// so I'm disabling that test for now will revisit later in a follow up PR. -// we have at least a e2e test that tests the monorepo functionality -// NOTE: turborepo-npm fixture is currently skipped in tests/prepare.mjs -// be sure to unskip it there if you would be working on making this integration test work -test.skip('should create the files in the correct directories', async (ctx) => { - await createFixture('turborepo-npm', ctx) - await runPlugin(ctx, { PACKAGE_PATH: 'apps/web' }) - - // test if the files got generated in the correct locations - expect( - existsSync(join(ctx.cwd, '.netlify')), - 'should not have a .netlify folder in the repository root', - ).toBeFalsy() - - expect(existsSync(join(ctx.cwd, 'apps/web/.netlify'))).toBeTruthy() - - await rm(join(ctx.cwd, 'apps/web/.netlify'), { recursive: true, force: true }) - await runPlugin(ctx, { PACKAGE_PATH: 'apps/page-router' }) - - const staticPageInitial = await invokeFunction(ctx, { url: '/static/revalidate-manual' }) - console.log(staticPageInitial.body) - expect(staticPageInitial.statusCode < 400).toBeTruthy() -}) diff --git a/tests/prepare.mjs b/tests/prepare.mjs index 2c7b34d380..3316fd2382 100644 --- a/tests/prepare.mjs +++ b/tests/prepare.mjs @@ -29,7 +29,6 @@ const e2eOnlyFixtures = new Set([ 'middleware-single-matcher', 'nx-integrated', 'turborepo', - // We do have a skipped test using this fixture (due to integration test setup not working for monorepos) 'turborepo-npm', 'unstable-cache', ])