core: replace remaining rawValue with numericValue #8421

brendankenny · 2019-04-19T01:03:33Z

part 3 of 3 after #8343 and #8385

Replaces rawValue with numericValue in the internal audit output (LH.Audit.Product). Went suspiciously easily :)

brendankenny · 2019-04-19T01:05:42Z

lighthouse-core/test/audits/metrics/first-contentful-paint-3g-test.js

@@ -26,7 +26,7 @@ describe('Performance: first-contentful-paint-3g audit', () => {

    const result = await FCP3G.audit(artifacts, {settings: {}, options, computedCache: new Map()});
    // Use InlineSnapshot here so changes to Lantern coefficients can be easily updated en masse


:roll_eyes: :P

oh puh-lease 😛 there's like 40 freaking values to manually and copy and paste when lantern changes and I've had to do it like 8 times. do you want to volunteer to manually fix every expectation every time this happens from here on out?

brendankenny · 2019-04-19T01:06:59Z

lighthouse-core/test/audits/load-fast-enough-for-pwa-test.js

@@ -75,8 +75,7 @@ describe('PWA: load-fast-enough-for-pwa audit', () => {

    const settings = {throttlingMethod: 'provided', throttling: {rttMs: 40, throughput: 100000}};
    const result = await FastPWAAudit.audit(artifacts, {settings, computedCache: new Map()});
-    expect(result.rawValue).toBeGreaterThan(2000); // If not overridden this would be 1582
-    expect(Math.round(result.rawValue)).toMatchSnapshot();


wasn't sure why this was this way, but just a single number, so removed

@brendankenny it's a lantern computed value it does it this way so that lantern changes can be updated with -u in jest

could you revert with a comment?

patrickhulce · 2019-04-19T01:46:00Z

lighthouse-core/test/audits/load-fast-enough-for-pwa-test.js

@@ -75,8 +75,7 @@ describe('PWA: load-fast-enough-for-pwa audit', () => {

    const settings = {throttlingMethod: 'provided', throttling: {rttMs: 40, throughput: 100000}};
    const result = await FastPWAAudit.audit(artifacts, {settings, computedCache: new Map()});
-    expect(result.rawValue).toBeGreaterThan(2000); // If not overridden this would be 1582
-    expect(Math.round(result.rawValue)).toMatchSnapshot();


@brendankenny it's a lantern computed value it does it this way so that lantern changes can be updated with -u in jest

patrickhulce · 2019-04-19T01:46:38Z

lighthouse-core/test/audits/load-fast-enough-for-pwa-test.js

@@ -75,8 +75,7 @@ describe('PWA: load-fast-enough-for-pwa audit', () => {

    const settings = {throttlingMethod: 'provided', throttling: {rttMs: 40, throughput: 100000}};
    const result = await FastPWAAudit.audit(artifacts, {settings, computedCache: new Map()});
-    expect(result.rawValue).toBeGreaterThan(2000); // If not overridden this would be 1582
-    expect(Math.round(result.rawValue)).toMatchSnapshot();


could you revert with a comment?

patrickhulce · 2019-04-19T01:49:45Z

lighthouse-core/test/audits/metrics/first-contentful-paint-3g-test.js

@@ -26,7 +26,7 @@ describe('Performance: first-contentful-paint-3g audit', () => {

    const result = await FCP3G.audit(artifacts, {settings: {}, options, computedCache: new Map()});
    // Use InlineSnapshot here so changes to Lantern coefficients can be easily updated en masse


oh puh-lease 😛 there's like 40 freaking values to manually and copy and paste when lantern changes and I've had to do it like 8 times. do you want to volunteer to manually fix every expectation every time this happens from here on out?

brendankenny · 2019-04-19T07:23:47Z

In memoriam:

The earliest reference to rawValue I can find is in #61 in a proposed audit output format, where you can see me worrying about tags (what were those going to be?) and the complexity of a possible emerging "audits of audits" architecture. We don't discuss the rawValue name, so it may have already been decided upon.

Also we were going to have auditResult.fault as the explanation of why the audit screwed up and couldn't handle your site, which is the modern day LH.Audit.Result['warnings']. I kind of like fault :)

The types landed a day later in the great Aggregator PR from @paullewis. At that point, audits had a value and a rawValue.

rawValue rapidly became the dumping ground for whatever seemed raw-ish. Finally, a few months
later, there's issue #458 from @paulirish suggesting we rename value to score, and describing rawValue as not exactly "raw" :) That issue birthed displayValue and gave us a refined rawValue of either a number or boolean [and eventually null], never a string that we kept around for some reason for another three years :)

connorjclark · 2019-04-25T17:48:16Z

docs/recipes/lighthouse-plugin-example/audits/preload-as.js

@@ -37,7 +37,7 @@ class PreloadAsAudit extends Audit {
    const passed = noAsLinks.length === 0;

    return {
-      rawValue: passed,
+      score: passed ? 1 : 0,


+passed? idk how hacky you'll allow js to be :)

I did something like Number(passed) for real audits, but since this is a recipe I thought I'd be more explicit. Not sure if this is actually more self explanatory, though :)

I like the explicitness of this for the recipe :)

brendankenny · 2019-04-25T19:23:42Z

@patrickhulce you're still requesting changes

patrickhulce · 2019-04-25T21:00:47Z

Oh my bad sorry thought I came back to this already :)

patrickhulce

LGTM

core: replace remaining rawValue with numericValue

ab69004

brendankenny requested review from patrickhulce and paulirish as code owners April 19, 2019 01:03

brendankenny commented Apr 19, 2019

View reviewed changes

patrickhulce suggested changes Apr 19, 2019

View reviewed changes

snapshot

0369994

brendankenny requested a review from exterkamp as a code owner April 19, 2019 06:46

vercel bot deployed to staging April 19, 2019 06:46 View deployment

paulirish approved these changes Apr 25, 2019

View reviewed changes

connorjclark reviewed Apr 25, 2019

View reviewed changes

patrickhulce approved these changes Apr 25, 2019

View reviewed changes

brendankenny merged commit d9e1a32 into master Apr 25, 2019

brendankenny deleted the rawvalue branch April 25, 2019 21:21

This was referenced Apr 30, 2019

[meta] 2019 I/O Burndown #8331

Closed

☂️ 👣 Breaking changes for v5 #7752

Closed

brendankenny mentioned this pull request May 22, 2019

Remove rawValue from Audit.Result #6199

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

core: replace remaining rawValue with numericValue #8421

core: replace remaining rawValue with numericValue #8421

brendankenny commented Apr 19, 2019

brendankenny Apr 19, 2019

patrickhulce Apr 19, 2019

brendankenny Apr 19, 2019

patrickhulce Apr 19, 2019

patrickhulce Apr 19, 2019

patrickhulce Apr 19, 2019

patrickhulce Apr 19, 2019

patrickhulce Apr 19, 2019

brendankenny commented Apr 19, 2019 •

edited

Loading

connorjclark Apr 25, 2019

brendankenny Apr 25, 2019

patrickhulce Apr 25, 2019

brendankenny commented Apr 25, 2019

patrickhulce commented Apr 25, 2019

patrickhulce left a comment

		@@ -26,7 +26,7 @@ describe('Performance: first-contentful-paint-3g audit', () => {

		const result = await FCP3G.audit(artifacts, {settings: {}, options, computedCache: new Map()});
		// Use InlineSnapshot here so changes to Lantern coefficients can be easily updated en masse

core: replace remaining rawValue with numericValue #8421

core: replace remaining rawValue with numericValue #8421

Conversation

brendankenny commented Apr 19, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brendankenny commented Apr 19, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brendankenny commented Apr 25, 2019

patrickhulce commented Apr 25, 2019

patrickhulce left a comment

Choose a reason for hiding this comment

brendankenny commented Apr 19, 2019 •

edited

Loading