Skip to content

Commit

Permalink
tests: smoke request count assertion (#12325)
Browse files Browse the repository at this point in the history
  • Loading branch information
adamraine authored Apr 15, 2021
1 parent bfa5351 commit 35aec59
Show file tree
Hide file tree
Showing 8 changed files with 122 additions and 24 deletions.
26 changes: 26 additions & 0 deletions lighthouse-cli/test/fixtures/static-server.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ class Server {
this._server = http.createServer(this._requestHandler.bind(this));
/** @type {(data: string) => string=} */
this._dataTransformer = undefined;
/** @type {string[]} */
this._requestUrls = [];
}

getPort() {
Expand Down Expand Up @@ -62,8 +64,32 @@ class Server {
this._dataTransformer = fn;
}

/**
* @return {string[]}
*/
takeRequestUrls() {
const requestUrls = this._requestUrls;
this._requestUrls = [];
return requestUrls;
}

/**
* @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);
}

/**
* @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);
Expand Down
6 changes: 5 additions & 1 deletion lighthouse-cli/test/smokehouse/frontends/smokehouse-bin.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
36 changes: 30 additions & 6 deletions lighthouse-cli/test/smokehouse/report-assert.js
Original file line number Diff line number Diff line change
Expand Up @@ -157,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
*/
Expand Down Expand Up @@ -199,14 +199,27 @@ function pruneExpectations(localConsole, lhr, expected) {
}

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) {
throw new Error(msg);
} else {
localConsole.log(`${msg}, pruning expectation: ${JSON.stringify(expected.networkRequests)}`);
delete cloned.networkRequests;
}
}

pruneNewerChromeExpectations(cloned);
return cloned;
}

/**
* 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[]}
*/
Expand Down Expand Up @@ -254,6 +267,16 @@ function collateResults(localConsole, actual, expected) {
return makeComparison(auditName + ' audit', actualResult, expectedResult);
});

/** @type {Comparison[]} */
const requestCountAssertion = [];
if (expected.networkRequests) {
requestCountAssertion.push(makeComparison(
'Requests',
actual.networkRequests,
expected.networkRequests
));
}

return [
{
name: 'final url',
Expand All @@ -263,6 +286,7 @@ function collateResults(localConsole, actual, expected) {
},
runtimeErrorAssertion,
runWarningsAssertion,
...requestCountAssertion,
...artifactAssertions,
...auditAssertions,
];
Expand Down Expand Up @@ -334,15 +358,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;
Expand Down
51 changes: 34 additions & 17 deletions lighthouse-cli/test/smokehouse/smokehouse.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,15 +35,16 @@ const DEFAULT_RETRIES = 0;
/**
* Runs the selected smoke tests. Returns whether all assertions pass.
* @param {Array<Smokehouse.TestDfn>} 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,
} = smokehouseOptions;
assertPositiveInteger('jobs', jobs);
assertNonNegativeInteger('retries', retries);
Expand All @@ -54,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);
}
Expand Down Expand Up @@ -110,21 +111,25 @@ 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<SmokehouseResult>}
*/
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 => ({
requestedUrl: expectation.lhr.requestedUrl,
configJson,
expectation,
lighthouseRunner,
retries,
isDebug,
}));
const individualTests = expectations.map(expectation => {
return {
requestedUrl: expectation.lhr.requestedUrl,
configJson,
expectation,
lighthouseRunner,
retries,
isDebug,
isSync: concurrency === 1,
takeNetworkRequestUrls,
};
});

// Loop sequentially over expectations, comparing against Lighthouse run, and
// reporting result.
Expand Down Expand Up @@ -171,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;
Expand All @@ -192,14 +206,17 @@ async function runSmokeTest(testOptions) {

// 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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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'),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,12 @@
*/
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: {
HostFormFactor: 'desktop',
Stacks: [{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,11 @@
*/
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: {
requestedUrl: 'http://localhost:10200/preload.html',
finalUrl: 'http://localhost:10200/preload.html',
Expand Down Expand Up @@ -62,6 +67,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',
Expand Down Expand Up @@ -140,6 +148,9 @@ module.exports = [
},
},
{
networkRequests: {
length: 5,
},
lhr: {
requestedUrl: 'http://localhost:10200/perf/fonts.html',
finalUrl: 'http://localhost:10200/perf/fonts.html',
Expand Down Expand Up @@ -168,6 +179,9 @@ module.exports = [
},
},
{
networkRequests: {
length: 3,
},
artifacts: {
TraceElements: [
{
Expand Down Expand Up @@ -291,6 +305,9 @@ module.exports = [
},
},
{
networkRequests: {
length: 2,
},
lhr: {
requestedUrl: 'http://localhost:10200/perf/frame-metrics.html',
finalUrl: 'http://localhost:10200/perf/frame-metrics.html',
Expand Down
3 changes: 3 additions & 0 deletions types/smokehouse.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
export type ExpectedRunnerResult = {
lhr: ExpectedLHR,
artifacts?: Partial<Record<keyof LH.Artifacts, any>>
networkRequests?: {length: number};
}

export interface TestDfn {
Expand All @@ -41,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 {
Expand Down

0 comments on commit 35aec59

Please sign in to comment.