tests: add type checking to cli/tests (#6874)

lighthouse-cli/test/cli/cli-flags-test.js

@@ -14,11 +14,14 @@ describe('CLI bin', function() {
     getFlags('chrome://version');
     const yargs = require('yargs');
 
+    // @ts-ignore - getGroups is private
     const optionGroups = yargs.getGroups();
+    /** @type {string[]} */
     const allOptions = [];
     Object.keys(optionGroups).forEach(key => {
       allOptions.push(...optionGroups[key]);
     });
+    // @ts-ignore - getUsageInstance is private
     const optionsWithDescriptions = Object.keys(yargs.getUsageInstance().getDescriptions());
 
     allOptions.forEach(opt => {

lighthouse-cli/test/cli/printer-test.js

@@ -18,7 +18,7 @@ describe('Printer', () => {
   });
 
   it('rejects invalid output paths', () => {
-    const path = undefined;
+    const path = /** @type {any} */ (undefined);
     assert.notEqual(Printer.checkOutputPath(path), path);
   });
 

lighthouse-cli/test/cli/run-test.js

@@ -28,14 +28,21 @@ describe('CLI run', function() {
     const timeoutFlag = `--max-wait-for-load=${9000}`;
     const flags = getFlags(`--output=json --output-path=${filename} ${timeoutFlag} ${url}`);
     return run.runLighthouse(url, flags, fastConfig).then(passedResults => {
+      if (!passedResults) {
+        assert.fail('no results');
+        return;
+      }
+
       const {lhr} = passedResults;
       assert.ok(fs.existsSync(filename));
+      /** @type {LH.Result} */
       const results = JSON.parse(fs.readFileSync(filename, 'utf-8'));
       assert.equal(results.audits.viewport.rawValue, false);
 
       // passed results match saved results
       assert.strictEqual(results.fetchTime, lhr.fetchTime);
-      assert.strictEqual(results.url, lhr.url);
+      assert.strictEqual(results.requestedUrl, lhr.requestedUrl);
+      assert.strictEqual(results.finalUrl, lhr.finalUrl);
       assert.strictEqual(results.audits.viewport.rawValue, lhr.audits.viewport.rawValue);
       assert.strictEqual(
           Object.keys(results.audits).length,
@@ -57,7 +64,9 @@ describe('flag coercing', () => {
 
 describe('saveResults', () => {
   it('will quit early if we\'re in gather mode', async () => {
-    const result = await run.saveResults(undefined, {gatherMode: true});
+    const result = await run.saveResults(
+      /** @type {LH.RunnerResult} */ ({}),
+      /** @type {LH.CliFlags} */ ({gatherMode: true}));
     assert.equal(result, undefined);
   });
 });

lighthouse-cli/test/global-mocha-hooks.js

@@ -1,39 +0,0 @@
-'use strict';
-
-const assert = require('assert');
-
-/**
- * THIS FILE IS A DUPLICATE OF lighthouse-core/test/global-mocha-hooks.js
- * Please keep them in sync.
- */
-
-/* eslint-env jest */
-let currTest;
-
-// monkey-patch all assert.* methods
-Object.keys(assert)
-  .filter(key => typeof assert[key] === 'function')
-  .forEach(key => {
-    const _origFn = assert[key];
-    assert[key] = function(...args) {
-      if (currTest) {
-        currTest._assertions++;
-      }
-      return _origFn.apply(this, args);
-    };
-  });
-
-// store the count of assertions on each test's state object
-beforeEach(function() {
-  // eslint-disable-next-line no-invalid-this
-  currTest = this.currentTest;
-  currTest._assertions = currTest._assertions || 0;
-});
-
-afterEach(function() {
-  if (currTest._assertions === 0) {
-    throw new Error(`ZERO assertions in test: "${currTest.title}"\n${currTest.file}`);
-  }
-  currTest = null;
-});
-

lighthouse-cli/test/smokehouse/run-smoke.js

@@ -12,6 +12,7 @@ const execAsync = promisify(require('child_process').exec);
 const {server, serverForOffline} = require('../fixtures/static-server');
 const log = require('lighthouse-logger');
 
+/** @param {string} str */
 const purpleify = str => `${log.purple}${str}${log.reset}`;
 const smokehouseDir = 'lighthouse-cli/test/smokehouse/';
 
@@ -88,18 +89,15 @@ const SMOKETESTS = [{
 
 /**
  * Display smokehouse output from child process
- * @param {{id: string, process: NodeJS.Process} || {id: string, error: Error & {stdout : NodeJS.WriteStream, stderr: NodeJS.WriteStream}}} result
+ * @param {{id: string, stdout: string, stderr: string, error?: Error}} result
  */
 function displaySmokehouseOutput(result) {
   console.log(`\n${purpleify(result.id)} smoketest results:`);
   if (result.error) {
     console.log(result.error.message);
-    process.stdout.write(result.error.stdout);
-    process.stderr.write(result.error.stderr);
-  } else {
-    process.stdout.write(result.process.stdout);
-    process.stderr.write(result.process.stderr);
   }
+  process.stdout.write(result.stdout);
+  process.stderr.write(result.stderr);
   console.timeEnd(`smoketest-${result.id}`);
   console.log(`${purpleify(result.id)} smoketest complete. \n`);
   return result;
@@ -124,8 +122,8 @@ async function runSmokehouse(smokes) {
 
     // The promise ensures we output immediately, even if the process errors
     const p = execAsync(cmd, {timeout: 6 * 60 * 1000, encoding: 'utf8'})
-      .then(cp => ({id: id, process: cp}))
-      .catch(err => ({id: id, error: err}))
+      .then(cp => ({id, ...cp}))
+      .catch(err => ({id, stdout: err.stdout, stderr: err.stderr, error: err}))
       .then(result => displaySmokehouseOutput(result));
 
     // If the machine is terribly slow, we'll run all smoketests in succession, not parallel
@@ -200,6 +198,7 @@ async function cli() {
   if (failingTests.length && (process.env.RETRY_SMOKES || process.env.CI)) {
     console.log('Retrying failed tests...');
     for (const failedResult of failingTests) {
+      /** @type {number} */
       const resultIndex = smokeResults.indexOf(failedResult);
       const smokeDefn = smokeDefns.get(failedResult.id);
       smokeResults[resultIndex] = (await runSmokehouse([smokeDefn]))[0];

lighthouse-cli/test/smokehouse/seo/expectations.js

@@ -7,6 +7,10 @@
 const BASE_URL = 'http://localhost:10200/seo/';
 const URLSearchParams = require('url').URLSearchParams;
 
+/**
+ * @param {[string, string][]} headers
+ * @return {string}
+ */
 function headersParam(headers) {
   const headerString = new URLSearchParams(headers).toString();
   return new URLSearchParams([['extra_header', headerString]]).toString();

lighthouse-cli/test/smokehouse/smokehouse.js

@@ -6,6 +6,22 @@
  */
 'use strict';
 
+/**
+ * @typedef {{path: string, actual: *, expected: *}} Difference
+ */
+
+/**
+ * @typedef {{category: string, actual: *, expected: *, equal: boolean, diff?: Difference | null}} Comparison
+ */
+
+/**
+ * @typedef {Pick<LH.Result, 'audits' | 'finalUrl' | 'requestedUrl'> & {errorCode?: string}} ExpectedLHR
+ */
+
+/**
+ * @typedef {{audits: Comparison[], errorCode: Comparison, finalUrl: Comparison}} LHRComparison
+ */
+
 /* eslint-disable no-console */
 
 const fs = require('fs');
@@ -19,7 +35,6 @@ const PAGE_HUNG_EXIT_CODE = 68;
 const RETRIES = 3;
 const NUMERICAL_EXPECTATION_REGEXP = /^(<=?|>=?)((\d|\.)+)$/;
 
-
 /**
  * Attempt to resolve a path locally. If this fails, attempts to locate the path
  * relative to the current working directory.
@@ -43,10 +58,10 @@ function resolveLocalOrCwd(payloadPath) {
  * @param {string} url
  * @param {string} configPath
  * @param {boolean=} isDebug
- * @return {!LighthouseResults}
+ * @return {ExpectedLHR}
  */
 function runLighthouse(url, configPath, isDebug) {
-  isDebug = isDebug || process.env.SMOKEHOUSE_DEBUG;
+  isDebug = isDebug || Boolean(process.env.SMOKEHOUSE_DEBUG);
 
   const command = 'node';
   const outputPath = `smokehouse-${Math.round(Math.random() * 100000)}.report.json`;
@@ -137,6 +152,8 @@ function matchesExpectation(actual, expected) {
         return actual < number;
       case '<=':
         return actual <= number;
+      default:
+        throw new Error(`unexpected operator ${operator}`);
     }
   } else if (typeof actual === 'string' && expected instanceof RegExp && expected.test(actual)) {
     return true;
@@ -157,7 +174,7 @@ function matchesExpectation(actual, expected) {
  * @param {string} path
  * @param {*} actual
  * @param {*} expected
- * @return {({path: string, actual: *, expected: *}|null)}
+ * @return {(Difference|null)}
  */
 function findDifference(path, actual, expected) {
   if (matchesExpectation(actual, expected)) {
@@ -199,9 +216,9 @@ function findDifference(path, actual, expected) {
 
 /**
  * Collate results into comparisons of actual and expected scores on each audit.
- * @param {{finalUrl: string, audits: !Array, errorCode: string}} actual
- * @param {{finalUrl: string, audits: !Array, errorCode: string}} expected
- * @return {{finalUrl: !Object, audits: !Array<!Object>}}
+ * @param {ExpectedLHR} actual
+ * @param {ExpectedLHR} expected
+ * @return {LHRComparison}
  */
 function collateResults(actual, expected) {
   const auditNames = Object.keys(expected.audits);
@@ -224,12 +241,6 @@ function collateResults(actual, expected) {
   });
 
   return {
-    finalUrl: {
-      category: 'final url',
-      actual: actual.finalUrl,
-      expected: expected.finalUrl,
-      equal: actual.finalUrl === expected.finalUrl,
-    },
     audits: collatedAudits,
     errorCode: {
       category: 'error code',
@@ -237,15 +248,23 @@ function collateResults(actual, expected) {
       expected: expected.errorCode,
       equal: actual.errorCode === expected.errorCode,
     },
+    finalUrl: {
+      category: 'final url',
+      actual: actual.finalUrl,
+      expected: expected.finalUrl,
+      equal: actual.finalUrl === expected.finalUrl,
+    },
   };
 }
 
 /**
  * Log the result of an assertion of actual and expected results.
- * @param {{category: string, equal: boolean, diff: ?Object, actual: boolean, expected: boolean}} assertion
+ * @param {Comparison} assertion
  */
 function reportAssertion(assertion) {
+  // @ts-ignore - this doesn't exist now but could one day, so try not to break the future
   const _toJSON = RegExp.prototype.toJSON;
+  // @ts-ignore
   // eslint-disable-next-line no-extend-native
   RegExp.prototype.toJSON = RegExp.prototype.toString;
 
@@ -273,6 +292,7 @@ function reportAssertion(assertion) {
     }
   }
 
+  // @ts-ignore
   // eslint-disable-next-line no-extend-native
   RegExp.prototype.toJSON = _toJSON;
 }
@@ -280,7 +300,7 @@ function reportAssertion(assertion) {
 /**
  * Log all the comparisons between actual and expected test results, then print
  * summary. Returns count of passed and failed tests.
- * @param {{finalUrl: !Object, audits: !Array<!Object>, errorCode: !Object}} results
+ * @param {LHRComparison} results
  * @return {{passed: number, failed: number}}
  */
 function report(results) {
@@ -316,11 +336,12 @@ const cli = yargs
     'expectations-path': 'The path to the expected audit results file',
     'debug': 'Save the artifacts along with the output',
   })
-  .require('config-path')
-  .require('expectations-path')
+  .require('config-path', true)
+  .require('expectations-path', true)
   .argv;
 
 const configPath = resolveLocalOrCwd(cli['config-path']);
+/** @type {ExpectedLHR[]} */
 const expectations = require(resolveLocalOrCwd(cli['expectations-path']));
 
 // Loop sequentially over expectations, comparing against Lighthouse run, and

lighthouse-core/test/global-mocha-hooks.js

@@ -1,40 +0,0 @@
-'use strict';
-
-const assert = global.assert = require('assert');
-
-/**
- * We don't want any mocha tests to run without evaluating any assertions
- * These hooks will throw if an it() test completes without any assert.foo()
- * having run.
- */
-
-/* eslint-env jest */
-let currTest;
-
-// monkey-patch all assert.* methods
-Object.keys(assert)
-  .filter(key => typeof assert[key] === 'function')
-  .forEach(key => {
-    const _origFn = assert[key];
-    assert[key] = function(...args) {
-      if (currTest) {
-        currTest._assertions++;
-      }
-      return _origFn.apply(this, args);
-    };
-  });
-
-// store the count of assertions on each test's state object
-beforeEach(function() {
-  // eslint-disable-next-line no-invalid-this
-  currTest = this.currentTest;
-  currTest._assertions = currTest._assertions || 0;
-});
-
-afterEach(function() {
-  if (currTest._assertions === 0) {
-    throw new Error(`ZERO assertions in test: "${currTest.title}"\n${currTest.file}`);
-  }
-  currTest = null;
-});
-

package.json

@@ -81,6 +81,7 @@
     "@types/google.analytics": "0.0.39",
     "@types/inquirer": "^0.0.35",
     "@types/intl-messageformat": "^1.3.0",
+    "@types/jest": "^23.3.10",
     "@types/jpeg-js": "^0.3.0",
     "@types/lodash.isequal": "^4.5.2",
     "@types/make-dir": "^1.0.3",

tsconfig.json

@@ -8,10 +8,6 @@
     "strict": true,
     // "listFiles": true,
     // "noErrorTruncation": true,
-    "typeRoots": [
-      "@types",
-      "./types"
-    ],
 
     "resolveJsonModule": true,
     "diagnostics": true
@@ -21,11 +17,11 @@
     "lighthouse-core/**/*.js",
     "clients/**/*.js",
     "build/**/*.js",
-    "./types/*.d.ts",
+    "./types/**/*.d.ts",
   ],
   "exclude": [
-    "lighthouse-cli/test/**/*.js",
     "lighthouse-core/test/**/*.js",
     "clients/test/**/*.js",
+    "lighthouse-cli/test/fixtures/**/*.js",
   ]
 }

types/lighthouse-logger/index.d.ts

@@ -19,8 +19,15 @@ declare module 'lighthouse-logger' {
   export function time(status: Status, level?: string): void;
   export function timeEnd(status: Status, level?: string): void;
   export function reset(): string;
+  export const cross: string;
+  export const dim: string;
+  export const tick: string;
+  export const bold: string;
+  export const purple: string;
+  export function redify(message: any): string;
+  export function greenify(message: any): string;
   /** Retrieves and clears all stored time entries */
   export function takeTimeEntries(): PerformanceEntry[];
   export function getTimeEntries(): PerformanceEntry[];
-  export var events: import('events').EventEmitter;
+  export const events: import('events').EventEmitter;
 }

yarn.lock

@@ -211,6 +211,11 @@
   resolved "https://registry.yarnpkg.com/@types/intl-messageformat/-/intl-messageformat-1.3.0.tgz#6af7144802b13d62ade9ad68f8420cdb33b75916"
   integrity sha1-avcUSAKxPWKt6a1o+EIM2zO3WRY=
 
+"@types/jest@^23.3.10":
+  version "23.3.10"
+  resolved "https://registry.yarnpkg.com/@types/jest/-/jest-23.3.10.tgz#4897974cc317bf99d4fe6af1efa15957fa9c94de"
+  integrity sha512-DC8xTuW/6TYgvEg3HEXS7cu9OijFqprVDXXiOcdOKZCU/5PJNLZU37VVvmZHdtMiGOa8wAA/We+JzbdxFzQTRQ==
+
 "@types/jpeg-js@^0.3.0":
   version "0.3.0"
   resolved "https://registry.yarnpkg.com/@types/jpeg-js/-/jpeg-js-0.3.0.tgz#5971f0aa50900194e9565711c265c10027385e89"