fix concurrency issue and invalid snapshots

vercel · Apr 12, 2023 · d84982d · d84982d
1 parent ea9f891
commit d84982d
Show file tree

Hide file tree

Showing 13 changed files with 62 additions and 31 deletions.
diff --git a/.github/workflows/build_test_deploy_new.yml b/.github/workflows/build_test_deploy_new.yml
@@ -11,7 +11,9 @@ env:
   PNPM_VERSION: 7.24.3
   NODE_MAINTENANCE_VERSION: 16
   NODE_LTS_VERSION: 18
-  TEST_CONCURRENCY: 10
+  TEST_CONCURRENCY: 6
+  # TODO: remove after testing
+  NEXT_TEST_CONTINUE_ON_ERROR: 'true'
 
 jobs:
   build:
@@ -61,10 +63,11 @@ jobs:
 
       - run: rm -rf /tmp/next-repo-*; rm -rf /tmp/next-install-*
 
+      - run: node run-tests.js --type unit -c ${TEST_CONCURRENCY}
+        name: Run development/ tests
+
       - run: node run-tests.js --type development -c ${TEST_CONCURRENCY}
         name: Run development/ tests
-        env:
-          NEXT_TEST_CONTINUE_ON_ERROR: 'true'
 
       - run: node run-tests.js --type production -c ${TEST_CONCURRENCY}
         name: Run production/ tests

diff --git a/run-tests.js b/run-tests.js
@@ -375,9 +375,14 @@ async function main() {
     testNames.map(async (test) => {
       const dirName = path.dirname(test)
       let dirSema = directorySemas.get(dirName)
-      if (dirSema === undefined)
+
+      // we only restrict 1 test per directory for
+      // legacy integration tests
+      if (!testType && dirSema === undefined) {
         directorySemas.set(dirName, (dirSema = new Sema(1)))
-      await dirSema.acquire()
+      }
+      if (dirSema) await dirSema.acquire()
+
       await sema.acquire()
       let passed = false
 
@@ -454,7 +459,7 @@ async function main() {
       }
 
       sema.release()
-      dirSema.release()
+      if (dirSema) dirSema.release()
     })
   )
 

diff --git a/test/development/acceptance-app/ReactRefreshLogBox.test.ts b/test/development/acceptance-app/ReactRefreshLogBox.test.ts
@@ -226,7 +226,7 @@ for (const variant of ['default', 'turbo']) {
       expect(await session.hasRedbox(true)).toBe(true)
 
       const source = await session.getRedboxSource()
-      expect(source).toMatchSnapshot()
+      expect(next.normalizeTestDirContent(source)).toMatchSnapshot()
 
       await cleanup()
     })

diff --git a/test/development/acceptance-app/__snapshots__/ReactRefreshLogBox.test.ts.snap b/test/development/acceptance-app/__snapshots__/ReactRefreshLogBox.test.ts.snap
@@ -104,7 +104,7 @@ exports[`ReactRefreshLogBox app default unterminated JSX 1`] = `
 "./index.js
 Error: 
   x Unexpected token. Did you mean \`{'}'}\` or \`&rbrace;\`?
-   ,-[5:1]
+   ,-[TEST_DIR/index.js:5:1]
  5 |               <p>lol</p>
  6 |             div
  7 |           )
@@ -114,7 +114,7 @@ Error:
    \`----
 
   x Unexpected eof
-   ,-[6:1]
+   ,-[TEST_DIR/index.js:6:1]
  6 |             div
  7 |           )
  8 |         }

diff --git a/test/development/acceptance-app/rsc-build-errors.test.ts b/test/development/acceptance-app/rsc-build-errors.test.ts
@@ -272,13 +272,14 @@ createNextDescribe(
 
       expect(await session.hasRedbox(true)).toBe(true)
       await check(() => session.getRedboxSource(), /must be a Client Component/)
-      expect(await session.getRedboxSource()).toMatchInlineSnapshot(`
+      expect(next.normalizeTestDirContent(await session.getRedboxSource()))
+        .toMatchInlineSnapshot(`
         "./app/server-with-errors/error-file/error.js
         ReactServerComponentsError:
 
         ./app/server-with-errors/error-file/error.js must be a Client Component. Add the \\"use client\\" directive the top of the file to resolve this issue.
 
-           ,----
+           ,-[TEST_DIR/app/server-with-errors/error-file/error.js:1:1]
          1 | export default function Error() {}
            : ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
            \`----
@@ -302,13 +303,14 @@ createNextDescribe(
 
       expect(await session.hasRedbox(true)).toBe(true)
       await check(() => session.getRedboxSource(), /must be a Client Component/)
-      expect(await session.getRedboxSource()).toMatchInlineSnapshot(`
+      expect(next.normalizeTestDirContent(await session.getRedboxSource()))
+        .toMatchInlineSnapshot(`
         "./app/server-with-errors/error-file/error.js
         ReactServerComponentsError:
 
         ./app/server-with-errors/error-file/error.js must be a Client Component. Add the \\"use client\\" directive the top of the file to resolve this issue.
 
-           ,----
+           ,-[TEST_DIR/app/server-with-errors/error-file/error.js:1:1]
          1 |  
            : ^
            \`----
@@ -386,13 +388,14 @@ createNextDescribe(
       )
 
       expect(await session.hasRedbox(true)).toBe(true)
-      expect(await session.getRedboxSource()).toMatchInlineSnapshot(`
+      expect(next.normalizeTestDirContent(await session.getRedboxSource()))
+        .toMatchInlineSnapshot(`
         "./app/Component.js
         ReactServerComponentsError:
 
         You're importing a component that needs useState. It only works in a Client Component but none of its parents are marked with \\"use client\\", so they're Server Components by default.
 
-           ,----
+           ,-[TEST_DIR/node_modules/client-package/module2.js:1:1]
          1 | import { useState } from 'react'
            :          ^^^^^^^^
            \`----

diff --git a/test/development/acceptance/ReactRefreshLogBox-app-doc.test.ts b/test/development/acceptance/ReactRefreshLogBox-app-doc.test.ts
@@ -109,7 +109,9 @@ for (const variant of ['default', 'turbo']) {
         ])
       )
       expect(await session.hasRedbox(true)).toBe(true)
-      expect(await session.getRedboxSource()).toMatchSnapshot()
+      expect(
+        next.normalizeTestDirContent(await session.getRedboxSource())
+      ).toMatchSnapshot()
 
       await session.patch(
         'pages/_app.js',
@@ -158,7 +160,9 @@ for (const variant of ['default', 'turbo']) {
         ])
       )
       expect(await session.hasRedbox(true)).toBe(true)
-      expect(await session.getRedboxSource()).toMatchSnapshot()
+      expect(
+        next.normalizeTestDirContent(await session.getRedboxSource())
+      ).toMatchSnapshot()
 
       await session.patch(
         'pages/_document.js',

diff --git a/test/development/acceptance/ReactRefreshLogBox.test.ts b/test/development/acceptance/ReactRefreshLogBox.test.ts
@@ -215,7 +215,7 @@ for (const variant of ['default', 'turbo']) {
       expect(await session.hasRedbox(true)).toBe(true)
 
       const source = await session.getRedboxSource()
-      expect(source).toMatchSnapshot()
+      expect(next.normalizeTestDirContent(source)).toMatchSnapshot()
 
       await cleanup()
     })

diff --git a/test/development/acceptance/__snapshots__/ReactRefreshLogBox-app-doc.test.ts.snap b/test/development/acceptance/__snapshots__/ReactRefreshLogBox-app-doc.test.ts.snap
@@ -4,7 +4,7 @@ exports[`ReactRefreshLogBox default _app syntax error shows logbox 1`] = `
 "./pages/_app.js
 Error: 
   x Expression expected
-   ,-[1:1]
+   ,-[TEST_DIR/pages/_app.js:1:1]
  1 | 
  2 |             function MyApp({ Component, pageProps }) {
  3 |               return <<Component {...pageProps} />;
@@ -15,7 +15,7 @@ Error:
    \`----
 
   x Expression expected
-   ,-[1:1]
+   ,-[TEST_DIR/pages/_app.js:1:1]
  1 | 
  2 |             function MyApp({ Component, pageProps }) {
  3 |               return <<Component {...pageProps} />;
@@ -33,7 +33,7 @@ exports[`ReactRefreshLogBox default _document syntax error shows logbox 1`] = `
 "./pages/_document.js
 Error: 
   x Unexpected token \`{\`. Expected identifier, string literal, numeric literal or [ for the computed key
-   ,-[1:1]
+   ,-[TEST_DIR/pages/_document.js:1:1]
  1 | 
  2 |             import Document, { Html, Head, Main, NextScript } from 'next/document'
  3 | 

diff --git a/test/development/acceptance/__snapshots__/ReactRefreshLogBox.test.ts.snap b/test/development/acceptance/__snapshots__/ReactRefreshLogBox.test.ts.snap
@@ -75,7 +75,7 @@ exports[`ReactRefreshLogBox default unterminated JSX 1`] = `
 "./index.js
 Error: 
   x Unexpected token. Did you mean \`{'}'}\` or \`&rbrace;\`?
-   ,-[5:1]
+   ,-[TEST_DIR/index.js:5:1]
  5 |               <p>lol</p>
  6 |             div
  7 |           )
@@ -85,7 +85,7 @@ Error:
    \`----
 
   x Unexpected eof
-   ,-[6:1]
+   ,-[TEST_DIR/index.js:6:1]
  6 |             div
  7 |           )
  8 |         }

diff --git a/test/development/acceptance/__snapshots__/error-recovery.test.ts.snap b/test/development/acceptance/__snapshots__/error-recovery.test.ts.snap
@@ -16,7 +16,7 @@ exports[`ReactRefreshLogBox default syntax > runtime error 2`] = `
 "./index.js
 Error: 
   x Expected '}', got '<eof>'
-   ,-[5:1]
+   ,-[TEST_DIR/index.js:5:1]
  5 |           i++
  6 |           throw Error('no ' + i)
  7 |         }, 1000)
@@ -35,7 +35,7 @@ exports[`ReactRefreshLogBox default syntax > runtime error 3`] = `
 "./index.js
 Error: 
   x Expected '}', got '<eof>'
-   ,-[5:1]
+   ,-[TEST_DIR/index.js:5:1]
  5 |           i++
  6 |           throw Error('no ' + i)
  7 |         }, 1000)

diff --git a/test/development/acceptance/error-recovery.test.ts b/test/development/acceptance/error-recovery.test.ts
@@ -426,12 +426,16 @@ for (const variant of ['default', 'turbo']) {
 
       await new Promise((resolve) => setTimeout(resolve, 1000))
       expect(await session.hasRedbox(true)).toBe(true)
-      expect(await session.getRedboxSource()).toMatchSnapshot()
+      expect(
+        next.normalizeTestDirContent(await session.getRedboxSource())
+      ).toMatchSnapshot()
 
       // Test that runtime error does not take over:
       await new Promise((resolve) => setTimeout(resolve, 2000))
       expect(await session.hasRedbox(true)).toBe(true)
-      expect(await session.getRedboxSource()).toMatchSnapshot()
+      expect(
+        next.normalizeTestDirContent(await session.getRedboxSource())
+      ).toMatchSnapshot()
 
       await cleanup()
     })

diff --git a/test/development/acceptance/server-component-compiler-errors-in-pages.test.ts b/test/development/acceptance/server-component-compiler-errors-in-pages.test.ts
@@ -48,13 +48,14 @@ createNextDescribe(
         () => session.getRedboxSource(),
         /That only works in a Server Component/
       )
-      expect(await session.getRedboxSource()).toMatchInlineSnapshot(`
+      expect(next.normalizeTestDirContent(await session.getRedboxSource()))
+        .toMatchInlineSnapshot(`
         "./components/Comp.js
         ReactServerComponentsError:
 
         You're importing a component that needs next/headers. That only works in a Server Component which is not supported in the pages/ directory. Read more: https://beta.nextjs.org/docs/rendering/server-and-client-components
 
-           ,-[1:1]
+           ,-[TEST_DIR/components/Comp.js:1:1]
          1 | 
          2 |         import { cookies } from 'next/headers'
            :         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -90,13 +91,14 @@ createNextDescribe(
         () => session.getRedboxSource(),
         /That only works in a Server Component/
       )
-      expect(await session.getRedboxSource()).toMatchInlineSnapshot(`
+      expect(next.normalizeTestDirContent(await session.getRedboxSource()))
+        .toMatchInlineSnapshot(`
         "./components/Comp.js
         ReactServerComponentsError:
 
         You're importing a component that needs server-only. That only works in a Server Component which is not supported in the pages/ directory. Read more: https://beta.nextjs.org/docs/rendering/server-and-client-components
 
-           ,-[1:1]
+           ,-[TEST_DIR/components/Comp.js:1:1]
          1 | 
          2 |           import 'server-only' 
            :           ^^^^^^^^^^^^^^^^^^^^

diff --git a/test/lib/next-modes/base.ts b/test/lib/next-modes/base.ts
@@ -11,6 +11,7 @@ import webdriver from '../next-webdriver'
 import { renderViaHTTP, fetchViaHTTP } from 'next-test-utils'
 import cheerio from 'cheerio'
 import { BrowserInterface } from '../browsers/base'
+import escapeStringRegexp from 'escape-string-regexp'
 
 type Event = 'stdout' | 'stderr' | 'error' | 'destroy'
 export type InstallCommand =
@@ -280,6 +281,15 @@ export class NextInstance {
       })
   }
 
+  // normalize snapshots or stack traces being tested
+  // to a consistent test dir value since it's random
+  public normalizeTestDirContent(content) {
+    return content.replace(
+      new RegExp(escapeStringRegexp(this.testDir), 'g'),
+      'TEST_DIR'
+    )
+  }
+
   public async clean() {
     if (this.childProcess) {
       throw new Error(`stop() must be called before cleaning`)