Continue testing react v17 with e2e tests (#35787)

* Continue testing react v17 with e2e tests * update react 17 expected error * Update env variable name and add log * fix starter cache version * remove extra setup condition * Revert "remove extra setup condition" This reverts commit 5760a9f. * update basepath test * remove log * stabilize test
vercel · Mar 31, 2022 · e148d2b · e148d2b
1 parent 6a1e8db
commit e148d2b
Show file tree

Hide file tree

Showing 8 changed files with 226 additions and 23 deletions.
diff --git a/.github/workflows/build_test_deploy.yml b/.github/workflows/build_test_deploy.yml
@@ -255,6 +255,57 @@ jobs:
           path: |
             test/traces
 
+  testDevReact17:
+    name: Test Development (react v17)
+    runs-on: ubuntu-latest
+    needs: [build, build-native-test]
+    env:
+      NEXT_TELEMETRY_DISABLED: 1
+      NEXT_TEST_JOB: 1
+      NEXT_TEST_REACT_VERSION: ^17
+    steps:
+      - name: Setup node
+        uses: actions/setup-node@v2
+        if: ${{needs.build.outputs.docsChange != 'docs only change'}}
+        with:
+          node-version: 16
+
+      - run: echo ${{needs.build.outputs.docsChange}}
+
+      # https://github.com/actions/virtual-environments/issues/1187
+      - name: tune linux network
+        run: sudo ethtool -K eth0 tx off rx off
+
+      - uses: actions/cache@v2
+        if: ${{needs.build.outputs.docsChange != 'docs only change'}}
+        id: restore-build
+        with:
+          path: ./*
+          key: ${{ github.sha }}-${{ github.run_number }}-${{ github.run_attempt }}
+
+      - uses: actions/download-artifact@v2
+        if: ${{needs.build.outputs.docsChange != 'docs only change'}}
+        with:
+          name: next-swc-test-binary
+          path: packages/next-swc/native
+
+      - run: npm i -g playwright-chromium@1.14.1 && npx playwright install-deps
+        if: ${{needs.build.outputs.docsChange != 'docs only change'}}
+
+      - run: node run-tests.js --type development
+        name: Run test/development
+        if: ${{needs.build.outputs.docsChange != 'docs only change'}}
+
+      - name: Upload test trace
+        if: always()
+        uses: actions/upload-artifact@v2
+        with:
+          name: test-trace
+          if-no-files-found: ignore
+          retention-days: 2
+          path: |
+            test/traces
+
   testDevE2E:
     name: Test Development (E2E)
     runs-on: ubuntu-latest
@@ -309,6 +360,57 @@ jobs:
           path: |
             test/traces
 
+  testDevE2EReact17:
+    name: Test Development (E2E) (react v17)
+    runs-on: ubuntu-latest
+    needs: [build, build-native-test]
+    env:
+      NEXT_TELEMETRY_DISABLED: 1
+      NEXT_TEST_JOB: 1
+      NEXT_TEST_REACT_VERSION: ^17
+    steps:
+      - name: Setup node
+        uses: actions/setup-node@v2
+        if: ${{needs.build.outputs.docsChange != 'docs only change'}}
+        with:
+          node-version: 16
+
+      - run: echo ${{needs.build.outputs.docsChange}}
+
+      # https://github.com/actions/virtual-environments/issues/1187
+      - name: tune linux network
+        run: sudo ethtool -K eth0 tx off rx off
+
+      - uses: actions/cache@v2
+        if: ${{needs.build.outputs.docsChange != 'docs only change'}}
+        id: restore-build
+        with:
+          path: ./*
+          key: ${{ github.sha }}-${{ github.run_number }}-${{ github.run_attempt }}
+
+      - uses: actions/download-artifact@v2
+        if: ${{needs.build.outputs.docsChange != 'docs only change'}}
+        with:
+          name: next-swc-test-binary
+          path: packages/next-swc/native
+
+      - run: npm i -g playwright-chromium@1.14.1 && npx playwright install-deps
+        if: ${{needs.build.outputs.docsChange != 'docs only change'}}
+
+      - run: NEXT_TEST_MODE=dev node run-tests.js --type e2e
+        name: Run test/e2e (dev)
+        if: ${{needs.build.outputs.docsChange != 'docs only change'}}
+
+      - name: Upload test trace
+        if: always()
+        uses: actions/upload-artifact@v2
+        with:
+          name: test-trace
+          if-no-files-found: ignore
+          retention-days: 2
+          path: |
+            test/traces
+
   testProd:
     name: Test Production
     runs-on: ubuntu-latest
@@ -353,6 +455,47 @@ jobs:
         name: Run test/production
         if: ${{needs.build.outputs.docsChange != 'docs only change'}}
 
+  testProdReact17:
+    name: Test Production (react v17)
+    runs-on: ubuntu-latest
+    needs: [build, build-native-test]
+    env:
+      NEXT_TELEMETRY_DISABLED: 1
+      NEXT_TEST_JOB: 1
+      NEXT_TEST_REACT_VERSION: ^17
+    steps:
+      - name: Setup node
+        uses: actions/setup-node@v2
+        if: ${{needs.build.outputs.docsChange != 'docs only change'}}
+        with:
+          node-version: 16
+
+      - run: echo ${{needs.build.outputs.docsChange}}
+
+      # https://github.com/actions/virtual-environments/issues/1187
+      - name: tune linux network
+        run: sudo ethtool -K eth0 tx off rx off
+
+      - uses: actions/cache@v2
+        if: ${{needs.build.outputs.docsChange != 'docs only change'}}
+        id: restore-build
+        with:
+          path: ./*
+          key: ${{ github.sha }}-${{ github.run_number }}-${{ github.run_attempt }}
+
+      - uses: actions/download-artifact@v2
+        if: ${{needs.build.outputs.docsChange != 'docs only change'}}
+        with:
+          name: next-swc-test-binary
+          path: packages/next-swc/native
+
+      - run: npm i -g playwright-chromium@1.14.1 && npx playwright install-deps
+        if: ${{needs.build.outputs.docsChange != 'docs only change'}}
+
+      - run: node run-tests.js --type production
+        name: Run test/production
+        if: ${{needs.build.outputs.docsChange != 'docs only change'}}
+
   testProdE2E:
     name: Test Production (E2E)
     runs-on: ubuntu-latest
@@ -397,6 +540,47 @@ jobs:
         name: Run test/e2e (production)
         if: ${{needs.build.outputs.docsChange != 'docs only change'}}
 
+  testProdE2EReact17:
+    name: Test Production (E2E) (react v17)
+    runs-on: ubuntu-latest
+    needs: [build, build-native-test]
+    env:
+      NEXT_TELEMETRY_DISABLED: 1
+      NEXT_TEST_JOB: 1
+      NEXT_TEST_REACT_VERSION: ^17
+    steps:
+      - name: Setup node
+        uses: actions/setup-node@v2
+        if: ${{needs.build.outputs.docsChange != 'docs only change'}}
+        with:
+          node-version: ${{ matrix.node }}
+
+      - run: echo ${{needs.build.outputs.docsChange}}
+
+      # https://github.com/actions/virtual-environments/issues/1187
+      - name: tune linux network
+        run: sudo ethtool -K eth0 tx off rx off
+
+      - uses: actions/cache@v2
+        if: ${{needs.build.outputs.docsChange != 'docs only change'}}
+        id: restore-build
+        with:
+          path: ./*
+          key: ${{ github.sha }}-${{ github.run_number }}-${{ github.run_attempt }}
+
+      - uses: actions/download-artifact@v2
+        if: ${{needs.build.outputs.docsChange != 'docs only change'}}
+        with:
+          name: next-swc-test-binary
+          path: packages/next-swc/native
+
+      - run: npm i -g playwright-chromium@1.14.1 && npx playwright install-deps
+        if: ${{needs.build.outputs.docsChange != 'docs only change'}}
+
+      - run: NEXT_TEST_MODE=start node run-tests.js --type e2e
+        name: Run test/e2e (production)
+        if: ${{needs.build.outputs.docsChange != 'docs only change'}}
+
   testIntegration:
     name: Test Integration
     runs-on: ubuntu-latest

diff --git a/packages/next/client/index.tsx b/packages/next/client/index.tsx
@@ -1024,6 +1024,14 @@ function Root({
     () => callbacks.forEach((callback) => callback()),
     [callbacks]
   )
+  // We should ask to measure the Web Vitals after rendering completes so we
+  // don't cause any hydration delay:
+  React.useEffect(() => {
+    measureWebVitals(onPerfEntry)
+
+    flushBufferedVitalsMetrics()
+  }, [])
+
   if (process.env.__NEXT_TEST_MODE) {
     // eslint-disable-next-line react-hooks/rules-of-hooks
     React.useEffect(() => {
@@ -1034,13 +1042,6 @@ function Root({
       }
     }, [])
   }
-  // We should ask to measure the Web Vitals after rendering completes so we
-  // don't cause any hydration delay:
-  React.useEffect(() => {
-    measureWebVitals(onPerfEntry)
-
-    flushBufferedVitalsMetrics()
-  }, [])
 
   return children as React.ReactElement
 }

diff --git a/run-tests.js b/run-tests.js
@@ -215,9 +215,10 @@ async function main() {
     // a starter Next.js install to re-use to speed up tests
     // to avoid having to run yarn each time
     console.log('Creating Next.js install for isolated tests')
+    const reactVersion = process.env.NEXT_TEST_REACT_VERSION || 'latest'
     const testStarter = await createNextInstall({
-      react: '17.0.2',
-      'react-dom': '17.0.2',
+      react: reactVersion,
+      'react-dom': reactVersion,
     })
     process.env.NEXT_TEST_STARTER = testStarter
   }

diff --git a/test/development/basic-basepath/hmr.test.ts b/test/development/basic-basepath/hmr.test.ts
@@ -532,6 +532,8 @@ describe('basic HMR', () => {
           )
         )
 
+        const isReact17 = process.env.NEXT_TEST_REACT_VERSION === '^17'
+
         expect(await hasRedbox(browser)).toBe(true)
         // TODO: Replace this when webpack 5 is the default
         expect(
@@ -540,7 +542,9 @@ describe('basic HMR', () => {
             'Unknown'
           )
         ).toMatch(
-          'Objects are not valid as a React child (found: /search/). If you meant to render a collection of children, use an array instead.'
+          `Objects are not valid as a React child (found: ${
+            isReact17 ? '/search/' : '[object RegExp]'
+          }). If you meant to render a collection of children, use an array instead.`
         )
 
         await next.patchFile(aboutPage, aboutContent)

diff --git a/test/development/basic/hmr.test.ts b/test/development/basic/hmr.test.ts
@@ -22,10 +22,6 @@ describe('basic HMR', () => {
         pages: new FileRef(join(__dirname, 'hmr/pages')),
         components: new FileRef(join(__dirname, 'hmr/components')),
       },
-      dependencies: {
-        react: 'latest',
-        'react-dom': 'latest',
-      },
     })
   })
   afterAll(() => next.destroy())
@@ -589,6 +585,8 @@ describe('basic HMR', () => {
           )
         )
 
+        const isReact17 = process.env.NEXT_TEST_REACT_VERSION === '^17'
+
         expect(await hasRedbox(browser)).toBe(true)
         // TODO: Replace this when webpack 5 is the default
         expect(
@@ -597,7 +595,9 @@ describe('basic HMR', () => {
             'Unknown'
           )
         ).toMatch(
-          'Objects are not valid as a React child (found: [object RegExp]). If you meant to render a collection of children, use an array instead.'
+          `Objects are not valid as a React child (found: ${
+            isReact17 ? '/search/' : '[object RegExp]'
+          }). If you meant to render a collection of children, use an array instead.`
         )
 
         await next.patchFile(aboutPage, aboutContent)

diff --git a/test/development/basic/styled-components.test.ts b/test/development/basic/styled-components.test.ts
@@ -17,8 +17,6 @@ describe('styled-components SWC transform', () => {
       },
       dependencies: {
         'styled-components': '5.3.3',
-        react: 'latest',
-        'react-dom': 'latest',
       },
     })
   })

diff --git a/test/e2e/basepath.test.ts b/test/e2e/basepath.test.ts
@@ -739,6 +739,10 @@ describe('basePath', () => {
     it('should use urls with basepath in router events for hash changes', async () => {
       const browser = await webdriver(next.url, `${basePath}/hello`)
       try {
+        await check(
+          () => browser.eval('window.next.router.isReady ? "ready" : "no"'),
+          'ready'
+        )
         await browser.eval('window._clearEventLog()')
         await browser.elementByCss('#hash-change').click()
 
@@ -763,7 +767,12 @@ describe('basePath', () => {
     it('should use urls with basepath in router events for cancelled routes', async () => {
       const browser = await webdriver(next.url, `${basePath}/hello`)
       try {
+        await check(
+          () => browser.eval('window.next.router.isReady ? "ready" : "no"'),
+          'ready'
+        )
         await browser.eval('window._clearEventLog()')
+
         await browser
           .elementByCss('#slow-route')
           .click()
@@ -793,6 +802,10 @@ describe('basePath', () => {
     it('should use urls with basepath in router events for failed route change', async () => {
       const browser = await webdriver(next.url, `${basePath}/hello`)
       try {
+        await check(
+          () => browser.eval('window.next.router.isReady ? "ready" : "no"'),
+          'ready'
+        )
         await browser.eval('window._clearEventLog()')
         await browser.elementByCss('#error-route').click()
 

diff --git a/test/lib/next-modes/base.ts b/test/lib/next-modes/base.ts
@@ -88,13 +88,15 @@ export class NextInstance {
     ) {
       await fs.copy(process.env.NEXT_TEST_STARTER, this.testDir)
     } else if (!skipIsolatedNext) {
+      const reactVersion = process.env.NEXT_TEST_REACT_VERSION || 'latest'
+      const finalDependencies = {
+        react: reactVersion,
+        'react-dom': reactVersion,
+        ...this.dependencies,
+        ...((this.packageJson.dependencies as object | undefined) || {}),
+      }
       this.testDir = await createNextInstall(
-        {
-          react: 'latest',
-          'react-dom': 'latest',
-          ...this.dependencies,
-          ...((this.packageJson.dependencies as object | undefined) || {}),
-        },
+        finalDependencies,
         this.installCommand,
         this.packageJson
       )