diff --git a/lighthouse-cli/test/smokehouse/frontends/back-compat-util.js b/lighthouse-cli/test/smokehouse/frontends/back-compat-util.js new file mode 100644 index 000000000000..2bf1bf5db862 --- /dev/null +++ b/lighthouse-cli/test/smokehouse/frontends/back-compat-util.js @@ -0,0 +1,42 @@ +/** + * @license Copyright 2021 The Lighthouse Authors. All Rights Reserved. + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. + */ +'use strict'; + +/** + * COMPAT: update from the old TestDefn format (array of `expectations` per + * definition) to the new format (single `expectations` per def), doing our best + * generating some unique IDs. + * TODO: remove in Lighthouse 9+ once PubAds (and others?) are updated. + * @see https://github.com/GoogleChrome/lighthouse/issues/11950 + * @param {ReadonlyArray} allTestDefns + * @return {Array} + */ +function updateTestDefnFormat(allTestDefns) { + const expandedTestDefns = allTestDefns.map(testDefn => { + if (Array.isArray(testDefn.expectations)) { + // Create a testDefn per expectation. + return testDefn.expectations.map((expectations, index) => { + return { + ...testDefn, + id: `${testDefn.id}-${index}`, + expectations, + }; + }); + } else { + // New object to make tsc happy. + return { + ...testDefn, + expectations: testDefn.expectations, + }; + } + }); + + return expandedTestDefns.flat(); +} + +module.exports = { + updateTestDefnFormat, +}; diff --git a/lighthouse-cli/test/smokehouse/frontends/lib.js b/lighthouse-cli/test/smokehouse/frontends/lib.js index fcee969648f2..1f77e4ca48cb 100644 --- a/lighthouse-cli/test/smokehouse/frontends/lib.js +++ b/lighthouse-cli/test/smokehouse/frontends/lib.js @@ -16,6 +16,7 @@ const cloneDeep = require('lodash.clonedeep'); const smokeTests = require('../test-definitions/core-tests.js'); const {runSmokehouse} = require('../smokehouse.js'); +const {updateTestDefnFormat} = require('./back-compat-util.js'); /** * @param {Smokehouse.SmokehouseLibOptions} options @@ -23,29 +24,23 @@ const {runSmokehouse} = require('../smokehouse.js'); async function smokehouse(options) { const {urlFilterRegex, skip, modify, ...smokehouseOptions} = options; - const clonedTests = cloneDeep(smokeTests); - const modifiedTests = clonedTests.map(test => { - const modifiedExpectations = []; - for (const expected of test.expectations) { - if (urlFilterRegex && !expected.lhr.requestedUrl.match(urlFilterRegex)) { - continue; - } - - const reasonToSkip = skip && skip(test, expected); - if (reasonToSkip) { - console.log(`skipping ${expected.lhr.requestedUrl}: ${reasonToSkip}`); - continue; - } - - modify && modify(test, expected); - modifiedExpectations.push(expected); + const updatedCoreTests = updateTestDefnFormat(smokeTests); + const clonedTests = cloneDeep(updatedCoreTests); + const modifiedTests = []; + for (const test of clonedTests) { + if (urlFilterRegex && !test.expectations.lhr.requestedUrl.match(urlFilterRegex)) { + continue; } - return { - ...test, - expectations: modifiedExpectations, - }; - }).filter(test => test.expectations.length > 0); + const reasonToSkip = skip && skip(test, test.expectations); + if (reasonToSkip) { + console.log(`skipping ${test.expectations.lhr.requestedUrl}: ${reasonToSkip}`); + continue; + } + + modify && modify(test, test.expectations); + modifiedTests.push(test); + } return runSmokehouse(modifiedTests, smokehouseOptions); } diff --git a/lighthouse-cli/test/smokehouse/frontends/smokehouse-bin.js b/lighthouse-cli/test/smokehouse/frontends/smokehouse-bin.js index 33deec918b91..4b5c9070b48d 100644 --- a/lighthouse-cli/test/smokehouse/frontends/smokehouse-bin.js +++ b/lighthouse-cli/test/smokehouse/frontends/smokehouse-bin.js @@ -18,6 +18,7 @@ const cloneDeep = require('lodash.clonedeep'); const yargs = require('yargs'); const log = require('lighthouse-logger'); const {runSmokehouse} = require('../smokehouse.js'); +const {updateTestDefnFormat} = require('./back-compat-util.js'); const coreTestDefnsPath = path.join(__dirname, '../test-definitions/core-tests.js'); @@ -46,12 +47,18 @@ function getDefinitionsToRun(allTestDefns, requestedIds, {invertMatch}) { console.log('Running ALL smoketests. Equivalent to:'); console.log(usage); } else { - smokes = allTestDefns.filter(test => invertMatch !== requestedIds.includes(test.id)); + smokes = allTestDefns.filter(test => { + // Include all tests that *include* requested id. + // e.g. a requested 'pwa' will match 'pwa-airhorner', 'pwa-caltrain', etc + let isRequested = requestedIds.some(requestedId => test.id.includes(requestedId)); + if (invertMatch) isRequested = !isRequested; + return isRequested; + }); console.log(`Running ONLY smoketests for: ${smokes.map(t => t.id).join(' ')}\n`); } const unmatchedIds = requestedIds.filter(requestedId => { - return !allTestDefns.map(t => t.id).includes(requestedId); + return !allTestDefns.map(t => t.id).some(id => id.includes(requestedId)); }); if (unmatchedIds.length) { console.log(log.redify(`Smoketests not found for: ${unmatchedIds.join(' ')}`)); @@ -80,23 +87,21 @@ function pruneExpectedNetworkRequests(testDefns, takeNetworkRequestUrls) { const clonedDefns = cloneDeep(testDefns); for (const {id, expectations, runSerially} of clonedDefns) { - for (const expectation of expectations) { - if (!runSerially && expectation.networkRequests) { - throw new Error(`'${id}' must be set to 'runSerially: true' to assert 'networkRequests'`); - } + if (!runSerially && expectations.networkRequests) { + throw new Error(`'${id}' must be set to 'runSerially: true' to assert 'networkRequests'`); + } - if (pruneNetworkRequests && expectation.networkRequests) { - // eslint-disable-next-line max-len - const msg = `'networkRequests' cannot be asserted in test '${id}'. They should only be asserted on tests from an in-process server`; - if (process.env.CI) { - // If we're in CI, we require any networkRequests expectations to be asserted. - throw new Error(msg); - } - - console.warn(log.redify('Warning:'), - `${msg}. Pruning expectation: ${JSON.stringify(expectation.networkRequests)}`); - expectation.networkRequests = undefined; + if (pruneNetworkRequests && expectations.networkRequests) { + // eslint-disable-next-line max-len + const msg = `'networkRequests' cannot be asserted in test '${id}'. They should only be asserted on tests from an in-process server`; + if (process.env.CI) { + // If we're in CI, we require any networkRequests expectations to be asserted. + throw new Error(msg); } + + console.warn(log.redify('Warning:'), + `${msg}. Pruning expectation: ${JSON.stringify(expectations.networkRequests)}`); + expectations.networkRequests = undefined; } } @@ -166,7 +171,8 @@ async function begin() { let testDefnPath = argv.testsPath || coreTestDefnsPath; testDefnPath = path.resolve(process.cwd(), testDefnPath); const requestedTestIds = argv._; - const allTestDefns = require(testDefnPath); + const rawTestDefns = require(testDefnPath); + const allTestDefns = updateTestDefnFormat(rawTestDefns); const invertMatch = argv.invertMatch; const testDefns = getDefinitionsToRun(allTestDefns, requestedTestIds, {invertMatch}); diff --git a/lighthouse-cli/test/smokehouse/lib/concurrent-mapper.js b/lighthouse-cli/test/smokehouse/lib/concurrent-mapper.js index ff5d48b25a7c..72b307b9ff19 100644 --- a/lighthouse-cli/test/smokehouse/lib/concurrent-mapper.js +++ b/lighthouse-cli/test/smokehouse/lib/concurrent-mapper.js @@ -106,6 +106,21 @@ class ConcurrentMapper { return Promise.all(result); } + + /** + * Runs `fn` concurrent to other operations in the pool, at a max of + * `concurrency` at a time across all callers on this instance. Default + * `concurrency` limit is `Infinity`. + * @template U + * @param {() => Promise} fn + * @param {{concurrency: number}} [options] + * @return {Promise} + */ + async runInPool(fn, options = {concurrency: Infinity}) { + // Let pooledMap handle the pool management for the cost of boxing a fake `value`. + const result = await this.pooledMap([''], fn, options); + return result[0]; + } } module.exports = ConcurrentMapper; diff --git a/lighthouse-cli/test/smokehouse/report-assert.js b/lighthouse-cli/test/smokehouse/report-assert.js index 9ce5c8560245..aead6bd6fda9 100644 --- a/lighthouse-cli/test/smokehouse/report-assert.js +++ b/lighthouse-cli/test/smokehouse/report-assert.js @@ -333,15 +333,6 @@ function reportAssertion(localConsole, assertion) { RegExp.prototype.toJSON = _toJSON; } -/** - * @param {number} count - * @return {string} - */ -function assertLogString(count) { - const plural = count === 1 ? '' : 's'; - return `${count} assertion${plural}`; -} - /** * Log all the comparisons between actual and expected test results, then print * summary. Returns count of passed and failed tests. @@ -371,17 +362,6 @@ function report(actual, expected, reportOptions = {}) { } }); - const correctStr = assertLogString(correctCount); - const colorFn = correctCount === 0 ? log.redify : log.greenify; - localConsole.log(` Correctly passed ${colorFn(correctStr)}`); - - if (failedCount) { - const failedString = assertLogString(failedCount); - const failedColorFn = failedCount === 0 ? log.greenify : log.redify; - localConsole.log(` Failed ${failedColorFn(failedString)}`); - } - localConsole.write('\n'); - return { passed: correctCount, failed: failedCount, diff --git a/lighthouse-cli/test/smokehouse/smokehouse.js b/lighthouse-cli/test/smokehouse/smokehouse.js index e2d09b93356d..990c69260892 100644 --- a/lighthouse-cli/test/smokehouse/smokehouse.js +++ b/lighthouse-cli/test/smokehouse/smokehouse.js @@ -28,8 +28,8 @@ const DEFAULT_RETRIES = 0; /** * @typedef SmokehouseResult * @property {string} id - * @property {boolean} success - * @property {Array<{passed: number, failed: number, log: string}>} expectationResults + * @property {number} passed + * @property {number} failed */ /** @@ -49,38 +49,32 @@ async function runSmokehouse(smokeTestDefns, smokehouseOptions) { assertPositiveInteger('jobs', jobs); assertNonNegativeInteger('retries', retries); - // Run each testDefn's tests in parallel based on the concurrencyLimit. + // Run each testDefn in parallel based on the concurrencyLimit. const concurrentMapper = new ConcurrentMapper(); - const smokePromises = []; - for (const testDefn of smokeTestDefns) { - // If defn is set to `runSerially`, we'll run its tests in succession, not parallel. + + const testOptions = {isDebug, retries, lighthouseRunner, takeNetworkRequestUrls}; + const smokePromises = smokeTestDefns.map(testDefn => { + // If defn is set to `runSerially`, we'll run it in succession with other tests, not parallel. const concurrency = testDefn.runSerially ? 1 : jobs; - const options = {concurrency, lighthouseRunner, retries, isDebug, takeNetworkRequestUrls}; - const result = runSmokeTestDefn(concurrentMapper, testDefn, options); - smokePromises.push(result); - } + return concurrentMapper.runInPool(() => runSmokeTest(testDefn, testOptions), {concurrency}); + }); const testResults = await Promise.all(smokePromises); + // Print final summary. let passingCount = 0; let failingCount = 0; for (const testResult of testResults) { - for (const expectationResult of testResult.expectationResults) { - passingCount += expectationResult.passed; - failingCount += expectationResult.failed; - } - } - if (passingCount) { - console.log(log.greenify(`${passingCount} expectations passing`)); - } - if (failingCount) { - console.log(log.redify(`${failingCount} expectations failing`)); + passingCount += testResult.passed; + failingCount += testResult.failed; } + if (passingCount) console.log(log.greenify(`${getAssertionLog(passingCount)} passing in total`)); + if (failingCount) console.log(log.redify(`${getAssertionLog(failingCount)} failing in total`)); - // Print and fail if there were failing tests. - const failingDefns = testResults.filter(result => !result.success); + // Print id(s) and fail if there were failing tests. + const failingDefns = testResults.filter(result => result.failed); if (failingDefns.length) { const testNames = failingDefns.map(d => d.id).join(', '); - console.error(log.redify(`We have ${failingDefns.length} failing smoketests: ${testNames}`)); + console.error(log.redify(`We have ${failingDefns.length} failing smoketest(s): ${testNames}`)); return {success: false, testResults}; } @@ -106,100 +100,32 @@ function assertNonNegativeInteger(loggableName, value) { } } -/** - * Run all the smoke tests specified, displaying output from each, in order, - * once all are finished. - * @param {ConcurrentMapper} concurrentMapper - * @param {Smokehouse.TestDfn} smokeTestDefn - * @param {{concurrency: number, retries: number, lighthouseRunner: Smokehouse.LighthouseRunner, isDebug?: boolean, takeNetworkRequestUrls?: () => string[]}} defnOptions - * @return {Promise} - */ -async function runSmokeTestDefn(concurrentMapper, smokeTestDefn, defnOptions) { - const {id, config: configJson, expectations} = smokeTestDefn; - const {concurrency, lighthouseRunner, retries, isDebug, takeNetworkRequestUrls} = defnOptions; - - const individualTests = expectations.map(expectation => { - return { - requestedUrl: expectation.lhr.requestedUrl, - configJson, - expectation, - lighthouseRunner, - retries, - isDebug, - takeNetworkRequestUrls, - }; - }); - - // Loop sequentially over expectations, comparing against Lighthouse run, and - // reporting result. - const results = await concurrentMapper.pooledMap(individualTests, (test, index) => { - if (index === 0) console.log(`${purpleify(id)} smoketest starting…`); - return runSmokeTest(test); - }, {concurrency}); - - console.log(`\n${purpleify(id)} smoketest results:`); - - let passingTestCount = 0; - let failingTestCount = 0; - for (const result of results) { - if (result.failed) { - failingTestCount++; - } else { - passingTestCount++; - } - - console.log(result.log); - } - - console.log(`${purpleify(id)} smoketest complete.`); - if (passingTestCount) { - console.log(log.greenify(` ${passingTestCount} test(s) passing`)); - } - if (failingTestCount) { - console.log(log.redify(` ${failingTestCount} test(s) failing`)); - } - console.log(''); // extra line break - - return { - id, - success: failingTestCount === 0, - expectationResults: results, - }; -} - /** @param {string} str */ function purpleify(str) { return `${log.purple}${str}${log.reset}`; } /** - * Run Lighthouse in the selected runner. Returns `log`` for logging once - * all tests in a defn are complete. - * @param {{requestedUrl: string, configJson?: LH.Config.Json, expectation: Smokehouse.ExpectedRunnerResult, lighthouseRunner: Smokehouse.LighthouseRunner, retries: number, isDebug?: boolean, takeNetworkRequestUrls?: () => string[]}} testOptions - * @return {Promise<{passed: number, failed: number, log: string}>} + * Run Lighthouse in the selected runner. + * @param {Smokehouse.TestDfn} smokeTestDefn + * @param {{isDebug?: boolean, retries: number, lighthouseRunner: Smokehouse.LighthouseRunner, takeNetworkRequestUrls?: () => string[]}} testOptions + * @return {Promise} */ -async function runSmokeTest(testOptions) { - // Use a buffered LocalConsole to keep logged output so it's not interleaved - // with other currently running tests. - const localConsole = new LocalConsole(); - const { - requestedUrl, - configJson, - expectation, - lighthouseRunner, - retries, - isDebug, - takeNetworkRequestUrls, - } = testOptions; +async function runSmokeTest(smokeTestDefn, testOptions) { + const {id, config: configJson, expectations} = smokeTestDefn; + const {lighthouseRunner, retries, isDebug, takeNetworkRequestUrls} = testOptions; + const requestedUrl = expectations.lhr.requestedUrl; + + console.log(`${purpleify(id)} smoketest starting…`); // Rerun test until there's a passing result or retries are exhausted to prevent flakes. let result; let report; + const bufferedConsole = new LocalConsole(); + bufferedConsole.log(`\n${purpleify(id)}: testing '${requestedUrl}'…`); for (let i = 0; i <= retries; i++) { - if (i === 0) { - localConsole.log(`Doing a run of '${requestedUrl}'...`); - } else { - localConsole.log(`Retrying run (${i} out of ${retries} retries)...`); + if (i !== 0) { + bufferedConsole.log(` Retrying run (${i} out of ${retries} retries)…`); } // Run Lighthouse. @@ -212,39 +138,47 @@ async function runSmokeTest(testOptions) { // Clear the network requests so that when we retry, we don't see duplicates. if (takeNetworkRequestUrls) takeNetworkRequestUrls(); - logChildProcessError(localConsole, e); + logChildProcessError(bufferedConsole, e); continue; // Retry, if possible. } // Assert result. - report = getAssertionReport(result, expectation, {isDebug}); + report = getAssertionReport(result, expectations, {isDebug}); if (report.failed) { - localConsole.log(`${report.failed} assertion(s) failed.`); + bufferedConsole.log(` ${getAssertionLog(report.failed)} failed.`); continue; // Retry, if possible. } break; // Passing result, no need to retry. } + bufferedConsole.log(` smoketest results:`); + // Write result log if we have one. - if (result) { - localConsole.write(result.log); - } + if (result) bufferedConsole.write(result.log); + + // If there's not an assertion report, just report the whole thing as a single failure. + if (report) bufferedConsole.write(report.log); + const passed = report ? report.passed : 0; + const failed = report ? report.failed : 1; - // Without an assertion report, not much detail to share but a failure. - if (!report) { - return { - passed: 0, - failed: 1, - log: localConsole.getLog(), - }; + const correctStr = getAssertionLog(passed); + const colorFn = passed === 0 ? log.redify : log.greenify; + bufferedConsole.log(` Correctly passed ${colorFn(correctStr)}`); + + if (failed) { + const failedString = getAssertionLog(failed); + bufferedConsole.log(` Failed ${log.redify(failedString)}`); } + bufferedConsole.log(`${purpleify(id)} smoketest complete.`); + + // Log now so right after finish, but all at once so not interleaved with other tests. + console.log(bufferedConsole.getLog()); - localConsole.write(report.log); return { - passed: report.passed, - failed: report.failed, - log: localConsole.getLog(), + id, + passed, + failed, }; } @@ -262,6 +196,15 @@ function logChildProcessError(localConsole, err) { localConsole.log(log.redify('Error: ') + err.message); } +/** + * @param {number} count + * @return {string} + */ +function getAssertionLog(count) { + const plural = count === 1 ? '' : 's'; + return `${count} assertion${plural}`; +} + module.exports = { runSmokehouse, }; diff --git a/lighthouse-cli/test/smokehouse/test-definitions/core-tests.js b/lighthouse-cli/test/smokehouse/test-definitions/core-tests.js index 7ab882157ff3..4ca442682f75 100644 --- a/lighthouse-cli/test/smokehouse/test-definitions/core-tests.js +++ b/lighthouse-cli/test/smokehouse/test-definitions/core-tests.js @@ -5,7 +5,7 @@ */ 'use strict'; -/** @type {ReadonlyArray} */ +/** @type {ReadonlyArray} */ const smokeTests = [{ id: 'a11y', expectations: require('./a11y/expectations.js'), diff --git a/types/smokehouse.d.ts b/types/smokehouse.d.ts index 13921cd1f06e..4526dd5b0793 100644 --- a/types/smokehouse.d.ts +++ b/types/smokehouse.d.ts @@ -18,18 +18,30 @@ export type ExpectedRunnerResult = { lhr: ExpectedLHR, - artifacts?: Partial> + artifacts?: Partial> networkRequests?: {length: number}; } export interface TestDfn { + /** Identification of test. Can be used for group selection (e.g. `yarn smoke pwa` will run all tests with `id.includes('pwa')`). */ id: string; - expectations: ExpectedRunnerResult[]; + /** Expected test results. */ + expectations: ExpectedRunnerResult; + /** An optional custom config. If none is present, uses the default Lighthouse config. */ config?: LH.Config.Json; /** If test is performance sensitive, set to true so that it won't be run parallel to other tests. */ runSerially?: boolean; } + /** + * A TestDefn type that's compatible with the old array of `expectations` type and the current TestDefn type. + * COMPAT: remove when no long needed. + * @deprecated + */ + export type BackCompatTestDefn = + Omit & + {expectations: Smokehouse.ExpectedRunnerResult | Array} + export type LighthouseRunner = (url: string, configJson?: LH.Config.Json, runnerOptions?: {isDebug?: boolean}) => Promise<{lhr: LH.Result, artifacts: LH.Artifacts, log: string}>;