chore: update how we track coverage during unit tests (#5779)

* chore: update how we track coverage during unit tests

The old method of tracking coverage was causing issues. If a test failed
on CI, that test's failure would be lost because the test failing would
in turn cause the coverage to fail, but the `process.exit(1)` in the
coverage code caused Mocha to not output anything useful.

Instead the coverage checker now:

* tracks the coverage in memory in a Map (this hasn't changed)
* after all tests, writes that to disk in test/coverage.json (which is
gitignored)
* we then run a single Mocha test that asserts every method was called.

This means if the test run fails, the build will fail and give the error
about that test run, and that output won't be lost when the coverage
then fails too.

Co-authored-by: Mathias Bynens <mathias@qiwi.be>

.gitignore

@@ -15,3 +15,4 @@ yarn.lock
 /utils/browser/puppeteer-web.js
 /index.d.ts
 /lib
+test/coverage.json

.travis.yml

@@ -29,13 +29,14 @@ jobs:
 
     # Runs unit tests on Linux + Chromium
     - node_js: "10.19.0"
-      name: 'Unit tests: Linux/Chromium'
+      name: 'Unit tests [with coverage]: Linux/Chromium'
       env:
         - CHROMIUM=true
       before_install:
         - PUPPETEER_PRODUCT=firefox npm install
       script:
-        - travis_retry npm run coverage
+        - travis_retry npm run unit-with-coverage
+        - npm run assert-unit-coverage
 
     # This bot runs all the extra checks that aren't the main Puppeteer unit tests
     - node_js: "10.19.0"

mocha-config/coverage-tests.js

@@ -0,0 +1,6 @@
+const base = require('./base');
+
+module.exports = {
+  ...base,
+  spec: 'test/assert-coverage-test.js',
+};

package.json

@@ -13,7 +13,8 @@
   },
   "scripts": {
     "unit": "mocha --config mocha-config/puppeteer-unit-tests.js",
-    "coverage": "cross-env COVERAGE=1 npm run unit",
+    "unit-with-coverage": "cross-env COVERAGE=1 npm run unit",
+    "assert-unit-coverage": "cross-env COVERAGE=1 mocha --config mocha-config/coverage-tests.js",
     "funit": "PUPPETEER_PRODUCT=firefox npm run unit",
     "debug-unit": "node --inspect-brk test/test.js",
     "test-doclint": "mocha --config mocha-config/doclint-tests.js",

test/assert-coverage-test.js

@@ -0,0 +1,24 @@
+const {describe, it} = require('mocha');
+const {getCoverageResults} = require('./coverage-utils');
+const expect = require('expect');
+
+describe('API coverage test', () => {
+  it('calls every method', () => {
+    if (!process.env.COVERAGE) return;
+
+    const coverageMap = getCoverageResults();
+    const missingMethods = [];
+    for (const method of coverageMap.keys()) {
+      if (!coverageMap.get(method))
+        missingMethods.push(method);
+    }
+    if (missingMethods.length) {
+      console.error('\nCoverage check failed: not all API methods called. See above output for list of missing methods.');
+      console.error(missingMethods.join('\n'));
+    }
+
+    // We know this will fail because we checked above
+    // but we need the actual test to fail.
+    expect(missingMethods.length).toEqual(0);
+  });
+});

test/coverage-utils.js

@@ -26,6 +26,9 @@
  * We run this when COVERAGE=1.
  */
 
+const path = require('path');
+const fs = require('fs');
+
 /**
  * @param {Map<string, boolean>} apiCoverage
  * @param {Object} events
@@ -59,9 +62,37 @@ function traceAPICoverage(apiCoverage, events, className, classType) {
   }
 }
 
-module.exports = function() {
+const coverageLocation = path.join(__dirname, 'coverage.json');
+
+const clearOldCoverage = () => {
+  try {
+    fs.unlinkSync(coverageLocation);
+  } catch (error) {
+    // do nothing, the file didn't exist
+  }
+};
+const writeCoverage = coverage => {
+  fs.writeFileSync(coverageLocation, JSON.stringify([...coverage.entries()]));
+};
+
+const getCoverageResults = () => {
+  let contents;
+  try {
+    contents = fs.readFileSync(coverageLocation, {encoding: 'utf8'});
+  } catch (error) {
+    console.error('Warning: coverage file does not exist or is not readable.');
+  }
+
+  const coverageMap = new Map(JSON.parse(contents));
+  return coverageMap;
+};
+
+const trackCoverage = () => {
+  clearOldCoverage();
   const coverageMap = new Map();
+
   before(() => {
+
     const api = require('../lib/api');
     const events = require('../lib/Events');
     for (const [className, classType] of Object.entries(api))
@@ -69,16 +100,11 @@ module.exports = function() {
   });
 
   after(() => {
-    const missingMethods = [];
-    for (const method of coverageMap.keys()) {
-      if (!coverageMap.get(method))
-        missingMethods.push(method);
-    }
-    if (missingMethods.length) {
-      console.error('\nCoverage check failed: not all API methods called. See above ouptut for list of missing methods.');
-      console.error(Array.from(missingMethods).join('\n'));
-      process.exit(1);
-    }
-    console.log('\nAll Puppeteer API methods were called. Coverage test passed.\n');
+    writeCoverage(coverageMap);
   });
 };
+
+module.exports = {
+  trackCoverage,
+  getCoverageResults
+};

test/mocha-utils.js

@@ -20,7 +20,7 @@ const fs = require('fs');
 const os = require('os');
 const puppeteer = require('../');
 const utils = require('./utils');
-const assertCoverage = require('./coverage-utils');
+const {trackCoverage} = require('./coverage-utils');
 
 const setupServer = async() => {
   const assetsPath = path.join(__dirname, 'assets');
@@ -115,7 +115,7 @@ global.describeChromeOnly = (...args) => {
 };
 
 if (process.env.COVERAGE)
-  assertCoverage();
+  trackCoverage();
 
 console.log(
     `Running unit tests with:

travis/chromium.sh

@@ -1,6 +0,0 @@
-#!/usr/bin/env sh
-
-npm run lint &&
-npm run coverage &&
-npm run test-doclint &&
-npm run test-types