Improve test setup (#5388)

* Update jest

* Let jest start chromedriver

This makes sure chromedriver always ends even if the test was canceled by the user.

* Properly close browser in production-config test

* Properly close browser in production/security test

* Properly close browser in export test

* Properly close browser in app-aspath test

* Remove taskr from project root

This isn’t needed anymore

* Readd taskr to project root (temporary)

* Improve global setup/teardown

* Properly close browser in basic/client-navigation test

Clicking an target=_blank link will open a second browser window. We can only close this by using broser.quit()

package.json

@@ -7,11 +7,9 @@
   "scripts": {
     "lerna": "lerna",
     "bootstrap": "lerna bootstrap",
-    "pretestonly": "taskr pretest",
     "dev": "lerna run build --stream --parallel",
-    "testonly": "cross-env NODE_PATH=test/lib jest \\.test.js",
-    "posttestonly": "taskr posttest",
-    "testall": "npm run testonly -- --coverage --forceExit --runInBand --verbose --bail",
+    "testonly": "cross-env NODE_PATH=test/lib jest \\.test.js --config=./test/jest-config.json",
+    "testall": "npm run testonly -- --coverage --forceExit --runInBand",
     "pretest": "npm run lint",
     "test": "cross-env npm run testall || npm run testall",
     "coveralls": "nyc --instrument=false --source-map=false report --temp-directory=./coverage --reporter=text-lcov | coveralls",
@@ -32,24 +30,15 @@
       "**/examples/with-mobx/**"
     ]
   },
-  "jest": {
-    "testEnvironment": "node",
-    "roots": [
-      "test/"
-    ]
-  },
   "devDependencies": {
     "@babel/plugin-proposal-object-rest-spread": "7.0.0",
     "@babel/preset-react": "7.0.0",
-    "@taskr/clear": "1.1.0",
-    "@taskr/esnext": "1.1.0",
-    "@taskr/watch": "1.1.0",
     "@zeit/next-css": "0.2.1-canary.1",
     "@zeit/next-sass": "0.2.1-canary.1",
     "@zeit/next-typescript": "1.1.0",
     "babel-core": "7.0.0-bridge.0",
     "babel-eslint": "9.0.0",
-    "babel-jest": "23.4.2",
+    "babel-jest": "23.6.0",
     "babel-plugin-transform-define": "1.3.0",
     "benchmark": "2.1.4",
     "cheerio": "0.22.0",
@@ -63,7 +52,7 @@
     "flow-bin": "0.73.0",
     "get-port": "3.2.0",
     "husky": "0.14.3",
-    "jest-cli": "21.2.0",
+    "jest-cli": "23.6.0",
     "lerna": "^3.4.0",
     "lint-staged": "4.2.3",
     "micro": "9.1.0",

packages/next-server/package.json

@@ -54,6 +54,7 @@
     "@babel/runtime": "7.1.2",
     "@babel/runtime-corejs2": "7.1.2",
     "@taskr/clear": "1.1.0",
-    "@taskr/watch": "1.1.0"
+    "@taskr/watch": "1.1.0",
+    "taskr": "1.1.0"
   }
 }

packages/next/package.json

@@ -109,6 +109,8 @@
   "devDependencies": {
     "@babel/preset-flow": "7.0.0",
     "@taskr/clear": "1.1.0",
-    "@taskr/watch": "1.1.0"
+    "@taskr/esnext": "1.1.0",
+    "@taskr/watch": "1.1.0",
+    "taskr": "1.1.0"
   }
 }

taskfile.js

@@ -1,35 +0,0 @@
-const isWindows = /^win/.test(process.platform)
-const childProcess = require('child_process')
-const rimraf = require('rimraf')
-const mkdirp = require('mkdirp')
-
-export async function pretest (task) {
-  // Create node_modules/next for the use of test apps
-  rimraf.sync('test/node_modules/next')
-  mkdirp.sync('test/node_modules')
-
-  if (isWindows) {
-    const symlinkCommand = 'mklink /D "next" "..\\..\\packages\\next"'
-    childProcess.execSync(symlinkCommand, { cwd: 'test/node_modules' })
-  }
-
-  // We run following task inside a NPM script chain and it runs chromedriver
-  // inside a child process tree.
-  // Even though we kill this task's process, chromedriver exists throughout
-  // the lifetime of the original npm script.
-  // Start chromedriver
-  const processName = isWindows ? 'chromedriver.cmd' : 'chromedriver'
-  childProcess.spawn(processName, { stdio: 'inherit' })
-
-  // We need to do this, otherwise this task's process will keep waiting.
-  setTimeout(() => process.exit(0), 2000)
-}
-
-export async function posttest (task) {
-  try {
-    const cmd = isWindows ? 'taskkill /im chromedriver* /t /f' : 'pkill chromedriver'
-    childProcess.execSync(cmd, { stdio: 'ignore' })
-  } catch (err) {
-    // Do nothing
-  }
-}

test/integration/app-aspath/test/index.test.js

@@ -32,24 +32,22 @@ describe('App asPath', () => {
     const appPath = join(__dirname, '../', 'pages', '_app.js')
     const originalContent = readFileSync(appPath, 'utf8')
 
-    try {
-      const text = await browser.elementByCss('body').text()
-      expect(text).toBe('{ "url": { "query": {}, "pathname": "/", "asPath": "/" } }')
+    const text = await browser.elementByCss('body').text()
+    expect(text).toBe('{ "url": { "query": {}, "pathname": "/", "asPath": "/" } }')
 
-      const editedContent = originalContent.replace('find this', 'replace with this')
+    const editedContent = originalContent.replace('find this', 'replace with this')
 
-      // Change the content to trigger a bundle rebuild
-      await writeFileSync(appPath, editedContent, 'utf8')
+    // Change the content to trigger a bundle rebuild
+    await writeFileSync(appPath, editedContent, 'utf8')
 
-      // Wait for the bundle rebuild
-      await waitFor(5000)
+    // Wait for the bundle rebuild
+    await waitFor(5000)
 
-      const newContent = await browser.elementByCss('body').text()
-      expect(newContent).toBe('{ "url": { "query": {}, "pathname": "/", "asPath": "/" } }')
-    } finally {
-      // Change back to the original content
-      writeFileSync(appPath, originalContent, 'utf8')
-      browser.close()
-    }
+    const newContent = await browser.elementByCss('body').text()
+    expect(newContent).toBe('{ "url": { "query": {}, "pathname": "/", "asPath": "/" } }')
+
+    // Change back to the original content
+    writeFileSync(appPath, originalContent, 'utf8')
+    browser.quit()
   })
 })

test/integration/basic/test/client-navigation.js

@@ -56,7 +56,7 @@ export default (context, render) => {
           .elementByCss('p').text()
 
         expect(text).toBe('This is the home.')
-        browser.close()
+        browser.quit()
       })
 
       it('should not navigate if the <a/> tag has a target', async () => {

test/integration/export/test/dev.js

@@ -12,15 +12,13 @@ export default function (context) {
   describe('Render in development mode', () => {
     it('should render the home page', async () => {
       const browser = await webdriver(context.port, '/')
-
       await check(() => getBrowserBodyText(browser), /This is the home page/)
+      browser.close()
     })
 
     it('should render pages only existent in exportPathMap page', async () => {
       const browser = await webdriver(context.port, '/dynamic/one')
-      const text = await browser
-        .elementByCss('#dynamic-page p').text()
-
+      const text = await browser.elementByCss('#dynamic-page p').text()
       expect(text).toBe('next export is nice')
       browser.close()
     })

test/integration/production-config/test/index.test.js

@@ -1,4 +1,5 @@
-/* global jasmine, describe, it, expect, beforeAll, afterAll */
+/* eslint-env jest */
+/* global jasmine */
 
 import { join } from 'path'
 import {
@@ -9,51 +10,39 @@ import {
 } from 'next-test-utils'
 import webdriver from 'next-webdriver'
 
-const appDir = join(__dirname, '../')
-let server
-let app
 jasmine.DEFAULT_TIMEOUT_INTERVAL = 1000 * 60 * 5
 
-const context = {}
+let server
 
 describe('Production Config Usage', () => {
   beforeAll(async () => {
+    const appDir = join(__dirname, '../')
     await nextBuild(appDir)
-    app = nextServer({
+    const app = nextServer({
       dir: join(__dirname, '../'),
       dev: false,
       quiet: true
     })
-
     server = await startApp(app)
-    context.appPort = server.address().port
   })
   afterAll(() => stopApp(server))
 
-  const openBrowser = (args) => webdriver(context.appPort, ...args)
-
   describe('with next-css', () => {
     it('should load styles', async () => {
-      let browser
-
-      async function testBrowser () {
-        browser = await openBrowser('/')
-        const element = await browser.elementByCss('#mounted')
-        const text = await element.text()
-        expect(text).toMatch(/ComponentDidMount executed on client\./)
-        expect(await element.getComputedCss('font-size')).toBe('40px')
-        expect(await element.getComputedCss('color')).toBe('rgba(255, 0, 0, 1)')
-      }
-      try {
-        // Try 3 times as the breaking happens intermittently
-        await testBrowser()
-        await testBrowser()
-        await testBrowser()
-      } finally {
-        if (browser) {
-          browser.close()
-        }
-      }
+      // Try 3 times as the breaking happens intermittently
+      await testBrowser()
+      await testBrowser()
+      await testBrowser()
     })
   })
 })
+
+async function testBrowser () {
+  const browser = await webdriver(server.address().port, '/')
+  const element = await browser.elementByCss('#mounted')
+  const text = await element.text()
+  expect(text).toMatch(/ComponentDidMount executed on client\./)
+  expect(await element.getComputedCss('font-size')).toBe('40px')
+  expect(await element.getComputedCss('color')).toBe('rgba(255, 0, 0, 1)')
+  return browser.close()
+}

test/integration/production/test/security.js

@@ -39,7 +39,7 @@ module.exports = (context) => {
         throw new Error('Vulnerable to XSS attacks')
       }
 
-      browser.close()
+      browser.quit()
     })
   })
 }

test/jest-config.json

@@ -0,0 +1,7 @@
+{
+  "verbose": true,
+  "bail": true,
+  "testEnvironment": "node",
+  "globalSetup": "./jest-global-setup.js",
+  "globalTeardown": "./jest-global-teardown.js"
+}

test/jest-global-setup.js

@@ -0,0 +1,5 @@
+const chromedriver = require('chromedriver')
+
+module.exports = async function globalSetup () {
+  chromedriver.start()
+}

test/jest-global-teardown.js

@@ -0,0 +1,5 @@
+const chromedriver = require('chromedriver')
+
+module.exports = async function globalSetup () {
+  chromedriver.stop()
+}