Skip to content

Commit

Permalink
tests(smokehouse): always assert lhr.runtimeError (#9130)
Browse files Browse the repository at this point in the history
  • Loading branch information
brendankenny authored Jun 6, 2019
1 parent 9e97e10 commit 4a83d2b
Show file tree
Hide file tree
Showing 9 changed files with 46 additions and 15 deletions.
2 changes: 2 additions & 0 deletions lighthouse-cli/test/smokehouse/error-config.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ module.exports = {
maxWaitForLoad: 5000,
onlyAudits: [
'first-contentful-paint',
// TODO: this audit collects a non-base artifact, allowing the runtimeError to be collected.
'content-width',
],
},
};
8 changes: 8 additions & 0 deletions lighthouse-cli/test/smokehouse/error-expectations.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,23 @@ module.exports = [
requestedUrl: 'http://localhost:10200/infinite-loop.html',
finalUrl: 'http://localhost:10200/infinite-loop.html',
audits: {},
// TODO: runtimeError only exists because of selection of audits.
runtimeError: {code: 'PAGE_HUNG'},
},
artifacts: {
ViewportDimensions: {code: 'PAGE_HUNG'},
},
},
{
lhr: {
requestedUrl: 'https://expired.badssl.com',
finalUrl: 'https://expired.badssl.com/',
audits: {},
// TODO: runtimeError only exists because of selection of audits.
runtimeError: {code: 'FAILED_DOCUMENT_REQUEST'},
},
artifacts: {
ViewportDimensions: {code: 'FAILED_DOCUMENT_REQUEST'},
},
},
];
4 changes: 4 additions & 0 deletions lighthouse-cli/test/smokehouse/seo/expectations.js
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,10 @@ module.exports = [
// Note: most scores are null (audit error) because the page 403ed.
requestedUrl: BASE_URL + 'seo-failure-cases.html?status_code=403',
finalUrl: BASE_URL + 'seo-failure-cases.html?status_code=403',
runtimeError: {
code: 'ERRORED_DOCUMENT_REQUEST',
message: /Status code: 403/,
},
audits: {
'http-status-code': {
score: 0,
Expand Down
9 changes: 8 additions & 1 deletion lighthouse-cli/test/smokehouse/smokehouse-report.js
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,11 @@ function makeComparison(name, actualResult, expectedResult) {
* @return {Smokehouse.Comparison[]}
*/
function collateResults(actual, expected) {
// If actual run had a runtimeError, expected *must* have a runtimeError.
// Relies on the fact that an `undefined` argument to makeComparison() can only match `undefined`.
const runtimeErrorAssertion = makeComparison('runtimeError', actual.lhr.runtimeError,
expected.lhr.runtimeError);

/** @type {Smokehouse.Comparison[]} */
let artifactAssertions = [];
if (expected.artifacts) {
Expand Down Expand Up @@ -161,6 +166,7 @@ function collateResults(actual, expected) {
expected: expected.lhr.finalUrl,
equal: actual.lhr.finalUrl === expected.lhr.finalUrl,
},
runtimeErrorAssertion,
...artifactAssertions,
...auditAssertions,
];
Expand Down Expand Up @@ -194,7 +200,8 @@ function reportAssertion(assertion) {
} else {
if (assertion.diff) {
const diff = assertion.diff;
const fullActual = JSON.stringify(assertion.actual, null, 2).replace(/\n/g, '\n ');
const fullActual = String(JSON.stringify(assertion.actual, null, 2))
.replace(/\n/g, '\n ');
const msg = `
${log.redify(log.cross)} difference at ${log.bold}${diff.path}${log.reset}
expected: ${JSON.stringify(diff.expected)}
Expand Down
26 changes: 18 additions & 8 deletions lighthouse-cli/test/smokehouse/smokehouse.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,10 @@ const path = require('path');
const spawnSync = require('child_process').spawnSync;
const yargs = require('yargs');
const log = require('lighthouse-logger');
const rimraf = require('rimraf');

const {collateResults, report} = require('./smokehouse-report.js');
const assetSaver = require('../../../lighthouse-core/lib/asset-saver.js');

const PROTOCOL_TIMEOUT_EXIT_CODE = 67;
const RETRIES = 3;
Expand Down Expand Up @@ -88,27 +91,34 @@ function runLighthouse(url, configPath, isDebug) {
console.error(`STDERR: ${runResults.stderr}`);
}

if (runResults.status === PROTOCOL_TIMEOUT_EXIT_CODE) {
const exitCode = runResults.status;
if (exitCode === PROTOCOL_TIMEOUT_EXIT_CODE) {
console.error(`Lighthouse debugger connection timed out ${RETRIES} times. Giving up.`);
process.exit(1);
} else if (!fs.existsSync(outputPath)) {
console.error(`Lighthouse run failed to produce a report and exited with ${runResults.status}. stderr to follow:`); // eslint-disable-line max-len
console.error(`Lighthouse run failed to produce a report and exited with ${exitCode}. stderr to follow:`); // eslint-disable-line max-len
console.error(runResults.stderr);
process.exit(runResults.status);
process.exit(exitCode);
}

/** @type {LH.Result} */
const lhr = JSON.parse(fs.readFileSync(outputPath, 'utf8'));
const artifacts = assetSaver.loadArtifacts(artifactsDirectory);

if (isDebug) {
console.log('LHR output available at: ', outputPath);
console.log('Artifacts avaiable in: ', artifactsDirectory);
} else {
fs.unlinkSync(outputPath);
rimraf.sync(artifactsDirectory);
}

// Artifacts are undefined if they weren't written to disk (e.g. if there was an error).
let artifacts;
try {
artifacts = JSON.parse(fs.readFileSync(`${artifactsDirectory}/artifacts.json`, 'utf8'));
} catch (e) {}
// There should either be both an error exitCode and a lhr.runtimeError or neither.
if (Boolean(exitCode) !== Boolean(lhr.runtimeError)) {
const runtimeErrorCode = lhr.runtimeError && lhr.runtimeError.code;
console.error(`Lighthouse did not exit with an error correctly, exiting with ${exitCode} but with runtimeError '${runtimeErrorCode}'`); // eslint-disable-line max-len
process.exit(1);
}

return {
lhr,
Expand Down
4 changes: 2 additions & 2 deletions lighthouse-core/lib/asset-saver.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,9 @@ const devtoolsLogSuffix = '.devtoolslog.json';
* Load artifacts object from files located within basePath
* Also save the traces to their own files
* @param {string} basePath
* @return {Promise<LH.Artifacts>}
* @return {LH.Artifacts}
*/
async function loadArtifacts(basePath) {
function loadArtifacts(basePath) {
log.log('Reading artifacts from disk:', basePath);

if (!fs.existsSync(basePath)) {
Expand Down
2 changes: 1 addition & 1 deletion lighthouse-core/runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ class Runner {
if (settings.auditMode && !settings.gatherMode) {
// No browser required, just load the artifacts from disk.
const path = Runner._getArtifactsPath(settings);
artifacts = await assetSaver.loadArtifacts(path);
artifacts = assetSaver.loadArtifacts(path);
requestedUrl = artifacts.URL.requestedUrl;

if (!requestedUrl) {
Expand Down
4 changes: 2 additions & 2 deletions lighthouse-core/scripts/update-report-fixtures.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ async function update(artifactName) {
res(address.port);
}));

const oldArtifacts = await assetSaver.loadArtifacts(artifactPath);
const oldArtifacts = assetSaver.loadArtifacts(artifactPath);

const url = `http://localhost:${port}/dobetterweb/dbw_tester.html`;
const rawFlags = [
Expand All @@ -41,7 +41,7 @@ async function update(artifactName) {

if (artifactName) {
// Revert everything except the one artifact
const newArtifacts = await assetSaver.loadArtifacts(artifactPath);
const newArtifacts = assetSaver.loadArtifacts(artifactPath);
if (!(artifactName in newArtifacts) && !(artifactName in oldArtifacts)) {
throw Error('Unknown artifact name: ' + artifactName);
}
Expand Down
2 changes: 1 addition & 1 deletion types/smokehouse.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
diff?: Difference | null;
}

export type ExpectedLHR = Pick<LH.Result, 'audits' | 'finalUrl' | 'requestedUrl'>
export type ExpectedLHR = Pick<LH.Result, 'audits' | 'finalUrl' | 'requestedUrl' | 'runtimeError'>

export type ExpectedRunnerResult = {
lhr: ExpectedLHR,
Expand Down

0 comments on commit 4a83d2b

Please sign in to comment.