Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

core(lhr): s/initialUrl/requestedUrl, s/url/finalUrl #5127

Merged
merged 4 commits into from
May 6, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions docs/understanding-results.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ The top-level Lighthouse Result object (LHR) is what the lighthouse node module
| lighthouseVersion | The version of Lighthouse with which this result were generated. |
| fetchTime | The ISO-8601 timestamp of when the result was generated. |
| userAgent | The user agent string of the version of Chrome that was used by Lighthouse. |
| initialUrl | The URL that was supplied to Lighthouse and initially navigated to. |
| url | The URL that Lighthouse ended up auditing after redirects were followed. |
| requestedUrl | The URL that was supplied to Lighthouse and initially navigated to. |
| finalUrl | The URL that Lighthouse ended up auditing after redirects were followed. |
| [audits](#audits) | An object containing the results of the audits. |
| [runtimeConfig](#runtime-config) | An object containing information about the configuration used by Lighthouse. |
| [timing](#timing) | An object containing information about how long Lighthouse spent auditing. |
Expand All @@ -28,8 +28,8 @@ The top-level Lighthouse Result object (LHR) is what the lighthouse node module
"lighthouseVersion": "2.4.0",
"fetchTime": "2017-10-05T20:50:54.185Z",
"userAgent": "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/63.0.3233.0 Safari/537.36",
"initialUrl": "http://example.com",
"url": "https://www.example.com/",
"requestedUrl": "http://example.com",
"finalUrl": "https://www.example.com/",
"score": 50,
"audits": {...},
"runtimeConfig": {...},
Expand Down
4 changes: 2 additions & 2 deletions lighthouse-cli/test/smokehouse/a11y/expectations.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@
*/
module.exports = [
{
initialUrl: 'http://localhost:10200/a11y/a11y_tester.html',
url: 'http://localhost:10200/a11y/a11y_tester.html',
requestedUrl: 'http://localhost:10200/a11y/a11y_tester.html',
finalUrl: 'http://localhost:10200/a11y/a11y_tester.html',
audits: {
'accesskeys': {
score: 0,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@
*/
module.exports = [
{
initialUrl: 'http://localhost:10200/byte-efficiency/tester.html',
url: 'http://localhost:10200/byte-efficiency/tester.html',
requestedUrl: 'http://localhost:10200/byte-efficiency/tester.html',
finalUrl: 'http://localhost:10200/byte-efficiency/tester.html',
audits: {
'unminified-css': {
extendedInfo: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@
*/
module.exports = [
{
initialUrl: 'http://localhost:10200/dobetterweb/dbw_tester.html',
url: 'http://localhost:10200/dobetterweb/dbw_tester.html',
requestedUrl: 'http://localhost:10200/dobetterweb/dbw_tester.html',
finalUrl: 'http://localhost:10200/dobetterweb/dbw_tester.html',
audits: {
'errors-in-console': {
score: 0,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@
*/
module.exports = [
{
initialUrl: 'http://localhost:10200/online-only.html',
url: 'http://localhost:10200/online-only.html',
requestedUrl: 'http://localhost:10200/online-only.html',
finalUrl: 'http://localhost:10200/online-only.html',
audits: {
'is-on-https': {
score: 1,
Expand Down Expand Up @@ -105,8 +105,8 @@ module.exports = [
},

{
initialUrl: 'http://localhost:10503/offline-ready.html',
url: 'http://localhost:10503/offline-ready.html',
requestedUrl: 'http://localhost:10503/offline-ready.html',
finalUrl: 'http://localhost:10503/offline-ready.html',
audits: {
'is-on-https': {
score: 1,
Expand Down
8 changes: 4 additions & 4 deletions lighthouse-cli/test/smokehouse/perf/expectations.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@
*/
module.exports = [
{
initialUrl: 'http://localhost:10200/preload.html',
url: 'http://localhost:10200/preload.html',
requestedUrl: 'http://localhost:10200/preload.html',
finalUrl: 'http://localhost:10200/preload.html',
audits: {
'speed-index': {
score: '>=0.80',
Expand Down Expand Up @@ -56,8 +56,8 @@ module.exports = [
},
},
{
initialUrl: 'http://localhost:10200/perf/fonts.html',
url: 'http://localhost:10200/perf/fonts.html',
requestedUrl: 'http://localhost:10200/perf/fonts.html',
finalUrl: 'http://localhost:10200/perf/fonts.html',
audits: {
'font-display': {
score: 0,
Expand Down
8 changes: 4 additions & 4 deletions lighthouse-cli/test/smokehouse/pwa-expectations.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@
*/
module.exports = [
{
initialUrl: 'https://airhorner.com',
url: 'https://airhorner.com/',
requestedUrl: 'https://airhorner.com',
finalUrl: 'https://airhorner.com/',
audits: {
'is-on-https': {
score: 1,
Expand Down Expand Up @@ -116,8 +116,8 @@ module.exports = [
},

{
initialUrl: 'https://www.chromestatus.com/',
url: 'https://www.chromestatus.com/features',
requestedUrl: 'https://www.chromestatus.com/',
finalUrl: 'https://www.chromestatus.com/features',
audits: {
'is-on-https': {
score: 1,
Expand Down
8 changes: 4 additions & 4 deletions lighthouse-cli/test/smokehouse/pwa2-expectations.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@
*/
module.exports = [
{
initialUrl: 'https://jakearchibald.github.io/svgomg/',
url: 'https://jakearchibald.github.io/svgomg/',
requestedUrl: 'https://jakearchibald.github.io/svgomg/',
finalUrl: 'https://jakearchibald.github.io/svgomg/',
audits: {
'is-on-https': {
score: 1,
Expand Down Expand Up @@ -118,8 +118,8 @@ module.exports = [
},

{
initialUrl: 'https://shop.polymer-project.org/',
url: 'https://shop.polymer-project.org/',
requestedUrl: 'https://shop.polymer-project.org/',
finalUrl: 'https://shop.polymer-project.org/',
audits: {
'is-on-https': {
score: 1,
Expand Down
4 changes: 2 additions & 2 deletions lighthouse-cli/test/smokehouse/pwa3-expectations.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@
*/
module.exports = [
{
initialUrl: 'https://pwa.rocks',
url: 'https://pwa.rocks/',
requestedUrl: 'https://pwa.rocks',
finalUrl: 'https://pwa.rocks/',
audits: {
'is-on-https': {
score: 1,
Expand Down
8 changes: 4 additions & 4 deletions lighthouse-cli/test/smokehouse/redirects/expectations.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ const cacheBuster = Number(new Date());

module.exports = [
{
initialUrl: `http://localhost:10200/online-only.html?delay=500&redirect=%2Foffline-only.html%3Fcb=${cacheBuster}%26delay=500%26redirect%3D%2Fredirects-final.html`,
url: 'http://localhost:10200/redirects-final.html',
requestedUrl: `http://localhost:10200/online-only.html?delay=500&redirect=%2Foffline-only.html%3Fcb=${cacheBuster}%26delay=500%26redirect%3D%2Fredirects-final.html`,
finalUrl: 'http://localhost:10200/redirects-final.html',
audits: {
'redirects': {
score: '<1',
Expand All @@ -27,8 +27,8 @@ module.exports = [
},
},
{
initialUrl: `http://localhost:10200/online-only.html?delay=300&redirect=%2Fredirects-final.html`,
url: 'http://localhost:10200/redirects-final.html',
requestedUrl: `http://localhost:10200/online-only.html?delay=300&redirect=%2Fredirects-final.html`,
finalUrl: 'http://localhost:10200/redirects-final.html',
audits: {
'redirects': {
score: 1,
Expand Down
8 changes: 4 additions & 4 deletions lighthouse-cli/test/smokehouse/seo/expectations.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ const passHeaders = headersParam([[
*/
module.exports = [
{
initialUrl: BASE_URL + 'seo-tester.html?' + passHeaders,
url: BASE_URL + 'seo-tester.html?' + passHeaders,
requestedUrl: BASE_URL + 'seo-tester.html?' + passHeaders,
finalUrl: BASE_URL + 'seo-tester.html?' + passHeaders,
audits: {
'viewport': {
score: 1,
Expand Down Expand Up @@ -78,8 +78,8 @@ module.exports = [
},
},
{
initialUrl: BASE_URL + 'seo-failure-cases.html?status_code=403&' + failureHeaders,
url: BASE_URL + 'seo-failure-cases.html?status_code=403&' + failureHeaders,
requestedUrl: BASE_URL + 'seo-failure-cases.html?status_code=403&' + failureHeaders,
finalUrl: BASE_URL + 'seo-failure-cases.html?status_code=403&' + failureHeaders,
audits: {
'viewport': {
score: 0,
Expand Down
16 changes: 8 additions & 8 deletions lighthouse-cli/test/smokehouse/smokehouse.js
Original file line number Diff line number Diff line change
Expand Up @@ -194,8 +194,8 @@ function findDifference(path, actual, expected) {

/**
* Collate results into comparisons of actual and expected scores on each audit.
* @param {{url: string, audits: !Array}} actual
* @param {{url: string, audits: !Array}} expected
* @param {{finalUrl: string, audits: !Array}} actual
* @param {{finalUrl: string, audits: !Array}} expected
* @return {{finalUrl: !Object, audits: !Array<!Object>}}
*/
function collateResults(actual, expected) {
Expand All @@ -221,9 +221,9 @@ function collateResults(actual, expected) {
return {
finalUrl: {
category: 'final url',
actual: actual.url,
expected: expected.url,
equal: actual.url === expected.url,
actual: actual.finalUrl,
expected: expected.finalUrl,
equal: actual.finalUrl === expected.finalUrl,
},
audits: collatedAudits,
};
Expand Down Expand Up @@ -316,10 +316,10 @@ const expectations = require(resolveLocalOrCwd(cli['expectations-path']));
let passingCount = 0;
let failingCount = 0;
expectations.forEach(expected => {
console.log(`Doing a run of '${expected.initialUrl}'...`);
const results = runLighthouse(expected.initialUrl, configPath, cli.debug);
console.log(`Doing a run of '${expected.requestedUrl}'...`);
const results = runLighthouse(expected.requestedUrl, configPath, cli.debug);

console.log(`Asserting expected results match those found. (${expected.initialUrl})`);
console.log(`Asserting expected results match those found. (${expected.requestedUrl})`);
const collated = collateResults(results, expected);
const counts = report(collated);
passingCount += counts.passed;
Expand Down
4 changes: 2 additions & 2 deletions lighthouse-cli/test/smokehouse/tricky-tti/expectations.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@
*/
module.exports = [
{
initialUrl: 'http://localhost:10200/tricky-tti.html',
url: 'http://localhost:10200/tricky-tti.html',
requestedUrl: 'http://localhost:10200/tricky-tti.html',
finalUrl: 'http://localhost:10200/tricky-tti.html',
audits: {
'first-cpu-idle': {
score: '<75',
Expand Down
4 changes: 2 additions & 2 deletions lighthouse-core/audits/works-offline.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,9 @@ class WorksOffline extends Audit {
let debugString;
const passed = artifacts.Offline === 200;
if (!passed &&
!URL.equalWithExcludedFragments(artifacts.URL.initialUrl, artifacts.URL.finalUrl)) {
!URL.equalWithExcludedFragments(artifacts.URL.requestedUrl, artifacts.URL.finalUrl)) {
debugString = 'WARNING: You may be failing this check because your test URL ' +
`(${artifacts.URL.initialUrl}) was redirected to "${artifacts.URL.finalUrl}". ` +
`(${artifacts.URL.requestedUrl}) was redirected to "${artifacts.URL.finalUrl}". ` +
'Try testing the second URL directly.';
}

Expand Down
1 change: 0 additions & 1 deletion lighthouse-core/config/default-config.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ module.exports = {
networkQuietThresholdMs: 1000,
cpuQuietThresholdMs: 1000,
gatherers: [
'url',
'scripts',
'css-usage',
'viewport',
Expand Down
29 changes: 17 additions & 12 deletions lighthouse-core/gather/gather-runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ class GatherRunner {
/**
* Loads options.url with specified options. If the main document URL
* redirects, options.url will be updated accordingly. As such, options.url
* will always represent the post-redirected URL. options.initialUrl is the
* will always represent the post-redirected URL. options.requestedUrl is the
* pre-redirect starting URL.
* @param {Driver} driver
* @param {LH.Gatherer.PassContext} passContext
Expand All @@ -99,14 +99,14 @@ class GatherRunner {
/**
* @param {Driver} driver
* @param {GathererResults} gathererResults
* @param {{url: string, settings: LH.Config.Settings}} options
* @param {{requestedUrl: string, settings: LH.Config.Settings}} options
* @return {Promise<void>}
*/
static setupDriver(driver, gathererResults, options) {
log.log('status', 'Initializing…');
const resetStorage = !options.settings.disableStorageReset;
// Enable emulation based on settings
return driver.assertNoSameOriginServiceWorkerClients(options.url)
return driver.assertNoSameOriginServiceWorkerClients(options.requestedUrl)
.then(_ => driver.getUserAgent())
.then(userAgent => {
gathererResults.UserAgent = [userAgent];
Expand All @@ -118,7 +118,7 @@ class GatherRunner {
.then(_ => driver.registerPerformanceObserver())
.then(_ => driver.dismissJavaScriptDialogs())
.then(_ => {
if (resetStorage) return driver.clearDataForOrigin(options.url);
if (resetStorage) return driver.clearDataForOrigin(options.requestedUrl);
});
}

Expand Down Expand Up @@ -399,7 +399,7 @@ class GatherRunner {

/**
* @param {Array<LH.Config.Pass>} passes
* @param {{driver: Driver, url: string, settings: LH.Config.Settings}} options
* @param {{driver: Driver, requestedUrl: string, settings: LH.Config.Settings}} options
* @return {Promise<LH.Artifacts>}
*/
static run(passes, options) {
Expand All @@ -414,6 +414,7 @@ class GatherRunner {
const gathererResults = {
LighthouseRunWarnings: [],
fetchTime: [(new Date()).toJSON()],
URL: [{requestedUrl: options.requestedUrl, finalUrl: ''}],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh there it is.

};

return driver.connect()
Expand All @@ -422,10 +423,15 @@ class GatherRunner {

// Run each pass
.then(_ => {
// If the main document redirects, we'll update this to keep track
let urlAfterRedirects = options.url;
return passes.reduce((chain, passConfig, passIndex) => {
const passContext = Object.assign({}, options, {passConfig});
const passContext = {
driver: options.driver,
// If the main document redirects, we'll update this to keep track
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ace. thx for leaving this.

url: options.requestedUrl,
settings: options.settings,
passConfig,
};

return chain
.then(_ => driver.setThrottling(options.settings, passConfig))
.then(_ => GatherRunner.beforePass(passContext, gathererResults))
Expand All @@ -441,12 +447,11 @@ class GatherRunner {
}

if (passIndex === 0) {
urlAfterRedirects = passContext.url;
// Copy redirected URL to artifact in the first pass only.
gathererResults.URL[0].finalUrl = passContext.url;
}
});
}, Promise.resolve()).then(_ => {
options.url = urlAfterRedirects;
});
}, Promise.resolve());
})
.then(_ => GatherRunner.disposeDriver(driver))
.then(_ => GatherRunner.collectArtifacts(gathererResults, tracingData, options.settings))
Expand Down
26 changes: 0 additions & 26 deletions lighthouse-core/gather/gatherers/url.js

This file was deleted.

4 changes: 2 additions & 2 deletions lighthouse-core/lib/file-namer.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,11 @@
* Generate a filenamePrefix of hostname_YYYY-MM-DD_HH-MM-SS
* Date/time uses the local timezone, however Node has unreliable ICU
* support, so we must construct a YYYY-MM-DD date format manually. :/
* @param {{url: string, fetchTime: string}} lhr
* @param {{finalUrl: string, fetchTime: string}} lhr
* @return {string}
*/
function getFilenamePrefix(lhr) {
const hostname = new (getUrlConstructor())(lhr.url).hostname;
const hostname = new (getUrlConstructor())(lhr.finalUrl).hostname;
const date = (lhr.fetchTime && new Date(lhr.fetchTime)) || new Date();

const timeStr = date.toLocaleTimeString('en-US', {hour12: false});
Expand Down
Loading