core: require score in audit product; make rawValue numeric only #8343
Conversation
Only done a quick scan, but it's fair to say those two files are the only meat right? |
yeah, |
| @@ -389,7 +390,7 @@ describe('Runner', () => { | |||
| 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); | |||
| assert.equal(audits['critical-request-chains'].details.longestChain.transferSize, 2468); | |||
There was a problem hiding this comment.
oh, I guess not quite string replace in this file because it made no sense to assert the score on these. They're all null since the audit is INFORMATIVE
| return { | ||
| id: 'numeric-time', | ||
| title: 'Numbersssss', | ||
| description: '01000000001011011111100001010100', |
There was a problem hiding this comment.
@-�T?
c'mon gotta have some next level messaging in here :)
There was a problem hiding this comment.
01101101011001010110111101110111
There was a problem hiding this comment.
not everything's a character :P
new Float32Array(new Uint32Array([0b01000000001011011111100001010100]).buffer)[0]
| const auditResult = Audit.generateAuditResult(PassOrFailAudit, | ||
| {score: 0, errorMessage: 'what errors lurk'}); | ||
| assert.strictEqual(auditResult.score, null); | ||
| assert.equal(auditResult.title, 'Passing'); |
There was a problem hiding this comment.
we use the passing title for errors? funny the things you learn in testing :)
There was a problem hiding this comment.
we use the passing title for errors? funny the things you learn in testing :)
yeah, I was really surprised when I made score === null explicitly use the failureTitle and a whole bunch of things changed in sample_v2.json :)
I'm not entirely convinced it was on purpose, but it makes sense for NOT_APPLICABLE and INFORMATIVE, and it kind of makes sense for ERROR, since we don't know the page was doing the failure thing.
The majority of the change in #6199 (comment).
scorea required part ofLH.Audit.Product, either a number or null (in case of ERROR, NOT_APPLICABLE, or INFORMATIVE)rawValueinto ascoreof 0 or 1, then drop thoserawValuesMostly string replace, except in
audit.jsandaudit-test.jsafter this I think we want to (at least) rename
rawValueto finish out #6199