test: use mocha in ci/cd (#3406)

.github/workflows/tests.yml

@@ -38,16 +38,11 @@ jobs:
     # XVFB-RUN merges both STDOUT and STDERR, whereas we need only STDERR
     # Wrap `npm run` in a subshell to redirect STDERR to file.
     # Enable core dumps in the subshell.
-    - run: xvfb-run --auto-servernum -- bash -c "ulimit -c unlimited && npm run jest -- --testTimeout=30000 && npm run coverage"
+    - run: xvfb-run --auto-servernum -- bash -c "ulimit -c unlimited && node test/mocha/index.js --max-workers=1 --timeout=30000 && npm run coverage"
       env:
         BROWSER: ${{ matrix.browser }}
-        DEBUG: "pw:*,-pw:wrapped*"
+        DEBUG: "pw:*,-pw:wrapped*,-pw:test*"
         DEBUG_FILE: "testrun.log"
-    - uses: actions/upload-artifact@v1
-      if: ${{ always() }}
-      with:
-        name: ${{ matrix.browser }}-${{ matrix.os }}-jest-report
-        path: jest-report.json
     - uses: actions/upload-artifact@v1
       if: failure()
       with:
@@ -74,16 +69,11 @@ jobs:
     - uses: microsoft/playwright-github-action@v1
     - run: npm ci
     - run: npm run build
-    - run: npm run jest -- --testTimeout=30000
+    - run: node test/mocha/index.js --max-workers=1 --timeout=30000
       env:
         BROWSER: ${{ matrix.browser }}
-        DEBUG: "pw:*,-pw:wrapped*"
+        DEBUG: "pw:*,-pw:wrapped*,-pw:test*"
         DEBUG_FILE: "testrun.log"
-    - uses: actions/upload-artifact@v1
-      if: ${{ always() }}
-      with:
-        name: ${{ matrix.browser }}-mac-jest-report
-        path: jest-report.json
     - uses: actions/upload-artifact@v1
       if: failure()
       with:
@@ -113,17 +103,12 @@ jobs:
     - uses: microsoft/playwright-github-action@v1
     - run: npm ci
     - run: npm run build
-    - run: npm run jest -- --testTimeout=30000
+    - run: node test/mocha/index.js --max-workers=1 --timeout=30000
       shell: bash
       env:
         BROWSER: ${{ matrix.browser }}
-        DEBUG: "pw:*,-pw:wrapped*"
+        DEBUG: "pw:*,-pw:wrapped*,-pw:test*"
         DEBUG_FILE: "testrun.log"
-    - uses: actions/upload-artifact@v1
-      if: ${{ always() }}
-      with:
-        name: ${{ matrix.browser }}-win-jest-report
-        path: jest-report.json
     - uses: actions/upload-artifact@v1
       if: failure()
       with:
@@ -175,17 +160,12 @@ jobs:
     # XVFB-RUN merges both STDOUT and STDERR, whereas we need only STDERR
     # Wrap `npm run` in a subshell to redirect STDERR to file.
     # Enable core dumps in the subshell.
-    - run: xvfb-run --auto-servernum -- bash -c "ulimit -c unlimited && npm run jest -- --testTimeout=30000"
+    - run: xvfb-run --auto-servernum -- bash -c "ulimit -c unlimited && node test/mocha/index.js --max-workers=1 --timeout=30000"
       if: ${{ always() }}
       env:
         BROWSER: ${{ matrix.browser }}
         HEADLESS: "false"
         DEBUG_FILE: "testrun.log"
-    - uses: actions/upload-artifact@v1
-      if: ${{ always() }}
-      with:
-        name: headful-${{ matrix.browser }}-linux-jest-report
-        path: jest-report.json
     - uses: actions/upload-artifact@v1
       if: ${{ always() }}
       with:
@@ -214,17 +194,12 @@ jobs:
     # XVFB-RUN merges both STDOUT and STDERR, whereas we need only STDERR
     # Wrap `npm run` in a subshell to redirect STDERR to file.
     # Enable core dumps in the subshell.
-    - run: xvfb-run --auto-servernum -- bash -c "ulimit -c unlimited && npm run jest -- --testTimeout=30000"
+    - run: xvfb-run --auto-servernum -- bash -c "ulimit -c unlimited && node test/mocha/index.js --max-workers=1 --timeout=30000"
       env:
         BROWSER: ${{ matrix.browser }}
-        DEBUG: "pw:*,-pw:wrapped*"
+        DEBUG: "pw:*,-pw:wrapped*,-pw:test*"
         DEBUG_FILE: "testrun.log"
         PWCHANNEL: ${{ matrix.transport }}
-    - uses: actions/upload-artifact@v1
-      if: ${{ always() }}
-      with:
-        name: rpc-${{ matrix.transport }}-${{ matrix.browser }}-linux-jest-report
-        path: jest-report.json
     - uses: actions/upload-artifact@v1
       if: failure()
       with:

package.json

@@ -9,13 +9,9 @@
     "node": ">=10.15.0"
   },
   "scripts": {
-    "ctest": "cross-env BROWSER=chromium jest",
-    "ftest": "cross-env BROWSER=firefox jest",
-    "wtest": "cross-env BROWSER=webkit jest",
-    "ctestd": "cross-env BROWSER=chromium jest --reporters=./jest/dot.js --colors",
-    "ftestd": "cross-env BROWSER=firefox jest --reporters=./jest/dot.js --colors",
-    "wtestd": "cross-env BROWSER=webkit jest --reporters=./jest/dot.js --colors",
-    "nojest": "cross-env BROWSER=chromium node --unhandled-rejections=strict ./test/nojest/nojest.js",
+    "ctest": "cross-env BROWSER=chromium node test/mocha/index.js",
+    "ftest": "cross-env BROWSER=firefox node test/mocha/index.js",
+    "wtest": "cross-env BROWSER=webkit node test/mocha/index.js",
     "test": "npm run ctest && npm run ftest && npm run wtest",
     "eslint": "[ \"$CI\" = true ] && eslint --quiet -f codeframe --ext js,ts ./src || eslint --ext js,ts ./src",
     "tsc": "tsc -p .",
@@ -34,7 +30,6 @@
     "generate-channels": "node utils/generate_channels.js",
     "typecheck-tests": "tsc -p ./test/",
     "roll-browser": "node utils/roll_browser.js",
-    "jest": "jest",
     "coverage": "node test/jest/checkCoverage.js",
     "check-deps": "node utils/check_deps.js"
   },
@@ -77,10 +72,8 @@
     "eslint": "^6.6.0",
     "eslint-plugin-notice": "^0.9.10",
     "esprima": "^4.0.0",
+    "expect": "^26.3.0",
     "formidable": "^1.2.1",
-    "jest": "^26.2.2",
-    "jest-circus": "^26.2.2",
-    "jest-image-snapshot": "^4.0.2",
     "mocha": "^8.1.1",
     "ncp": "^2.0.0",
     "node-stream-zip": "^1.8.2",

test/base.fixture.ts

@@ -90,7 +90,7 @@ registerWorkerFixture('defaultBrowserOptions', async({}, test) => {
   });
 });
 
-registerWorkerFixture('playwright', async({}, test) => {
+registerWorkerFixture('playwright', async({parallelIndex}, test) => {
   const {coverage, uninstall} = installCoverageHooks(browserName);
   if (process.env.PWCHANNEL === 'wire') {
     const connection = new Connection();
@@ -126,8 +126,7 @@ registerWorkerFixture('playwright', async({}, test) => {
 
   async function teardownCoverage() {
     uninstall();
-    const relativeTestPath = path.relative(__dirname, testPath);
-    const coveragePath = path.join(path.join(__dirname, 'output-' + browserName), 'coverage', relativeTestPath + '.json');
+    const coveragePath = path.join(path.join(__dirname, 'output-' + browserName), 'coverage', parallelIndex + '.json');
     const coverageJSON = [...coverage.keys()].filter(key => coverage.get(key));
     await fs.promises.mkdir(path.dirname(coveragePath), { recursive: true });
     await fs.promises.writeFile(coveragePath, JSON.stringify(coverageJSON, undefined, 2), 'utf8');

test/mocha/fixturesUI.js

@@ -35,6 +35,34 @@ function fixturesUI(trialRun, suite) {
   suite.on(Suite.constants.EVENT_FILE_PRE_REQUIRE, function(context, file, mocha) {
     const common = commonSuite(suites, context, mocha);
 
+    const itBuilder = (markers) => {
+      return function(title, fn) {
+        const suite = suites[0];
+        if (suite.isPending())
+          fn = null;
+        let wrapper;
+        if (trialRun) {
+          if (fn)
+            wrapper = () => {};
+        } else {
+          const wrapped = fixturePool.wrapTestCallback(fn);
+          wrapper = wrapped ? (done, ...args) => {
+            wrapped(...args).then(done).catch(done);
+          } : undefined;
+        }
+        if (wrapper) {
+          wrapper.toString = () => fn.toString();
+          wrapper.__original = fn;
+        }
+        const test = new Test(title, wrapper);
+        if (markers && markers.includes('slow'))
+          test.timeout(90000);
+        test.file = file;
+        suite.addTest(test);
+        return test;
+      };
+    };
+
     context.beforeEach = common.beforeEach;
     context.afterEach = common.afterEach;
     if (trialRun) {
@@ -56,7 +84,6 @@ function fixturesUI(trialRun, suite) {
         fn: fn
       });
     };
-
     context.xdescribe = (title, fn) => {
       return common.suite.skip({
         title: title,
@@ -64,11 +91,9 @@ function fixturesUI(trialRun, suite) {
         fn: fn
       });
     };
-
     context.describe.skip = function(condition) {
       return condition ? context.xdescribe : context.describe;
     };
-
     context.describe.only = (title, fn) => {
       return common.suite.only({
         title: title,
@@ -79,52 +104,21 @@ function fixturesUI(trialRun, suite) {
 
     context.fdescribe = context.describe.only;
 
-    context.it = context.specify = function(title, fn) {
-      const suite = suites[0];
-      if (suite.isPending())
-        fn = null;
-      let wrapper = fn;
-      if (trialRun) {
-        if (wrapper)
-          wrapper = () => {};
-      } else {
-        const wrapped = fixturePool.wrapTestCallback(wrapper);
-        wrapper = wrapped ? (done, ...args) => {
-          wrapped(...args).then(done).catch(done);
-        } : undefined;
-      }
-      if (wrapper) {
-        wrapper.toString = () => fn.toString();
-        wrapper.__original = fn;
-      }
-      const test = new Test(title, wrapper);
-      test.file = file;
-      suite.addTest(test);
-      return test;
-    };
-
+    context.it = itBuilder();
     context.it.only = function(title, fn) {
       return common.test.only(mocha, context.it(title, fn));
     };
-
     context.fit = context.it.only;
-
     context.xit = function(title) {
       return context.it(title);
     };
-
     context.it.skip = function(condition) {
       return condition ? context.xit : context.it;
     };
-
     context.it.fail = function(condition) {
       return condition ? context.xit : context.it;
     };
-
-    context.it.slow = function(condition) {
-      return context.it;
-    };
-
+    context.it.slow = () => itBuilder(['slow']);
     context.it.retries = function(n) {
       context.retries(n);
     };

test/mocha/index.js

@@ -31,7 +31,8 @@ program
   .option('--retries <retries>', 'number of times to retry a failing test', 1)
   .action(async (command) => {
     // Collect files
-    const files = collectFiles(command.args);
+    const files = [];
+    collectFiles(path.join(process.cwd(), 'test'), command.args, files);
     const rootSuite = new Mocha.Suite('', new Mocha.Context(), true);
 
     // Build the test model, suite per file.
@@ -49,10 +50,6 @@ program
       await new Promise(f => mocha.run(f));
     }
 
-    if (rootSuite.hasOnly())
-      rootSuite.filterOnly();
-
-    console.log(`Running ${rootSuite.total()} tests`);
     const runner = new Runner(rootSuite, {
       maxWorkers: command.maxWorkers,
       reporter: command.reporter,
@@ -65,22 +62,23 @@ program
 
 program.parse(process.argv);
 
-function collectFiles(args) {
-  const testDir = path.join(process.cwd(), 'test');
-  const files = [];
-  for (const name of fs.readdirSync(testDir)) {
-    if (!name.includes('.spec.'))
-      continue;
-    if (!args.length) {
-      files.push(path.join(testDir, name));
+function collectFiles(dir, filters, files) {
+  for (const name of fs.readdirSync(dir)) {
+    if (fs.lstatSync(path.join(dir, name)).isDirectory()) {
+      collectFiles(path.join(dir, name), filters, files);
       continue;
     }
-    for (const filter of args) {
+    if (!name.includes('spec'))
+      continue;
+    if (!filters.length) {
+      files.push(path.join(dir, name));
+      continue;
+    }
+    for (const filter of filters) {
       if (name.includes(filter)) {
-        files.push(path.join(testDir, name));
+        files.push(path.join(dir, name));
         break;
       }
     }
   }
-  return files;
 }

test/mocha/runner.js

@@ -22,6 +22,8 @@ const builtinReporters = require('mocha/lib/reporters');
 const DotRunner = require('./dotReporter');
 
 const constants = Mocha.Runner.constants;
+// Mocha runner does not remove uncaughtException listeners.
+process.setMaxListeners(0);
 
 class Runner extends EventEmitter {
   constructor(suite, options) {
@@ -31,8 +33,9 @@ class Runner extends EventEmitter {
     this._maxWorkers = options.maxWorkers;
     this._workers = new Set();
     this._freeWorkers = [];
-    this._callbacks = [];
+    this._workerClaimers = [];
     this._workerId = 0;
+    this._pendingJobs = 0;
     this.stats = {
       duration: 0,
       failures: 0,
@@ -45,6 +48,10 @@ class Runner extends EventEmitter {
 
     this._tests = new Map();
     this._files = new Map();
+
+    if (suite.hasOnly())
+      suite.filterOnly();
+    console.log(`Running ${suite.total()} tests`);
     this._traverse(suite);
   }
 
@@ -62,60 +69,90 @@ class Runner extends EventEmitter {
 
   async run() {
     this.emit(constants.EVENT_RUN_BEGIN, {});
-    const result = new Promise(f => this._runCallback = f);
     for (const file of this._files.keys()) {
       const worker = await this._obtainWorker();
-      worker.send({ method: 'run', params: { file, options: this._options } });
+      this._runJob(worker, file);
     }
-    await result;
+    await new Promise(f => this._runCompleteCallback = f);
     this.emit(constants.EVENT_RUN_END, {});
   }
 
-  async _obtainWorker() {
-    if (this._freeWorkers.length)
-      return this._freeWorkers.pop();
+  _runJob(worker, file) {
+    ++this._pendingJobs;
+    worker.send({ method: 'run', params: { file, options: this._options } });
+    const messageListener = (message) => {
+      const { method, params } = message;
+      if (method !== 'done') {
+        this._messageFromWorker(method, params);
+        return;
+      }
+      worker.off('message', messageListener);
 
-    if (this._workers.size < this._maxWorkers) {
-      const worker = child_process.fork(path.join(__dirname, 'worker.js'), {
-        detached: false
-      });
-      let readyCallback;
-      const result = new Promise(f => readyCallback = f);
-      worker.send({ method: 'init', params: { workerId: ++this._workerId } });
-      worker.on('message', message => {
-        if (message.method === 'ready')
-          readyCallback();
-        this._messageFromWorker(worker, message);
-      });
-      worker.on('exit', () => {
-        this._workers.delete(worker);
-        if (!this._workers.size)
-          this._stopCallback();
-      });
-      this._workers.add(worker);
-      await result;
-      return worker;
-    }
-
-    return new Promise(f => this._callbacks.push(f));
+      --this._pendingJobs;
+      this.stats.duration += params.stats.duration;
+      this.stats.failures += params.stats.failures;
+      this.stats.passes += params.stats.passes;
+      this.stats.pending += params.stats.pending;
+      this.stats.tests += params.stats.tests;
+      if (params.error)
+        this._restartWorker(worker);
+      else
+        this._workerAvailable(worker);
+      if (this._runCompleteCallback && !this._pendingJobs)
+        this._runCompleteCallback();
+    };
+    worker.on('message', messageListener)
   }
 
-  _messageFromWorker(worker, message) {
-    const { method, params } = message;
+  async _obtainWorker() {
+    // If there is worker, use it.
+    if (this._freeWorkers.length)
+      return this._freeWorkers.pop();
+    // If we can create worker, create it.
+    if (this._workers.size < this._maxWorkers)
+      this._createWorker();
+    // Wait for the next available worker.
+    await new Promise(f => this._workerClaimers.push(f));
+    return this._freeWorkers.pop();
+  }
+
+  async _workerAvailable(worker) {
+    this._freeWorkers.push(worker);
+    if (this._workerClaimers.length) {
+      const callback = this._workerClaimers.shift();
+      callback();
+    }
+  }
+
+  _createWorker() {
+    const worker = child_process.fork(path.join(__dirname, 'worker.js'), {
+      detached: false,
+      env: process.env,
+    });
+    worker.on('exit', () => {
+      this._workers.delete(worker);
+      if (this._stopCallback && !this._workers.size)
+        this._stopCallback();
+    });
+    this._workers.add(worker);
+    worker.send({ method: 'init', params: { workerId: ++this._workerId } });
+    worker.once('message', () => {
+      // Ready ack.
+      this._workerAvailable(worker);
+    });
+  }
+
+  _stopWorker(worker) {
+    worker.send({ method: 'stop' });
+  }
+
+  async _restartWorker(worker) {
+    this._stopWorker(worker);
+    this._createWorker();
+  }
+
+  _messageFromWorker(method, params) {
     switch (method) {
-      case 'done': {
-        if (this._callbacks.length) {
-          const callback = this._callbacks.shift();
-          callback(worker);
-        } else {
-          this._freeWorkers.push(worker);
-          if (this._freeWorkers.length === this._workers.size)
-            this._runCallback();
-        }
-        break;
-      }
-      case 'start':
-        break;
       case 'test':
         this.emit(constants.EVENT_TEST_BEGIN, this._updateTest(params.test));
         break;
@@ -126,30 +163,21 @@ class Runner extends EventEmitter {
         this.emit(constants.EVENT_TEST_PASS, this._updateTest(params.test));
         break;
       case 'fail':
-        const test = this._updateTest(params.test);
-        this.emit(constants.EVENT_TEST_FAIL, test, params.error);
-        break;
-      case 'end':
-        this.stats.duration += params.stats.duration;
-        this.stats.failures += params.stats.failures;
-        this.stats.passes += params.stats.passes;
-        this.stats.pending += params.stats.pending;
-        this.stats.tests += params.stats.tests;
+        this.emit(constants.EVENT_TEST_FAIL, this._updateTest(params.test), params.error);
         break;
     }
   }
 
   _updateTest(serialized) {
     const test = this._tests.get(serialized.id);
-    test._currentRetry = serialized.currentRetry;
-    this.duration = serialized.duration;
+    test.duration = serialized.duration;
     return test;
   }
 
   async stop() {
     const result = new Promise(f => this._stopCallback = f);
     for (const worker of this._workers)
-      worker.send({ method: 'stop' });
+      this._stopWorker(worker);
     await result;
   }
 }

test/mocha/worker.js

@@ -26,6 +26,8 @@ const outputPath = path.join(__dirname, '..', 'output-' + browserName);
 global.expect = require('expect');
 global.testOptions = require('../harness/testOptions');
 
+const constants = Mocha.Runner.constants;
+
 extendExpects();
 
 let closed = false;
@@ -33,9 +35,10 @@ let closed = false;
 process.on('message', async message => {
   if (message.method === 'init')
     process.env.JEST_WORKER_ID = message.params.workerId;
-  if (message.method === 'stop')
+  if (message.method === 'stop') {
+    await fixturePool.teardownScope('worker');
     await gracefullyCloseAndExit();
-  if (message.method === 'run')
+  } if (message.method === 'run')
     await runSingleTest(message.params.file, message.params.options);
 });
 
@@ -55,50 +58,45 @@ async function gracefullyCloseAndExit() {
 
 class NullReporter {}
 
+let failedWithError = false;
+
 async function runSingleTest(file, options) {
-  let nextOrdinal = 0;
+  let lastOrdinal = -1;
   const mocha = new Mocha({
     ui: fixturesUI.bind(null, false),
-    retries: options.retries === 1 ? undefined : options.retries,
     timeout: options.timeout,
     reporter: NullReporter
   });
   mocha.addFile(file);
+  mocha.suite.filterOnly();
 
-  const runner = mocha.run();
-
-  const constants = Mocha.Runner.constants;
-  runner.on(constants.EVENT_RUN_BEGIN, () => {
-    sendMessageToParent('start');
+  const runner = mocha.run(() => {
+    // Runner adds these; if we don't remove them, we'll get a leak.
+    process.removeAllListeners('uncaughtException');
   });
 
   runner.on(constants.EVENT_TEST_BEGIN, test => {
-    // Retries will produce new test instances, store ordinal on the original function.
-    let ordinal = nextOrdinal++;
-    if (typeof test.fn.__original.__ordinal !== 'number')
-      test.fn.__original.__ordinal = ordinal;
-    sendMessageToParent('test', { test: serializeTest(test, ordinal) });
+    sendMessageToParent('test', { test: serializeTest(test, ++lastOrdinal) });
   });
 
   runner.on(constants.EVENT_TEST_PENDING, test => {
-    // Pending does not get test begin signal, so increment ordinal.
-    sendMessageToParent('pending', { test: serializeTest(test, nextOrdinal++) });
+    sendMessageToParent('pending', { test: serializeTest(test, ++lastOrdinal) });
   });
 
   runner.on(constants.EVENT_TEST_PASS, test => {
-    sendMessageToParent('pass', { test: serializeTest(test, test.fn.__original.__ordinal) });
+    sendMessageToParent('pass', { test: serializeTest(test, lastOrdinal) });
   });
 
   runner.on(constants.EVENT_TEST_FAIL, (test, error) => {
+    failedWithError = error;
     sendMessageToParent('fail', {
-      test: serializeTest(test, test.fn.__original.__ordinal),
+      test: serializeTest(test, lastOrdinal),
       error: serializeError(error),
     });
   });
 
   runner.once(constants.EVENT_RUN_END, async () => {
-    sendMessageToParent('end', { stats: serializeStats(runner.stats) });
-    sendMessageToParent('done');
+    sendMessageToParent('done', { stats: serializeStats(runner.stats), error: failedWithError });
   });
 }
 
@@ -115,9 +113,7 @@ function sendMessageToParent(method, params = {}) {
 function serializeTest(test, origin) {
   return {
     id: `${test.file}::${origin}`,
-    currentRetry: test.currentRetry(),
     duration: test.duration,
-    title: test.title,
   };
 }