From 0281b0729cc90a2c910ab034d0b148e70a1d3de5 Mon Sep 17 00:00:00 2001 From: panteliselef Date: Wed, 29 Jan 2025 15:53:18 +0200 Subject: [PATCH 1/2] test(nextjs): E2E tests for middleware file placement --- .changeset/selfish-news-invent.md | 2 + integration/models/application.ts | 9 ++ integration/models/applicationConfig.ts | 38 ++++-- .../tests/middleware-placement.test.ts | 115 ++++++++++++++++++ 4 files changed, 154 insertions(+), 10 deletions(-) create mode 100644 .changeset/selfish-news-invent.md create mode 100644 integration/tests/middleware-placement.test.ts diff --git a/.changeset/selfish-news-invent.md b/.changeset/selfish-news-invent.md new file mode 100644 index 00000000000..a845151cc84 --- /dev/null +++ b/.changeset/selfish-news-invent.md @@ -0,0 +1,2 @@ +--- +--- diff --git a/integration/models/application.ts b/integration/models/application.ts index 3d354a04a4e..5e7e3915710 100644 --- a/integration/models/application.ts +++ b/integration/models/application.ts @@ -20,6 +20,7 @@ export const application = ( const stdoutFilePath = path.resolve(appDirPath, `e2e.${now}.log`); const stderrFilePath = path.resolve(appDirPath, `e2e.${now}.err.log`); let buildOutput = ''; + let serveOutput = ''; const self = { name, @@ -93,7 +94,11 @@ export const application = ( get buildOutput() { return buildOutput; }, + get serveOutput() { + return serveOutput; + }, serve: async (opts: { port?: number; manualStart?: boolean } = {}) => { + const log = logger.child({ prefix: 'serve' }).info; const port = opts.port || (await getPort()); // TODO: get serverUrl as in dev() const serverUrl = `http://localhost:${port}`; @@ -102,6 +107,10 @@ export const application = ( const proc = run(scripts.serve, { cwd: appDirPath, env: { PORT: port.toString() }, + log: (msg: string) => { + serveOutput += `\n${msg}`; + log(msg); + }, }); cleanupFns.push(() => awaitableTreekill(proc.pid, 'SIGKILL')); await waitForIdleProcess(proc); diff --git a/integration/models/applicationConfig.ts b/integration/models/applicationConfig.ts index c3366a14187..0894b7ae33a 100644 --- a/integration/models/applicationConfig.ts +++ b/integration/models/applicationConfig.ts @@ -13,7 +13,7 @@ type Scripts = { dev: string; build: string; setup: string; serve: string }; export const applicationConfig = () => { let name = ''; let serverUrl = ''; - const templates: string[] = []; + let template: string; const files = new Map(); const scripts: Scripts = { dev: 'pnpm dev', serve: 'pnpm serve', build: 'pnpm build', setup: 'pnpm install' }; const envFormatters = { public: (key: string) => key, private: (key: string) => key }; @@ -26,7 +26,9 @@ export const applicationConfig = () => { clone.setName(name); clone.setEnvFormatter('public', envFormatters.public); clone.setEnvFormatter('private', envFormatters.private); - templates.forEach(t => clone.useTemplate(t)); + if (template) { + clone.useTemplate(template); + } dependencies.forEach((v, k) => clone.addDependency(k, v)); Object.entries(scripts).forEach(([k, v]) => clone.addScript(k as keyof typeof scripts, v)); files.forEach((v, k) => clone.addFile(k, () => v)); @@ -44,8 +46,12 @@ export const applicationConfig = () => { files.set(filePath, cbOrPath(helpers)); return self; }, + removeFile: (filePath: string) => { + files.set(filePath, ''); + return self; + }, useTemplate: (path: string) => { - templates.push(path); + template = path; return self; }, setEnvFormatter: (type: keyof typeof envFormatters, formatter: typeof envFormatters.public) => { @@ -76,20 +82,32 @@ export const applicationConfig = () => { const appDirPath = path.resolve(constants.TMP_DIR, appDirName); // Copy template files - for (const template of templates) { + if (template) { logger.info(`Copying template "${path.basename(template)}" -> ${appDirPath}`); await fs.ensureDir(appDirPath); await fs.copy(template, appDirPath, { overwrite: true, filter: (p: string) => !p.includes('node_modules') }); } + await Promise.all( + [...files] + .filter(([, contents]) => !contents) + .map(async ([pathname]) => { + const dest = path.resolve(appDirPath, pathname); + logger.info(`Deleting file ${dest}`); + await fs.remove(dest); + }), + ); + // Create individual files await Promise.all( - [...files].map(async ([pathname, contents]) => { - const dest = path.resolve(appDirPath, pathname); - logger.info(`Copying file "${pathname}" -> ${dest}`); - await fs.ensureFile(dest); - await fs.writeFile(dest, contents); - }), + [...files] + .filter(([, contents]) => contents) + .map(async ([pathname, contents]) => { + const dest = path.resolve(appDirPath, pathname); + logger.info(`Copying file "${pathname}" -> ${dest}`); + await fs.ensureFile(dest); + await fs.writeFile(dest, contents); + }), ); // Adjust package.json dependencies diff --git a/integration/tests/middleware-placement.test.ts b/integration/tests/middleware-placement.test.ts new file mode 100644 index 00000000000..c5c814f97fd --- /dev/null +++ b/integration/tests/middleware-placement.test.ts @@ -0,0 +1,115 @@ +import { expect, test } from '@playwright/test'; + +import type { Application } from '../models/application'; +import { appConfigs } from '../presets'; +import { createTestUtils } from '../testUtils'; + +const commonSetup = appConfigs.next.appRouterQuickstart.clone().removeFile('src/middleware.ts'); + +test.describe('next start - missing middleware @quickstart', () => { + test.describe.configure({ mode: 'parallel' }); + let app: Application; + + test.beforeAll(async () => { + app = await commonSetup.commit(); + await app.setup(); + await app.withEnv(appConfigs.envs.withEmailCodesQuickstart); + await app.build(); + await app.serve(); + }); + + test.afterAll(async () => { + await app.teardown(); + }); + + test('Display error for missing middleware', async ({ page, context }) => { + const u = createTestUtils({ app, page, context }); + await u.page.goToAppHome(); + + expect(app.serveOutput).toContain('Your Middleware exists at ./src/middleware.(ts|js)'); + }); +}); + +test.describe('next start - invalid middleware at root on src/ @quickstart', () => { + test.describe.configure({ mode: 'parallel' }); + let app: Application; + + test.beforeAll(async () => { + app = await commonSetup + .addFile( + 'middleware.ts', + () => ` +import { clerkMiddleware } from '@clerk/nextjs/server'; +export default clerkMiddleware(); + +export const config = { + matcher: ['/((?!.*\\..*|_next).*)', '/', '/(api|trpc)(.*)'], +}; + `, + ) + .commit(); + await app.setup(); + await app.withEnv(appConfigs.envs.withEmailCodesQuickstart); + await app.build(); + await app.serve(); + }); + + test.afterAll(async () => { + await app.teardown(); + }); + + test('Display suggestion for moving middleware to from `./middleware.ts` to `./src/middleware.ts`', async ({ + page, + context, + }) => { + const u = createTestUtils({ app, page, context }); + await u.page.goToAppHome(); + + expect(app.serveOutput).not.toContain('Your Middleware exists at ./src/middleware.(ts|js)'); + expect(app.serveOutput).toContain( + 'Clerk: clerkMiddleware() was not run, your middleware file might be misplaced. Move your middleware file to ./src/middleware.ts. Currently located at ./middleware.ts', + ); + }); +}); + +test.describe('next start - invalid middleware inside app on src/ @quickstart', () => { + test.describe.configure({ mode: 'parallel' }); + let app: Application; + + test.beforeAll(async () => { + app = await commonSetup + .addFile( + 'src/app/middleware.ts', + () => ` +import { clerkMiddleware } from '@clerk/nextjs/server'; +export default clerkMiddleware(); + +export const config = { + matcher: ['/((?!.*\\..*|_next).*)', '/', '/(api|trpc)(.*)'], +}; + `, + ) + .commit(); + await app.setup(); + await app.withEnv(appConfigs.envs.withEmailCodesQuickstart); + // await app.dev({ allowFailures: true }); + await app.build(); + await app.serve(); + }); + + test.afterAll(async () => { + await app.teardown(); + }); + + test('Display suggestion for moving middleware to from `./src/app/middleware.ts` to `./src/middleware.ts`', async ({ + page, + context, + }) => { + const u = createTestUtils({ app, page, context }); + await u.page.goToAppHome(); + expect(app.serveOutput).not.toContain('Your Middleware exists at ./src/middleware.(ts|js)'); + expect(app.serveOutput).toContain( + 'Clerk: clerkMiddleware() was not run, your middleware file might be misplaced. Move your middleware file to ./src/middleware.ts. Currently located at ./src/app/middleware.ts', + ); + }); +}); From 4b8c0ece14ad2ef163b18b33ca6686a1cde0772d Mon Sep 17 00:00:00 2001 From: panteliselef Date: Thu, 30 Jan 2025 12:32:15 +0200 Subject: [PATCH 2/2] cleanup --- .../tests/middleware-placement.test.ts | 38 ++++++------------- 1 file changed, 11 insertions(+), 27 deletions(-) diff --git a/integration/tests/middleware-placement.test.ts b/integration/tests/middleware-placement.test.ts index c5c814f97fd..bb3d7e7548a 100644 --- a/integration/tests/middleware-placement.test.ts +++ b/integration/tests/middleware-placement.test.ts @@ -4,6 +4,15 @@ import type { Application } from '../models/application'; import { appConfigs } from '../presets'; import { createTestUtils } from '../testUtils'; +const middlewareFileContents = ` +import { clerkMiddleware } from '@clerk/nextjs/server'; +export default clerkMiddleware(); + +export const config = { + matcher: ['/((?!.*\\..*|_next).*)', '/', '/(api|trpc)(.*)'], +}; +`; + const commonSetup = appConfigs.next.appRouterQuickstart.clone().removeFile('src/middleware.ts'); test.describe('next start - missing middleware @quickstart', () => { @@ -35,19 +44,7 @@ test.describe('next start - invalid middleware at root on src/ @quickstart', () let app: Application; test.beforeAll(async () => { - app = await commonSetup - .addFile( - 'middleware.ts', - () => ` -import { clerkMiddleware } from '@clerk/nextjs/server'; -export default clerkMiddleware(); - -export const config = { - matcher: ['/((?!.*\\..*|_next).*)', '/', '/(api|trpc)(.*)'], -}; - `, - ) - .commit(); + app = await commonSetup.addFile('middleware.ts', () => middlewareFileContents).commit(); await app.setup(); await app.withEnv(appConfigs.envs.withEmailCodesQuickstart); await app.build(); @@ -77,22 +74,9 @@ test.describe('next start - invalid middleware inside app on src/ @quickstart', let app: Application; test.beforeAll(async () => { - app = await commonSetup - .addFile( - 'src/app/middleware.ts', - () => ` -import { clerkMiddleware } from '@clerk/nextjs/server'; -export default clerkMiddleware(); - -export const config = { - matcher: ['/((?!.*\\..*|_next).*)', '/', '/(api|trpc)(.*)'], -}; - `, - ) - .commit(); + app = await commonSetup.addFile('src/app/middleware.ts', () => middlewareFileContents).commit(); await app.setup(); await app.withEnv(appConfigs.envs.withEmailCodesQuickstart); - // await app.dev({ allowFailures: true }); await app.build(); await app.serve(); });