From ffca488c2d5af02505a28b1d6aa00872b48c429f Mon Sep 17 00:00:00 2001 From: Shane Exterkamp Date: Fri, 1 Mar 2019 15:57:45 -0800 Subject: [PATCH 1/5] Remove NO_ERROR from response json. --- lighthouse-core/runner.js | 7 ++----- types/lhr.d.ts | 2 +- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/lighthouse-core/runner.js b/lighthouse-core/runner.js index e08294dc111a..b021b81cb702 100644 --- a/lighthouse-core/runner.js +++ b/lighthouse-core/runner.js @@ -331,7 +331,7 @@ class Runner { /** * Returns first runtimeError found in artifacts. * @param {LH.Artifacts} artifacts - * @return {LH.Result['runtimeError']} + * @return {LH.Result['runtimeError']|undefined} */ static getArtifactRuntimeError(artifacts) { for (const possibleErrorArtifact of Object.values(artifacts)) { @@ -345,10 +345,7 @@ class Runner { } } - return { - code: LHError.NO_ERROR, - message: '', - }; + return undefined; } /** diff --git a/types/lhr.d.ts b/types/lhr.d.ts index d1b25612db85..6e7193fe2845 100644 --- a/types/lhr.d.ts +++ b/types/lhr.d.ts @@ -53,7 +53,7 @@ declare global { /** List of top-level warnings for this Lighthouse run. */ runWarnings: string[]; /** A top-level error message that, if present, indicates a serious enough problem that this Lighthouse result may need to be discarded. */ - runtimeError: {code: string, message: string}; + runtimeError?: {code: string, message: string}; /** The User-Agent string of the browser used run Lighthouse for these results. */ userAgent: string; /** Information about the environment in which Lighthouse was run. */ From eb2b89a2b6c644ae4b954c3d6aef888de1c7a49f Mon Sep 17 00:00:00 2001 From: Shane Exterkamp Date: Fri, 1 Mar 2019 16:29:49 -0800 Subject: [PATCH 2/5] Update sample json. --- lighthouse-core/test/results/sample_v2.json | 4 ---- 1 file changed, 4 deletions(-) diff --git a/lighthouse-core/test/results/sample_v2.json b/lighthouse-core/test/results/sample_v2.json index fb0b83114dc0..6c72f1e38335 100644 --- a/lighthouse-core/test/results/sample_v2.json +++ b/lighthouse-core/test/results/sample_v2.json @@ -10,10 +10,6 @@ "requestedUrl": "http://localhost:10200/dobetterweb/dbw_tester.html", "finalUrl": "http://localhost:10200/dobetterweb/dbw_tester.html", "runWarnings": [], - "runtimeError": { - "code": "NO_ERROR", - "message": "" - }, "audits": { "is-on-https": { "id": "is-on-https", From 6bd033c436bb3f2ff51478c6579e2bb3b2de5087 Mon Sep 17 00:00:00 2001 From: Shane Exterkamp Date: Mon, 4 Mar 2019 13:27:46 -0800 Subject: [PATCH 3/5] Added non-zero exit for cli w/runtime errors. --- lighthouse-cli/index.js | 7 ++++++- lighthouse-cli/test/cli/index-test.js | 16 ++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/lighthouse-cli/index.js b/lighthouse-cli/index.js index f89edfa6eb6c..3d8d8d47e88b 100755 --- a/lighthouse-cli/index.js +++ b/lighthouse-cli/index.js @@ -6,7 +6,12 @@ */ 'use strict'; -require('./bin.js').begin().catch(err => { +require('./bin.js').begin().then((lhr) => { + // Exit with non-zero exit code if LHR contains runtime error. + if (lhr && lhr.lhr.runtimeError) { + process.exit(1); + } +}).catch(err => { process.stderr.write(err.stack); process.exit(1); }); diff --git a/lighthouse-cli/test/cli/index-test.js b/lighthouse-cli/test/cli/index-test.js index 11d31388b313..e819a91c8d1b 100644 --- a/lighthouse-cli/test/cli/index-test.js +++ b/lighthouse-cli/test/cli/index-test.js @@ -75,6 +75,7 @@ describe('CLI Tests', function() { const config = JSON.parse(ret.stdout); assert.strictEqual(config.settings.output[0], 'html'); assert.strictEqual(config.settings.auditMode, false); + assert.equal(ret.status, 0); expect(config).toMatchSnapshot(); }); @@ -91,8 +92,23 @@ describe('CLI Tests', function() { assert.strictEqual(config.settings.output[0], 'json'); assert.strictEqual(config.settings.auditMode, true); assert.strictEqual(config.audits.length, 1); + assert.equal(ret.status, 0); expect(config).toMatchSnapshot(); }); }); + + describe('non-zero exit codes', () => { + it('should exit with code 1 if the LHR contains an error', () => { + const flags = [ + 'chrome://error', + `--max-wait-for-load=${9000}`, + '--output-path=stdout', + ]; + const ret = spawnSync('node', [indexPath, ...flags], {encoding: 'utf8'}); + + // Exit with non-zero code. + assert.equal(ret.status, 1); + }, 20 * 1000); + }); }); From 6328238e6cba9f2496c8e9391e530abeecf60134 Mon Sep 17 00:00:00 2001 From: Shane Exterkamp Date: Mon, 4 Mar 2019 14:36:28 -0800 Subject: [PATCH 4/5] Rollback exit code code, add Brendan's test. --- lighthouse-cli/index.js | 7 +------ lighthouse-cli/test/cli/index-test.js | 14 -------------- lighthouse-core/test/runner-test.js | 15 +++++++++++++++ 3 files changed, 16 insertions(+), 20 deletions(-) diff --git a/lighthouse-cli/index.js b/lighthouse-cli/index.js index 3d8d8d47e88b..f89edfa6eb6c 100755 --- a/lighthouse-cli/index.js +++ b/lighthouse-cli/index.js @@ -6,12 +6,7 @@ */ 'use strict'; -require('./bin.js').begin().then((lhr) => { - // Exit with non-zero exit code if LHR contains runtime error. - if (lhr && lhr.lhr.runtimeError) { - process.exit(1); - } -}).catch(err => { +require('./bin.js').begin().catch(err => { process.stderr.write(err.stack); process.exit(1); }); diff --git a/lighthouse-cli/test/cli/index-test.js b/lighthouse-cli/test/cli/index-test.js index e819a91c8d1b..574781a45fac 100644 --- a/lighthouse-cli/test/cli/index-test.js +++ b/lighthouse-cli/test/cli/index-test.js @@ -97,18 +97,4 @@ describe('CLI Tests', function() { expect(config).toMatchSnapshot(); }); }); - - describe('non-zero exit codes', () => { - it('should exit with code 1 if the LHR contains an error', () => { - const flags = [ - 'chrome://error', - `--max-wait-for-load=${9000}`, - '--output-path=stdout', - ]; - const ret = spawnSync('node', [indexPath, ...flags], {encoding: 'utf8'}); - - // Exit with non-zero code. - assert.equal(ret.status, 1); - }, 20 * 1000); - }); }); diff --git a/lighthouse-core/test/runner-test.js b/lighthouse-core/test/runner-test.js index 52230e3a634f..18a05dab68cd 100644 --- a/lighthouse-core/test/runner-test.js +++ b/lighthouse-core/test/runner-test.js @@ -112,6 +112,21 @@ describe('Runner', () => { } }); + it('does not include a top-level runtimeError when gatherers were successful', async () => { + const config = new Config({ + settings: { + auditMode: __dirname + '/fixtures/artifacts/perflog/', + + }, + audits: [ + 'content-width', + ], + }); + + const {lhr} = await Runner.run(undefined, {config}); + assert.strictEqual(lhr.runtimeError, undefined); + }); + it('-GA is a normal run but it saves artifacts to disk', () => { const settings = {auditMode: artifactsPath, gatherMode: artifactsPath}; const opts = {url, config: generateConfig(settings), driverMock}; From 1403a0220f27db80efe6edf9a88e78e4337786ed Mon Sep 17 00:00:00 2001 From: Shane Exterkamp Date: Mon, 4 Mar 2019 14:38:19 -0800 Subject: [PATCH 5/5] remove status code checks. --- lighthouse-cli/test/cli/index-test.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/lighthouse-cli/test/cli/index-test.js b/lighthouse-cli/test/cli/index-test.js index 574781a45fac..11d31388b313 100644 --- a/lighthouse-cli/test/cli/index-test.js +++ b/lighthouse-cli/test/cli/index-test.js @@ -75,7 +75,6 @@ describe('CLI Tests', function() { const config = JSON.parse(ret.stdout); assert.strictEqual(config.settings.output[0], 'html'); assert.strictEqual(config.settings.auditMode, false); - assert.equal(ret.status, 0); expect(config).toMatchSnapshot(); }); @@ -92,7 +91,6 @@ describe('CLI Tests', function() { assert.strictEqual(config.settings.output[0], 'json'); assert.strictEqual(config.settings.auditMode, true); assert.strictEqual(config.audits.length, 1); - assert.equal(ret.status, 0); expect(config).toMatchSnapshot(); });