-
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: update FCP score curve #12556
core: update FCP score curve #12556
Changes from 2 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 |
---|---|---|
|
@@ -29,12 +29,11 @@ class FirstContentfulPaint3G extends Audit { | |
*/ | ||
static get defaultOptions() { | ||
return { | ||
// 25th and 5th percentiles HTTPArchive on Fast 3G -> multiply by 1.5 for RTT differential -> median and PODR | ||
// p10 is then derived from them. | ||
// 25th and 8th percentiles HTTPArchive on Slow 4G -> multiply by 1.5 for RTT differential -> median and p10. | ||
// https://bigquery.cloud.google.com/table/httparchive:lighthouse.2018_04_01_mobile?pli=1 | ||
// https://www.desmos.com/calculator/fflcrsn9sj | ||
p10: 3504, | ||
median: 6000, | ||
// https://www.desmos.com/calculator/xi5oympawp | ||
p10: 2700, | ||
median: 4500, | ||
Comment on lines
+35
to
+36
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. these are updated based on the 1.5x multiplier mentioned above applied to the updated control points |
||
}; | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,14 +4,17 @@ | |
* 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'; | ||
const Audit = require('../../../audits/metrics/first-contentful-paint.js'); | ||
const FcpAudit = require('../../../audits/metrics/first-contentful-paint.js'); | ||
const assert = require('assert').strict; | ||
const options = Audit.defaultOptions; | ||
const options = FcpAudit.defaultOptions; | ||
const constants = require('../../../config/constants.js'); | ||
|
||
const pwaTrace = require('../../fixtures/traces/progressive-app-m60.json'); | ||
const pwaDevtoolsLog = require('../../fixtures/traces/progressive-app-m60.devtools.log.json'); | ||
|
||
const frameTrace = require('../../fixtures/traces/frame-metrics-m90.json'); | ||
const frameDevtoolsLog = require('../../fixtures/traces/frame-metrics-m90.devtools.log.json'); | ||
|
||
/** | ||
* @param {{ | ||
* {LH.SharedFlagsSettings['formFactor']} formFactor | ||
|
@@ -35,16 +38,32 @@ describe('Performance: first-contentful-paint audit', () => { | |
it('evaluates valid input correctly', async () => { | ||
const artifacts = { | ||
traces: { | ||
[Audit.DEFAULT_PASS]: pwaTrace, | ||
[FcpAudit.DEFAULT_PASS]: pwaTrace, | ||
}, | ||
devtoolsLogs: { | ||
[Audit.DEFAULT_PASS]: pwaDevtoolsLog, | ||
[FcpAudit.DEFAULT_PASS]: pwaDevtoolsLog, | ||
}, | ||
}; | ||
|
||
const context = getFakeContext({formFactor: 'mobile', throttlingMethod: 'provided'}); | ||
const result = await Audit.audit(artifacts, context); | ||
const result = await FcpAudit.audit(artifacts, context); | ||
assert.equal(result.score, 1); | ||
assert.equal(result.numericValue, 498.87); | ||
}); | ||
|
||
it('evaluates a modern trace correctly', async () => { | ||
const artifacts = { | ||
traces: { | ||
[FcpAudit.DEFAULT_PASS]: frameTrace, | ||
}, | ||
devtoolsLogs: { | ||
[FcpAudit.DEFAULT_PASS]: frameDevtoolsLog, | ||
}, | ||
}; | ||
|
||
const context = getFakeContext({formFactor: 'mobile', throttlingMethod: 'provided'}); | ||
const result = await FcpAudit.audit(artifacts, context); | ||
assert.equal(result.score, 0.06); | ||
assert.equal(result.numericValue, 5668.275); | ||
}); | ||
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. we didn't have any unit test that needed updating from the score change (aside from sample_v2 snapshot ones), which is always suspicious, so added one 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.
isnt the dataset different?
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.
ah yeah, good catch