Skip to content

Commit

Permalink
core(audit-mode): do not require a URL (#5495)
Browse files Browse the repository at this point in the history
  • Loading branch information
@patrickhulce
patrickhulce committed Jun 13, 2018
1 parent b339844 commit deaf607
Show file tree
Hide file tree
Showing 6 changed files with 52 additions and 48 deletions.
10 changes: 7 additions & 3 deletions lighthouse-cli/cli-flags.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,9 +136,13 @@ function getFlags(manualArgv) {
.default('port', 0)
.default('hostname', 'localhost')
.check(/** @param {!LH.Flags} argv */ (argv) => {
// Make sure lighthouse has been passed a url, or at least one of --list-all-audits
// or --list-trace-categories. If not, stop the program and ask for a url
if (!argv.listAllAudits && !argv.listTraceCategories && argv._.length === 0) {
// Lighthouse doesn't need a URL if...
// - We're in auditMode (and we have artifacts already)
// - We're just listing the available options.
// If one of these don't apply, stop the program and ask for a url.
const isListMode = argv.listAllAudits || argv.listTraceCategories;
const isOnlyAuditMode = !!argv.auditMode && !argv.gatherMode;
if (!isListMode && !isOnlyAuditMode && argv._.length === 0) {
throw new Error('Please provide a url');
}

Expand Down
44 changes: 21 additions & 23 deletions lighthouse-core/runner.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ const Connection = require('./gather/connections/connection.js'); // eslint-disa
class Runner {
/**
* @param {Connection} connection
* @param {{config: LH.Config, url: string, driverMock?: Driver}} opts
* @param {{config: LH.Config, url?: string, driverMock?: Driver}} opts
* @return {Promise<LH.RunnerResult|undefined>}
*/
static async run(connection, opts) {
Expand All @@ -37,19 +37,6 @@ class Runner {
*/
const lighthouseRunWarnings = [];

// save the requestedUrl provided by the user
const rawRequestedUrl = opts.url;
if (typeof rawRequestedUrl !== 'string' || rawRequestedUrl.length === 0) {
throw new Error(`You must provide a url to the runner. '${rawRequestedUrl}' provided.`);
}

let parsedURL;
try {
parsedURL = new URL(opts.url);
} catch (e) {
throw new Error('The url provided should have a proper protocol and hostname.');
}

const sentryContext = Sentry.getContext();
// @ts-ignore TODO(bckenny): Sentry type checking
Sentry.captureBreadcrumb({
Expand All @@ -59,27 +46,38 @@ class Runner {
data: sentryContext && sentryContext.extra,
});

// If the URL isn't https and is also not localhost complain to the user.
if (parsedURL.protocol !== 'https:' && parsedURL.hostname !== 'localhost') {
log.warn('Lighthouse', 'The URL provided should be on HTTPS');
log.warn('Lighthouse', 'Performance stats will be skewed redirecting from HTTP to HTTPS.');
}

// canonicalize URL with any trailing slashes neccessary
const requestedUrl = parsedURL.href;

// User can run -G solo, -A solo, or -GA together
// -G and -A will run partial lighthouse pipelines,
// and -GA will run everything plus save artifacts to disk

// Gather phase
// Either load saved artifacts off disk or from the browser
let artifacts;
let requestedUrl;
if (settings.auditMode && !settings.gatherMode) {
// No browser required, just load the artifacts from disk.
const path = Runner._getArtifactsPath(settings);
artifacts = await assetSaver.loadArtifacts(path);
requestedUrl = artifacts.URL.requestedUrl;

if (!requestedUrl) {
throw new Error('Cannot run audit mode on empty URL');
}
if (opts.url && opts.url !== requestedUrl) {
throw new Error('Cannot run audit mode on different URL');
}
} else {
if (typeof opts.url !== 'string' || opts.url.length === 0) {
throw new Error(`You must provide a url to the runner. '${opts.url}' provided.`);
}

try {
// Use canonicalized URL (with trailing slashes and such)
requestedUrl = new URL(opts.url).href;
} catch (e) {
throw new Error('The url provided should have a proper protocol and hostname.');
}

artifacts = await Runner._gatherArtifactsFromBrowser(requestedUrl, opts, connection);
// -G means save these to ./latest-run, etc.
if (settings.gatherMode) {
Expand Down
2 changes: 1 addition & 1 deletion lighthouse-core/scripts/assert-golden-lhr-unchanged.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ trap teardown EXIT
colorText "Generating a fresh LHR..." "$purple"
set -x
# TODO(phulce): add a lantern LHR-differ
node "$lhroot_path/lighthouse-cli" -A="$lhroot_path/lighthouse-core/test/results/artifacts" --throttling-method=devtools --quiet --output=json --output-path="$freshLHRPath" http://localhost/dobetterweb/dbw_tester.html
node "$lhroot_path/lighthouse-cli" -A="$lhroot_path/lighthouse-core/test/results/artifacts" --throttling-method=devtools --quiet --output=json --output-path="$freshLHRPath"
set +x

# remove timing from both
Expand Down
2 changes: 1 addition & 1 deletion lighthouse-core/test/results/sample_v2.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"userAgent": "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_13_3) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/66.0.3358.0 Safari/537.36",
"lighthouseVersion": "3.0.0-beta.0",
"fetchTime": "2018-03-13T00:55:45.840Z",
"requestedUrl": "http://localhost/dobetterweb/dbw_tester.html",
"requestedUrl": "http://localhost:10200/dobetterweb/dbw_tester.html",
"finalUrl": "http://localhost:10200/dobetterweb/dbw_tester.html",
"runWarnings": [],
"audits": {
Expand Down
40 changes: 21 additions & 19 deletions lighthouse-core/test/runner-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ describe('Runner', () => {

// uses the files on disk from the -G test. ;)
it('-A audits from saved artifacts and doesn\'t gather', () => {
const opts = {url, config: generateConfig({auditMode: artifactsPath}), driverMock};
const opts = {config: generateConfig({auditMode: artifactsPath}), driverMock};
return Runner.run(null, opts).then(_ => {
assert.equal(loadArtifactsSpy.called, true, 'loadArtifacts was not called');
assert.equal(gatherRunnerRunSpy.called, false, 'GatherRunner.run was called');
Expand All @@ -83,7 +83,7 @@ describe('Runner', () => {

it('-A throws if the settings change', async () => {
const settings = {auditMode: artifactsPath, disableDeviceEmulation: true};
const opts = {url, config: generateConfig(settings), driverMock};
const opts = {config: generateConfig(settings), driverMock};
try {
await Runner.run(null, opts);
assert.fail('should have thrown');
Expand All @@ -92,6 +92,17 @@ describe('Runner', () => {
}
});

it('-A throws if the URL changes', async () => {
const settings = {auditMode: artifactsPath, disableDeviceEmulation: true};
const opts = {url: 'https://example.com', config: generateConfig(settings), driverMock};
try {
await Runner.run(null, opts);
assert.fail('should have thrown');
} catch (err) {
assert.ok(/different URL/.test(err.message), 'should have prevented run');
}
});

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};
Expand Down Expand Up @@ -187,8 +198,6 @@ describe('Runner', () => {
});

it('accepts trace artifacts as paths and outputs appropriate data', () => {
const url = 'https://example.com/';

const config = new Config({
settings: {
auditMode: __dirname + '/fixtures/artifacts/perflog/',
Expand All @@ -198,7 +207,7 @@ describe('Runner', () => {
],
});

return Runner.run({}, {url, config}).then(results => {
return Runner.run({}, {config}).then(results => {
const audits = results.lhr.audits;
assert.equal(audits['user-timings'].displayValue[1], 2);
assert.equal(audits['user-timings'].rawValue, false);
Expand Down Expand Up @@ -233,7 +242,6 @@ describe('Runner', () => {

describe('Bad required artifact handling', () => {
it('outputs an error audit result when trace required but not provided', () => {
const url = 'https://example.com';
const config = new Config({
settings: {
auditMode: __dirname + '/fixtures/artifacts/empty-artifacts/',
Expand All @@ -244,7 +252,7 @@ describe('Runner', () => {
],
});

return Runner.run({}, {url, config}).then(results => {
return Runner.run({}, {config}).then(results => {
const auditResult = results.lhr.audits['user-timings'];
assert.strictEqual(auditResult.rawValue, null);
assert.strictEqual(auditResult.scoreDisplayMode, 'error');
Expand All @@ -253,7 +261,6 @@ describe('Runner', () => {
});

it('outputs an error audit result when missing a required artifact', () => {
const url = 'https://example.com';
const config = new Config({
settings: {
auditMode: __dirname + '/fixtures/artifacts/empty-artifacts/',
Expand All @@ -264,7 +271,7 @@ describe('Runner', () => {
],
});

return Runner.run({}, {url, config}).then(results => {
return Runner.run({}, {config}).then(results => {
const auditResult = results.lhr.audits['content-width'];
assert.strictEqual(auditResult.rawValue, null);
assert.strictEqual(auditResult.scoreDisplayMode, 'error');
Expand Down Expand Up @@ -312,7 +319,6 @@ describe('Runner', () => {

it('produces an error audit result when an audit throws a non-fatal Error', () => {
const errorMessage = 'Audit yourself';
const url = 'https://example.com';
const config = new Config({
settings: {
auditMode: __dirname + '/fixtures/artifacts/empty-artifacts/',
Expand All @@ -329,7 +335,7 @@ describe('Runner', () => {
],
});

return Runner.run({}, {url, config}).then(results => {
return Runner.run({}, {config}).then(results => {
const auditResult = results.lhr.audits['throwy-audit'];
assert.strictEqual(auditResult.rawValue, null);
assert.strictEqual(auditResult.scoreDisplayMode, 'error');
Expand All @@ -339,7 +345,6 @@ describe('Runner', () => {

it('rejects if an audit throws a fatal error', () => {
const errorMessage = 'Uh oh';
const url = 'https://example.com';
const config = new Config({
settings: {
auditMode: __dirname + '/fixtures/artifacts/empty-artifacts/',
Expand All @@ -358,14 +363,13 @@ describe('Runner', () => {
],
});

return Runner.run({}, {url, config}).then(
return Runner.run({}, {config}).then(
_ => assert.ok(false),
err => assert.strictEqual(err.message, errorMessage));
});
});

it('accepts devtoolsLog in artifacts', () => {
const url = 'https://example.com';
const config = new Config({
settings: {
auditMode: __dirname + '/fixtures/artifacts/perflog/',
Expand All @@ -375,7 +379,7 @@ describe('Runner', () => {
],
});

return Runner.run({}, {url, config}).then(results => {
return Runner.run({}, {config}).then(results => {
const audits = results.lhr.audits;
assert.equal(audits['critical-request-chains'].displayValue, '5 chains found');
assert.equal(audits['critical-request-chains'].rawValue, false);
Expand Down Expand Up @@ -487,7 +491,6 @@ describe('Runner', () => {
});

it('results include artifacts when given artifacts and audits', () => {
const url = 'https://example.com';
const config = new Config({
settings: {
auditMode: __dirname + '/fixtures/artifacts/perflog/',
Expand All @@ -497,7 +500,7 @@ describe('Runner', () => {
],
});

return Runner.run({}, {url, config}).then(results => {
return Runner.run({}, {config}).then(results => {
assert.strictEqual(results.artifacts.ViewportDimensions.innerWidth, 412);
assert.strictEqual(results.artifacts.ViewportDimensions.innerHeight, 732);
});
Expand Down Expand Up @@ -528,15 +531,14 @@ describe('Runner', () => {
});

it('includes any LighthouseRunWarnings from artifacts in output', () => {
const url = 'https://example.com';
const config = new Config({
settings: {
auditMode: __dirname + '/fixtures/artifacts/perflog/',
},
audits: [],
});

return Runner.run(null, {url, config, driverMock}).then(results => {
return Runner.run(null, {config, driverMock}).then(results => {
assert.deepStrictEqual(results.lhr.runWarnings, [
'I\'m a warning!',
'Also a warning',
Expand Down
2 changes: 1 addition & 1 deletion package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@
"changelog": "conventional-changelog --config ./build/changelog-generator/index.js --infile changelog.md --same-file",
"type-check": "tsc -p . && cd ./lighthouse-viewer && yarn type-check",
"update:sample-artifacts": "node lighthouse-core/scripts/update-report-fixtures.js -G",
"update:sample-json": "node ./lighthouse-cli -A=./lighthouse-core/test/results/artifacts --throttling-method=devtools --output=json --output-path=./lighthouse-core/test/results/sample_v2.json http://localhost/dobetterweb/dbw_tester.html && node lighthouse-core/scripts/cleanup-LHR-for-diff.js ./lighthouse-core/test/results/sample_v2.json --only-remove-timing",
"update:sample-json": "node ./lighthouse-cli -A=./lighthouse-core/test/results/artifacts --throttling-method=devtools --output=json --output-path=./lighthouse-core/test/results/sample_v2.json && node lighthouse-core/scripts/cleanup-LHR-for-diff.js ./lighthouse-core/test/results/sample_v2.json --only-remove-timing",
"diff:sample-json": "bash lighthouse-core/scripts/assert-golden-lhr-unchanged.sh",
"update:crdp-typings": "node lighthouse-core/scripts/extract-crdp-mapping.js",
"mixed-content": "./lighthouse-cli/index.js --chrome-flags='--headless' --config-path=./lighthouse-core/config/mixed-content.js",
Expand Down

0 comments on commit deaf607

Please sign in to comment.