From abf72452e070ca36625247bde10ede6eb6bccfd3 Mon Sep 17 00:00:00 2001 From: Adam Raine Date: Fri, 2 Apr 2021 12:30:31 -0400 Subject: [PATCH 1/9] tests: smoke request count assertion --- lighthouse-cli/test/fixtures/static-server.js | 28 +++++++++++++++++++ .../test/smokehouse/report-assert.js | 14 ++++++++++ types/smokehouse.d.ts | 1 + 3 files changed, 43 insertions(+) diff --git a/lighthouse-cli/test/fixtures/static-server.js b/lighthouse-cli/test/fixtures/static-server.js index 6df8ab4cc2dc..4210aec26bcd 100644 --- a/lighthouse-cli/test/fixtures/static-server.js +++ b/lighthouse-cli/test/fixtures/static-server.js @@ -28,6 +28,10 @@ class Server { this._server = http.createServer(this._requestHandler.bind(this)); /** @type {(data: string) => string=} */ this._dataTransformer = undefined; + /** @type {Map} */ + this._requestUrlsByFirstUrl = new Map(); + /** @type {Map} */ + this._requestUrlsBySocket = new Map(); } getPort() { @@ -62,8 +66,32 @@ class Server { this._dataTransformer = fn; } + /** + * @param {string} firstUrl + * @return {string[]} + */ + getRequestUrls(firstUrl) { + if (!this._requestUrlsByFirstUrl.size) { + for (const urlList of this._requestUrlsBySocket.values()) { + if (!urlList.length) continue; + this._requestUrlsByFirstUrl.set(urlList[0], urlList); + } + } + return this._requestUrlsByFirstUrl.get(firstUrl); + } + + /** + * @param {http.IncomingMessage} request + * @param {http.ServerResponse} response + */ _requestHandler(request, response) { const requestUrl = parseURL(request.url); + + // Collect URLs + const urlList = this._requestUrlsBySocket.get(request.socket) || []; + if (request.url !== '/favicon.ico' && request.url !== '/robots.txt') urlList.push(request.url); + this._requestUrlsBySocket.set(request.socket, urlList); + const filePath = requestUrl.pathname; const queryString = requestUrl.search && parseQueryString(requestUrl.search.slice(1)); let absoluteFilePath = path.join(this.baseDir, filePath); diff --git a/lighthouse-cli/test/smokehouse/report-assert.js b/lighthouse-cli/test/smokehouse/report-assert.js index 7fc1ebf7d8d3..61d1212159c6 100644 --- a/lighthouse-cli/test/smokehouse/report-assert.js +++ b/lighthouse-cli/test/smokehouse/report-assert.js @@ -13,6 +13,7 @@ const cloneDeep = require('lodash.clonedeep'); const log = require('lighthouse-logger'); const LocalConsole = require('./lib/local-console.js'); +const {server} = require('../fixtures/static-server.js'); const NUMBER_REGEXP = /(?:\d|\.)+/.source; const OPS_REGEXP = /<=?|>=?|\+\/-|±/.source; @@ -254,6 +255,18 @@ function collateResults(localConsole, actual, expected) { return makeComparison(auditName + ' audit', actualResult, expectedResult); }); + /** @type {Comparison[]} */ + const requestCountAssertion = []; + if (expected.networkRequests) { + const url = new URL(actual.lhr.requestedUrl); + const urlPath = url.pathname + url.search; + requestCountAssertion.push(makeComparison( + 'Request count', + server.getRequestUrls(urlPath), + expected.networkRequests + )); + } + return [ { name: 'final url', @@ -263,6 +276,7 @@ function collateResults(localConsole, actual, expected) { }, runtimeErrorAssertion, runWarningsAssertion, + ...requestCountAssertion, ...artifactAssertions, ...auditAssertions, ]; diff --git a/types/smokehouse.d.ts b/types/smokehouse.d.ts index a815c02663e4..90ba9288290f 100644 --- a/types/smokehouse.d.ts +++ b/types/smokehouse.d.ts @@ -19,6 +19,7 @@ export type ExpectedRunnerResult = { lhr: ExpectedLHR, artifacts?: Partial> + networkRequests?: any; } export interface TestDfn { From b4ddf5a7189991fdbfbc2c9c89c3e48fb816a87a Mon Sep 17 00:00:00 2001 From: Adam Raine Date: Tue, 6 Apr 2021 16:47:14 -0400 Subject: [PATCH 2/9] change --- lighthouse-cli/test/fixtures/static-server.js | 25 +++++++++++-------- .../test-definitions/perf/expectations.js | 15 +++++++++++ 2 files changed, 29 insertions(+), 11 deletions(-) diff --git a/lighthouse-cli/test/fixtures/static-server.js b/lighthouse-cli/test/fixtures/static-server.js index 4210aec26bcd..93eb3115cf9d 100644 --- a/lighthouse-cli/test/fixtures/static-server.js +++ b/lighthouse-cli/test/fixtures/static-server.js @@ -71,27 +71,30 @@ class Server { * @return {string[]} */ getRequestUrls(firstUrl) { - if (!this._requestUrlsByFirstUrl.size) { - for (const urlList of this._requestUrlsBySocket.values()) { - if (!urlList.length) continue; - this._requestUrlsByFirstUrl.set(urlList[0], urlList); - } + for (const urlList of this._requestUrlsBySocket.values()) { + if (!urlList.length) continue; + const firstUrl = urlList[0]; + this._requestUrlsByFirstUrl.set(firstUrl, urlList); } return this._requestUrlsByFirstUrl.get(firstUrl); } /** * @param {http.IncomingMessage} request - * @param {http.ServerResponse} response */ - _requestHandler(request, response) { - const requestUrl = parseURL(request.url); - - // Collect URLs + _updateRequestUrls(request) { const urlList = this._requestUrlsBySocket.get(request.socket) || []; - if (request.url !== '/favicon.ico' && request.url !== '/robots.txt') urlList.push(request.url); + if (!['/favicon.ico', '/robots.txt'].includes(request.url)) urlList.push(request.url); this._requestUrlsBySocket.set(request.socket, urlList); + } + /** + * @param {http.IncomingMessage} request + * @param {http.ServerResponse} response + */ + _requestHandler(request, response) { + const requestUrl = parseURL(request.url); + this._updateRequestUrls(request); const filePath = requestUrl.pathname; const queryString = requestUrl.search && parseQueryString(requestUrl.search.slice(1)); let absoluteFilePath = path.join(this.baseDir, filePath); diff --git a/lighthouse-cli/test/smokehouse/test-definitions/perf/expectations.js b/lighthouse-cli/test/smokehouse/test-definitions/perf/expectations.js index d3f01e61cad3..934a2f73f054 100644 --- a/lighthouse-cli/test/smokehouse/test-definitions/perf/expectations.js +++ b/lighthouse-cli/test/smokehouse/test-definitions/perf/expectations.js @@ -11,6 +11,9 @@ */ module.exports = [ { + networkRequests: { + length: 8, + }, lhr: { requestedUrl: 'http://localhost:10200/preload.html', finalUrl: 'http://localhost:10200/preload.html', @@ -62,6 +65,9 @@ module.exports = [ }, }, { + networkRequests: { + length: 8, + }, lhr: { requestedUrl: 'http://localhost:10200/perf/perf-budgets/load-things.html', finalUrl: 'http://localhost:10200/perf/perf-budgets/load-things.html', @@ -140,6 +146,9 @@ module.exports = [ }, }, { + networkRequests: { + length: 4, + }, lhr: { requestedUrl: 'http://localhost:10200/perf/fonts.html', finalUrl: 'http://localhost:10200/perf/fonts.html', @@ -168,6 +177,9 @@ module.exports = [ }, }, { + networkRequests: { + length: 3, + }, artifacts: { TraceElements: [ { @@ -291,6 +303,9 @@ module.exports = [ }, }, { + networkRequests: { + length: 2, + }, lhr: { requestedUrl: 'http://localhost:10200/perf/frame-metrics.html', finalUrl: 'http://localhost:10200/perf/frame-metrics.html', From 3fd2d0562c1af05b2cccfdf5fc97a59348ecd652 Mon Sep 17 00:00:00 2001 From: Adam Raine <6752989+adamraine@users.noreply.github.com> Date: Wed, 7 Apr 2021 12:30:17 -0400 Subject: [PATCH 3/9] Update lighthouse-cli/test/smokehouse/report-assert.js Co-authored-by: Connor Clark --- lighthouse-cli/test/smokehouse/report-assert.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lighthouse-cli/test/smokehouse/report-assert.js b/lighthouse-cli/test/smokehouse/report-assert.js index 61d1212159c6..06be7e8ff9d4 100644 --- a/lighthouse-cli/test/smokehouse/report-assert.js +++ b/lighthouse-cli/test/smokehouse/report-assert.js @@ -261,7 +261,7 @@ function collateResults(localConsole, actual, expected) { const url = new URL(actual.lhr.requestedUrl); const urlPath = url.pathname + url.search; requestCountAssertion.push(makeComparison( - 'Request count', + 'Requests', server.getRequestUrls(urlPath), expected.networkRequests )); From 5e71ba8dff4a9487c4bf985a4635f87d5b9997f2 Mon Sep 17 00:00:00 2001 From: Adam Raine Date: Fri, 9 Apr 2021 19:00:38 -0400 Subject: [PATCH 4/9] only serial --- lighthouse-cli/test/fixtures/static-server.js | 25 ++++++++----------- .../test/smokehouse/report-assert.js | 4 +-- lighthouse-cli/test/smokehouse/smokehouse.js | 25 +++++++++++++------ .../test-definitions/perf/expectations.js | 4 +-- 4 files changed, 30 insertions(+), 28 deletions(-) diff --git a/lighthouse-cli/test/fixtures/static-server.js b/lighthouse-cli/test/fixtures/static-server.js index 93eb3115cf9d..106600ef73fd 100644 --- a/lighthouse-cli/test/fixtures/static-server.js +++ b/lighthouse-cli/test/fixtures/static-server.js @@ -28,10 +28,8 @@ class Server { this._server = http.createServer(this._requestHandler.bind(this)); /** @type {(data: string) => string=} */ this._dataTransformer = undefined; - /** @type {Map} */ - this._requestUrlsByFirstUrl = new Map(); - /** @type {Map} */ - this._requestUrlsBySocket = new Map(); + /** @type {string[]} */ + this._requestUrls = []; } getPort() { @@ -66,26 +64,23 @@ class Server { this._dataTransformer = fn; } + clearRequestUrls() { + return this._requestUrls = []; + } + /** - * @param {string} firstUrl * @return {string[]} */ - getRequestUrls(firstUrl) { - for (const urlList of this._requestUrlsBySocket.values()) { - if (!urlList.length) continue; - const firstUrl = urlList[0]; - this._requestUrlsByFirstUrl.set(firstUrl, urlList); - } - return this._requestUrlsByFirstUrl.get(firstUrl); + getRequestUrls() { + return this._requestUrls; } /** * @param {http.IncomingMessage} request */ _updateRequestUrls(request) { - const urlList = this._requestUrlsBySocket.get(request.socket) || []; - if (!['/favicon.ico', '/robots.txt'].includes(request.url)) urlList.push(request.url); - this._requestUrlsBySocket.set(request.socket, urlList); + if (['/favicon.ico', '/robots.txt'].includes(request.url)) return; + this._requestUrls.push(request.url); } /** diff --git a/lighthouse-cli/test/smokehouse/report-assert.js b/lighthouse-cli/test/smokehouse/report-assert.js index 06be7e8ff9d4..e2e720204a2f 100644 --- a/lighthouse-cli/test/smokehouse/report-assert.js +++ b/lighthouse-cli/test/smokehouse/report-assert.js @@ -258,11 +258,9 @@ function collateResults(localConsole, actual, expected) { /** @type {Comparison[]} */ const requestCountAssertion = []; if (expected.networkRequests) { - const url = new URL(actual.lhr.requestedUrl); - const urlPath = url.pathname + url.search; requestCountAssertion.push(makeComparison( 'Requests', - server.getRequestUrls(urlPath), + server.getRequestUrls(), expected.networkRequests )); } diff --git a/lighthouse-cli/test/smokehouse/smokehouse.js b/lighthouse-cli/test/smokehouse/smokehouse.js index a17bb1a9e9ad..cc5bf0a1efce 100644 --- a/lighthouse-cli/test/smokehouse/smokehouse.js +++ b/lighthouse-cli/test/smokehouse/smokehouse.js @@ -16,6 +16,7 @@ const cliLighthouseRunner = require('./lighthouse-runners/cli.js').runLighthouse const getAssertionReport = require('./report-assert.js'); const LocalConsole = require('./lib/local-console.js'); const ConcurrentMapper = require('./lib/concurrent-mapper.js'); +const {server} = require('../fixtures/static-server.js'); /* eslint-disable no-console */ @@ -117,14 +118,19 @@ async function runSmokeTestDefn(concurrentMapper, smokeTestDefn, defnOptions) { const {id, config: configJson, expectations} = smokeTestDefn; const {concurrency, lighthouseRunner, retries, isDebug} = defnOptions; - const individualTests = expectations.map(expectation => ({ - requestedUrl: expectation.lhr.requestedUrl, - configJson, - expectation, - lighthouseRunner, - retries, - isDebug, - })); + const individualTests = expectations.map(expectation => { + if (expectation.networkRequests && concurrency > 1) { + throw new Error(`Test must be run serially to assert network requests (${id})`); + } + return { + requestedUrl: expectation.lhr.requestedUrl, + configJson, + expectation, + lighthouseRunner, + retries, + isDebug, + }; + }); // Loop sequentially over expectations, comparing against Lighthouse run, and // reporting result. @@ -190,6 +196,9 @@ async function runSmokeTest(testOptions) { localConsole.log(`Retrying run (${i} out of ${retries} retries)...`); } + // Ensure server only records URLs fetched for this run. + server.clearRequestUrls(); + // Run Lighthouse. try { result = await lighthouseRunner(requestedUrl, configJson, {isDebug}); diff --git a/lighthouse-cli/test/smokehouse/test-definitions/perf/expectations.js b/lighthouse-cli/test/smokehouse/test-definitions/perf/expectations.js index 934a2f73f054..5ce6282e2785 100644 --- a/lighthouse-cli/test/smokehouse/test-definitions/perf/expectations.js +++ b/lighthouse-cli/test/smokehouse/test-definitions/perf/expectations.js @@ -12,7 +12,7 @@ module.exports = [ { networkRequests: { - length: 8, + length: 9, }, lhr: { requestedUrl: 'http://localhost:10200/preload.html', @@ -147,7 +147,7 @@ module.exports = [ }, { networkRequests: { - length: 4, + length: 5, }, lhr: { requestedUrl: 'http://localhost:10200/perf/fonts.html', From f42b2f432cfcef940e7da716f561da4d36f42c73 Mon Sep 17 00:00:00 2001 From: Adam Raine Date: Tue, 13 Apr 2021 11:42:32 -0400 Subject: [PATCH 5/9] review --- lighthouse-cli/test/fixtures/static-server.js | 10 +++--- .../smokehouse/frontends/smokehouse-bin.js | 6 +++- .../test/smokehouse/report-assert.js | 26 +++++++++----- lighthouse-cli/test/smokehouse/smokehouse.js | 36 +++++++++++-------- types/smokehouse.d.ts | 4 ++- 5 files changed, 52 insertions(+), 30 deletions(-) diff --git a/lighthouse-cli/test/fixtures/static-server.js b/lighthouse-cli/test/fixtures/static-server.js index 106600ef73fd..67ae7aae354c 100644 --- a/lighthouse-cli/test/fixtures/static-server.js +++ b/lighthouse-cli/test/fixtures/static-server.js @@ -64,15 +64,13 @@ class Server { this._dataTransformer = fn; } - clearRequestUrls() { - return this._requestUrls = []; - } - /** * @return {string[]} */ - getRequestUrls() { - return this._requestUrls; + takeRequestUrls() { + const requestUrls = this._requestUrls; + this._requestUrls = []; + return requestUrls; } /** diff --git a/lighthouse-cli/test/smokehouse/frontends/smokehouse-bin.js b/lighthouse-cli/test/smokehouse/frontends/smokehouse-bin.js index 4eec27a18c33..b19357d294bd 100644 --- a/lighthouse-cli/test/smokehouse/frontends/smokehouse-bin.js +++ b/lighthouse-cli/test/smokehouse/frontends/smokehouse-bin.js @@ -132,7 +132,11 @@ async function begin() { const invertMatch = argv.invertMatch; const testDefns = getDefinitionsToRun(allTestDefns, requestedTestIds, {invertMatch}); - const options = {jobs, retries, isDebug: argv.debug, lighthouseRunner}; + const takeNetworkRequestUrls = () => { + return server.takeRequestUrls(); + }; + + const options = {jobs, retries, isDebug: argv.debug, lighthouseRunner, takeNetworkRequestUrls}; let isPassing; try { diff --git a/lighthouse-cli/test/smokehouse/report-assert.js b/lighthouse-cli/test/smokehouse/report-assert.js index e2e720204a2f..ee0fda04f8e2 100644 --- a/lighthouse-cli/test/smokehouse/report-assert.js +++ b/lighthouse-cli/test/smokehouse/report-assert.js @@ -13,7 +13,6 @@ const cloneDeep = require('lodash.clonedeep'); const log = require('lighthouse-logger'); const LocalConsole = require('./lib/local-console.js'); -const {server} = require('../fixtures/static-server.js'); const NUMBER_REGEXP = /(?:\d|\.)+/.source; const OPS_REGEXP = /<=?|>=?|\+\/-|±/.source; @@ -158,13 +157,13 @@ function makeComparison(name, actualResult, expectedResult) { * @param {LocalConsole} localConsole * @param {LH.Result} lhr * @param {Smokehouse.ExpectedRunnerResult} expected + * @param {boolean|undefined} isSync */ -function pruneExpectations(localConsole, lhr, expected) { +function pruneExpectations(localConsole, lhr, expected, isSync) { const userAgent = lhr.environment.hostUserAgent; const userAgentMatch = /Chrome\/(\d+)/.exec(userAgent); // Chrome/85.0.4174.0 if (!userAgentMatch) throw new Error('Could not get chrome version.'); const actualChromeVersion = Number(userAgentMatch[1]); - /** * @param {*} obj */ @@ -200,6 +199,17 @@ function pruneExpectations(localConsole, lhr, expected) { } const cloned = cloneDeep(expected); + + if (!isSync && expected.networkRequests) { + const msg = 'Network requests should only be asserted on tests run synchronously'; + if (process.env.CI) { + throw new Error(msg); + } else { + localConsole.log(`${msg}, pruning expectation: ${JSON.stringify(expected.networkRequests)}`); + delete cloned.networkRequests; + } + } + pruneNewerChromeExpectations(cloned); return cloned; } @@ -207,7 +217,7 @@ function pruneExpectations(localConsole, lhr, expected) { /** * Collate results into comparisons of actual and expected scores on each audit/artifact. * @param {LocalConsole} localConsole - * @param {{lhr: LH.Result, artifacts: LH.Artifacts}} actual + * @param {{lhr: LH.Result, artifacts: LH.Artifacts, networkRequests: string[]}} actual * @param {Smokehouse.ExpectedRunnerResult} expected * @return {Comparison[]} */ @@ -260,7 +270,7 @@ function collateResults(localConsole, actual, expected) { if (expected.networkRequests) { requestCountAssertion.push(makeComparison( 'Requests', - server.getRequestUrls(), + actual.networkRequests, expected.networkRequests )); } @@ -346,15 +356,15 @@ function assertLogString(count) { /** * Log all the comparisons between actual and expected test results, then print * summary. Returns count of passed and failed tests. - * @param {{lhr: LH.Result, artifacts: LH.Artifacts}} actual + * @param {{lhr: LH.Result, artifacts: LH.Artifacts, networkRequests: string[]}} actual * @param {Smokehouse.ExpectedRunnerResult} expected - * @param {{isDebug?: boolean}=} reportOptions + * @param {{isDebug?: boolean, isSync?: boolean}=} reportOptions * @return {{passed: number, failed: number, log: string}} */ function report(actual, expected, reportOptions = {}) { const localConsole = new LocalConsole(); - expected = pruneExpectations(localConsole, actual.lhr, expected); + expected = pruneExpectations(localConsole, actual.lhr, expected, reportOptions.isSync); const comparisons = collateResults(localConsole, actual, expected); let correctCount = 0; diff --git a/lighthouse-cli/test/smokehouse/smokehouse.js b/lighthouse-cli/test/smokehouse/smokehouse.js index cc5bf0a1efce..8eec6080c33a 100644 --- a/lighthouse-cli/test/smokehouse/smokehouse.js +++ b/lighthouse-cli/test/smokehouse/smokehouse.js @@ -16,7 +16,6 @@ const cliLighthouseRunner = require('./lighthouse-runners/cli.js').runLighthouse const getAssertionReport = require('./report-assert.js'); const LocalConsole = require('./lib/local-console.js'); const ConcurrentMapper = require('./lib/concurrent-mapper.js'); -const {server} = require('../fixtures/static-server.js'); /* eslint-disable no-console */ @@ -45,6 +44,7 @@ async function runSmokehouse(smokeTestDefns, smokehouseOptions = {}) { jobs = DEFAULT_CONCURRENT_RUNS, retries = DEFAULT_RETRIES, lighthouseRunner = cliLighthouseRunner, + takeNetworkRequestUrls = () => [], } = smokehouseOptions; assertPositiveInteger('jobs', jobs); assertNonNegativeInteger('retries', retries); @@ -55,7 +55,7 @@ async function runSmokehouse(smokeTestDefns, smokehouseOptions = {}) { for (const testDefn of smokeTestDefns) { // If defn is set to `runSerially`, we'll run its tests in succession, not parallel. const concurrency = testDefn.runSerially ? 1 : jobs; - const options = {concurrency, lighthouseRunner, retries, isDebug}; + const options = {concurrency, lighthouseRunner, retries, isDebug, takeNetworkRequestUrls}; const result = runSmokeTestDefn(concurrentMapper, testDefn, options); smokePromises.push(result); } @@ -111,17 +111,14 @@ function assertNonNegativeInteger(loggableName, value) { * once all are finished. * @param {ConcurrentMapper} concurrentMapper * @param {Smokehouse.TestDfn} smokeTestDefn - * @param {{concurrency: number, retries: number, lighthouseRunner: Smokehouse.LighthouseRunner, isDebug?: boolean}} defnOptions + * @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} = defnOptions; + const {concurrency, lighthouseRunner, retries, isDebug, takeNetworkRequestUrls} = defnOptions; const individualTests = expectations.map(expectation => { - if (expectation.networkRequests && concurrency > 1) { - throw new Error(`Test must be run serially to assert network requests (${id})`); - } return { requestedUrl: expectation.lhr.requestedUrl, configJson, @@ -129,6 +126,8 @@ async function runSmokeTestDefn(concurrentMapper, smokeTestDefn, defnOptions) { lighthouseRunner, retries, isDebug, + isSync: concurrency === 1, + takeNetworkRequestUrls, }; }); @@ -177,14 +176,23 @@ function purpleify(str) { /** * 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}} testOptions + * @param {{requestedUrl: string, configJson?: LH.Config.Json, expectation: Smokehouse.ExpectedRunnerResult, lighthouseRunner: Smokehouse.LighthouseRunner, retries: number, isDebug?: boolean, isSync?: boolean, takeNetworkRequestUrls: () => string[]}} testOptions * @return {Promise<{passed: number, failed: number, log: string}>} */ 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} = testOptions; + const { + requestedUrl, + configJson, + expectation, + lighthouseRunner, + retries, + isDebug, + isSync, + takeNetworkRequestUrls, + } = testOptions; // Rerun test until there's a passing result or retries are exhausted to prevent flakes. let result; @@ -196,19 +204,19 @@ async function runSmokeTest(testOptions) { localConsole.log(`Retrying run (${i} out of ${retries} retries)...`); } - // Ensure server only records URLs fetched for this run. - server.clearRequestUrls(); - // Run Lighthouse. try { - result = await lighthouseRunner(requestedUrl, configJson, {isDebug}); + result = { + ...await lighthouseRunner(requestedUrl, configJson, {isDebug}), + ...{networkRequests: takeNetworkRequestUrls()}, + }; } catch (e) { logChildProcessError(localConsole, e); continue; // Retry, if possible. } // Assert result. - report = getAssertionReport(result, expectation, {isDebug}); + report = getAssertionReport(result, expectation, {isDebug, isSync}); if (report.failed) { localConsole.log(`${report.failed} assertion(s) failed.`); continue; // Retry, if possible. diff --git a/types/smokehouse.d.ts b/types/smokehouse.d.ts index 90ba9288290f..13921cd1f06e 100644 --- a/types/smokehouse.d.ts +++ b/types/smokehouse.d.ts @@ -19,7 +19,7 @@ export type ExpectedRunnerResult = { lhr: ExpectedLHR, artifacts?: Partial> - networkRequests?: any; + networkRequests?: {length: number}; } export interface TestDfn { @@ -42,6 +42,8 @@ retries?: number; /** A function that runs Lighthouse with the given options. Defaults to running Lighthouse via the CLI. */ lighthouseRunner?: LighthouseRunner; + /** A function that gets a list of URLs requested to the server since the last fetch. */ + takeNetworkRequestUrls?: () => string[]; } export interface SmokehouseLibOptions extends SmokehouseOptions { From e578427bdb447391cca7749ceea45db76d644f63 Mon Sep 17 00:00:00 2001 From: Adam Raine Date: Tue, 13 Apr 2021 14:17:18 -0400 Subject: [PATCH 6/9] nits --- lighthouse-cli/test/fixtures/static-server.js | 2 ++ lighthouse-cli/test/smokehouse/report-assert.js | 2 ++ lighthouse-cli/test/smokehouse/smokehouse.js | 2 +- 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/lighthouse-cli/test/fixtures/static-server.js b/lighthouse-cli/test/fixtures/static-server.js index 67ae7aae354c..44f12f045038 100644 --- a/lighthouse-cli/test/fixtures/static-server.js +++ b/lighthouse-cli/test/fixtures/static-server.js @@ -77,6 +77,8 @@ class Server { * @param {http.IncomingMessage} request */ _updateRequestUrls(request) { + // Favicon is not fetched in headless mode and robots is not fetched by every test. + // Ignoring these makes the assertion much simpler. if (['/favicon.ico', '/robots.txt'].includes(request.url)) return; this._requestUrls.push(request.url); } diff --git a/lighthouse-cli/test/smokehouse/report-assert.js b/lighthouse-cli/test/smokehouse/report-assert.js index ee0fda04f8e2..4c2814da4f87 100644 --- a/lighthouse-cli/test/smokehouse/report-assert.js +++ b/lighthouse-cli/test/smokehouse/report-assert.js @@ -200,6 +200,8 @@ function pruneExpectations(localConsole, lhr, expected, isSync) { const cloned = cloneDeep(expected); + // Tests must be run synchronously so we can clear the request list between tests. + // We do not have a good way to map requests to test definitions if the tests are run in parallel. if (!isSync && expected.networkRequests) { const msg = 'Network requests should only be asserted on tests run synchronously'; if (process.env.CI) { diff --git a/lighthouse-cli/test/smokehouse/smokehouse.js b/lighthouse-cli/test/smokehouse/smokehouse.js index 8eec6080c33a..9d9655841f83 100644 --- a/lighthouse-cli/test/smokehouse/smokehouse.js +++ b/lighthouse-cli/test/smokehouse/smokehouse.js @@ -208,7 +208,7 @@ async function runSmokeTest(testOptions) { try { result = { ...await lighthouseRunner(requestedUrl, configJson, {isDebug}), - ...{networkRequests: takeNetworkRequestUrls()}, + networkRequests: takeNetworkRequestUrls(), }; } catch (e) { logChildProcessError(localConsole, e); From 46ea5909501b0ca79f8f63561b20a51e06baa2fb Mon Sep 17 00:00:00 2001 From: Adam Raine Date: Tue, 13 Apr 2021 14:48:53 -0400 Subject: [PATCH 7/9] require --- lighthouse-cli/test/smokehouse/smokehouse.js | 6 +++--- types/smokehouse.d.ts | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lighthouse-cli/test/smokehouse/smokehouse.js b/lighthouse-cli/test/smokehouse/smokehouse.js index 9d9655841f83..ee78a97a1e35 100644 --- a/lighthouse-cli/test/smokehouse/smokehouse.js +++ b/lighthouse-cli/test/smokehouse/smokehouse.js @@ -35,16 +35,16 @@ const DEFAULT_RETRIES = 0; /** * Runs the selected smoke tests. Returns whether all assertions pass. * @param {Array} smokeTestDefns - * @param {Smokehouse.SmokehouseOptions=} smokehouseOptions + * @param {Smokehouse.SmokehouseOptions} smokehouseOptions * @return {Promise<{success: boolean, testResults: SmokehouseResult[]}>} */ -async function runSmokehouse(smokeTestDefns, smokehouseOptions = {}) { +async function runSmokehouse(smokeTestDefns, smokehouseOptions) { const { isDebug, jobs = DEFAULT_CONCURRENT_RUNS, retries = DEFAULT_RETRIES, lighthouseRunner = cliLighthouseRunner, - takeNetworkRequestUrls = () => [], + takeNetworkRequestUrls, } = smokehouseOptions; assertPositiveInteger('jobs', jobs); assertNonNegativeInteger('retries', retries); diff --git a/types/smokehouse.d.ts b/types/smokehouse.d.ts index 13921cd1f06e..f36b18c8d3ce 100644 --- a/types/smokehouse.d.ts +++ b/types/smokehouse.d.ts @@ -43,7 +43,7 @@ /** A function that runs Lighthouse with the given options. Defaults to running Lighthouse via the CLI. */ lighthouseRunner?: LighthouseRunner; /** A function that gets a list of URLs requested to the server since the last fetch. */ - takeNetworkRequestUrls?: () => string[]; + takeNetworkRequestUrls: () => string[]; } export interface SmokehouseLibOptions extends SmokehouseOptions { From 10964525ea84ab0c865c46a402bdfb0d3341103e Mon Sep 17 00:00:00 2001 From: Adam Raine Date: Tue, 13 Apr 2021 16:21:14 -0400 Subject: [PATCH 8/9] add dbw --- lighthouse-cli/test/smokehouse/test-definitions/core-tests.js | 1 + .../test-definitions/dobetterweb/dbw-expectations.js | 3 +++ 2 files changed, 4 insertions(+) diff --git a/lighthouse-cli/test/smokehouse/test-definitions/core-tests.js b/lighthouse-cli/test/smokehouse/test-definitions/core-tests.js index cdada35caef6..273dfde63a8e 100644 --- a/lighthouse-cli/test/smokehouse/test-definitions/core-tests.js +++ b/lighthouse-cli/test/smokehouse/test-definitions/core-tests.js @@ -35,6 +35,7 @@ const smokeTests = [{ id: 'dbw', expectations: require('./dobetterweb/dbw-expectations.js'), config: require('./dobetterweb/dbw-config.js'), + runSerially: true, // Need access to network request assertions. }, { id: 'redirects', expectations: require('./redirects/expectations.js'), diff --git a/lighthouse-cli/test/smokehouse/test-definitions/dobetterweb/dbw-expectations.js b/lighthouse-cli/test/smokehouse/test-definitions/dobetterweb/dbw-expectations.js index c09b6a954bc8..ea8408b005ec 100644 --- a/lighthouse-cli/test/smokehouse/test-definitions/dobetterweb/dbw-expectations.js +++ b/lighthouse-cli/test/smokehouse/test-definitions/dobetterweb/dbw-expectations.js @@ -11,6 +11,9 @@ */ const expectations = [ { + networkRequests: { + length: 59, + }, artifacts: { HostFormFactor: 'desktop', Stacks: [{ From 9bb6c76efc1ab164a1856c832483071d59325ffe Mon Sep 17 00:00:00 2001 From: Adam Raine Date: Thu, 15 Apr 2021 13:43:28 -0400 Subject: [PATCH 9/9] comments --- .../test-definitions/dobetterweb/dbw-expectations.js | 3 +++ .../test/smokehouse/test-definitions/perf/expectations.js | 2 ++ 2 files changed, 5 insertions(+) diff --git a/lighthouse-cli/test/smokehouse/test-definitions/dobetterweb/dbw-expectations.js b/lighthouse-cli/test/smokehouse/test-definitions/dobetterweb/dbw-expectations.js index ea8408b005ec..30bd211c72d6 100644 --- a/lighthouse-cli/test/smokehouse/test-definitions/dobetterweb/dbw-expectations.js +++ b/lighthouse-cli/test/smokehouse/test-definitions/dobetterweb/dbw-expectations.js @@ -12,6 +12,9 @@ const expectations = [ { networkRequests: { + // 50 requests made for normal page testing. + // 6 extra requests made because stylesheets are evicted from the cache by the time DT opens. + // 3 extra requests made to /dobetterweb/clock.appcache length: 59, }, artifacts: { diff --git a/lighthouse-cli/test/smokehouse/test-definitions/perf/expectations.js b/lighthouse-cli/test/smokehouse/test-definitions/perf/expectations.js index 9bc6d179881a..fbdbccd43f2a 100644 --- a/lighthouse-cli/test/smokehouse/test-definitions/perf/expectations.js +++ b/lighthouse-cli/test/smokehouse/test-definitions/perf/expectations.js @@ -12,6 +12,8 @@ module.exports = [ { networkRequests: { + // 8 requests made for normal page testing. + // 1 extra request made because stylesheets are evicted from the cache by the time DT opens. length: 9, }, lhr: {