-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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): expose environment info #5871
Changes from 3 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -374,7 +374,9 @@ class GatherRunner { | |
return { | ||
fetchTime: (new Date()).toJSON(), | ||
LighthouseRunWarnings: [], | ||
UserAgent: await options.driver.getUserAgent(), | ||
HostUserAgent: await options.driver.getUserAgent(), | ||
NetworkUserAgent: '', // updated later | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sg, I also tried There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah. I still have nothings, so that's fine with me. If @paulirish has ideas we can do a follow up PR quickly before we ship it :) |
||
BenchmarkIndex: 0, // updated later | ||
traces: {}, | ||
devtoolsLogs: {}, | ||
settings: options.settings, | ||
|
@@ -397,6 +399,7 @@ class GatherRunner { | |
await driver.connect(); | ||
const baseArtifacts = await GatherRunner.getBaseArtifacts(options); | ||
await GatherRunner.loadBlank(driver); | ||
baseArtifacts.BenchmarkIndex = await options.driver.getBenchmarkIndex(); | ||
await GatherRunner.setupDriver(driver, options); | ||
|
||
// Run each pass | ||
|
@@ -420,6 +423,16 @@ class GatherRunner { | |
// Save devtoolsLog, but networkRecords are discarded and not added onto artifacts. | ||
baseArtifacts.devtoolsLogs[passConfig.passName] = passData.devtoolsLog; | ||
|
||
const userAgentEntry = passData.devtoolsLog.find(entry => | ||
entry.method === 'Network.requestWillBeSent' && | ||
!!entry.params.request.headers['User-Agent'] | ||
); | ||
|
||
if (userAgentEntry && !baseArtifacts.NetworkUserAgent) { | ||
// @ts-ignore - guaranteed to exist by the find above | ||
baseArtifacts.NetworkUserAgent = userAgentEntry.params.request.headers['User-Agent']; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. could also do There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Given the number of ways that have popped up in issues as the way folks are trying to set the user agent string, I thought it was best to grab it directly from network records instead of assume it's done being manipulated after setupDriver There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. SG. I guess |
||
|
||
// If requested by config, save pass's trace. | ||
if (passData.trace) { | ||
baseArtifacts.traces[passConfig.passName] = passData.trace; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -57,7 +57,6 @@ function registerPerformanceObserverInPage() { | |
window.____lhPerformanceObserver = observer; | ||
} | ||
|
||
|
||
/** | ||
* Used by _waitForCPUIdle and executed in the context of the page, returns time since last long task. | ||
*/ | ||
|
@@ -114,7 +113,35 @@ function getElementsInDocument(selector) { | |
function getOuterHTMLSnippet(element) { | ||
const reOpeningTag = /^.*?>/; | ||
const match = element.outerHTML.match(reOpeningTag); | ||
return match && match[0] || ''; | ||
return (match && match[0]) || ''; | ||
} | ||
|
||
/** | ||
* Computes a memory/CPU performance benchmark index to determine rough device class. | ||
* @see https://docs.google.com/spreadsheets/d/1E0gZwKsxegudkjJl8Fki_sOwHKpqgXwt8aBAfuUaB8A/edit?usp=sharing | ||
* | ||
* The benchmark creates a string of length 100,000 in a loop. | ||
* The returned index is the number of times per second the string can be created. | ||
* | ||
* - 750+ is a desktop-class device, Core i3 PC, iPhone X, etc | ||
* - 300+ is a high-end Android phone, Galaxy S8, low-end Chromebook, etc | ||
* - 75+ is a mid-tier Android phone, Nexus 5X, etc | ||
* - <75 is a budget Android phone, Alcatel Ideal, Galaxy J2, etc | ||
*/ | ||
/* istanbul ignore next */ | ||
function ultradumbBenchmark() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I know we just had a whole thing with page-functions, but I kind of wish this was in its own file for easier running :) I guess we can always add There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
const start = Date.now(); | ||
let iterations = 0; | ||
|
||
while (Date.now() - start < 500) { | ||
let s = ''; // eslint-disable-line no-unused-vars | ||
for (let j = 0; j < 100000; j++) s += 'a'; | ||
|
||
iterations++; | ||
} | ||
|
||
const durationInSeconds = (Date.now() - start) / 1000; | ||
return iterations / durationInSeconds; | ||
} | ||
|
||
module.exports = { | ||
|
@@ -123,4 +150,5 @@ module.exports = { | |
checkTimeSinceLastLongTask, | ||
getElementsInDocument, | ||
getOuterHTMLSnippet, | ||
ultradumbBenchmark, | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
#!/usr/bin/env node | ||
|
||
/** | ||
* @license Copyright 2018 Google Inc. All Rights Reserved. | ||
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 | ||
* Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. | ||
*/ | ||
'use strict'; | ||
|
||
/* eslint-disable no-console */ | ||
|
||
const ultradumbBenchmark = require('../lib/page-functions').ultradumbBenchmark; | ||
|
||
console.log('Running ULTRADUMB™ benchmark 10 times...') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. lint says you need a |
||
|
||
let total = 0; | ||
for (let i = 0; i < 10; i++) { | ||
const result = ultradumbBenchmark(); | ||
console.log(`Result ${i + 1}: ${result.toFixed(0)}`); | ||
total += result; | ||
} | ||
|
||
console.log('----------------------------------------'); | ||
console.log('Final result:', (total / 10).toFixed(0)); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -106,16 +106,37 @@ describe('GatherRunner', function() { | |
}); | ||
}); | ||
|
||
it('collects user agent as an artifact', () => { | ||
it('collects benchmark as an artifact', async () => { | ||
const url = 'https://example.com'; | ||
const driver = fakeDriver; | ||
const config = new Config({}); | ||
const settings = {}; | ||
const options = {url, driver, config, settings}; | ||
|
||
return GatherRunner.run([], options).then(results => { | ||
assert.equal(results.UserAgent, 'Fake user agent', 'did not find expected user agent string'); | ||
}); | ||
const results = await GatherRunner.run([], options); | ||
expect(Number.isFinite(results.BenchmarkIndex)).toBeTruthy(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I feel like you must be trolling here... |
||
}); | ||
|
||
it('collects host user agent as an artifact', async () => { | ||
const url = 'https://example.com'; | ||
const driver = fakeDriver; | ||
const config = new Config({}); | ||
const settings = {}; | ||
const options = {url, driver, config, settings}; | ||
|
||
const results = await GatherRunner.run([], options); | ||
expect(results.HostUserAgent).toEqual('Fake user agent'); | ||
}); | ||
|
||
it('collects network user agent as an artifact', async () => { | ||
const url = 'https://example.com'; | ||
const driver = fakeDriver; | ||
const config = new Config({passes: [{}]}); | ||
const settings = {}; | ||
const options = {url, driver, config, settings}; | ||
|
||
const results = await GatherRunner.run(config.passes, options); | ||
expect(results.NetworkUserAgent).toContain('Mozilla'); | ||
}); | ||
|
||
it('collects requested and final URLs as an artifact', () => { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,6 +16,15 @@ declare global { | |
[varName: string]: string; | ||
} | ||
|
||
export interface Environment { | ||
/** The user agent string of the version of Chrome used. */ | ||
hostUserAgent: string; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can you copy the above doc strings down for these too? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
/** The user agent string that was sent over the network. */ | ||
networkUserAgent: string; | ||
/** The benchmark index number that indicates rough device class. */ | ||
benchmarkIndex: number; | ||
} | ||
|
||
/** | ||
* The full output of a Lighthouse run. | ||
*/ | ||
|
@@ -43,6 +52,8 @@ declare global { | |
runWarnings: 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. */ | ||
environment: Environment; | ||
/** Execution timings for the Lighthouse run */ | ||
timing: {total: number, [t: string]: number}; | ||
/** The record of all formatted string locations in the LHR and their corresponding source values. */ | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will you add a simple docstring here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yep, done