core: require score in audit product; make rawValue numeric only (#8343)

GoogleChrome · Apr 17, 2019 · ed2676a · ed2676a
1 parent 7154ebd
commit ed2676a
Show file tree

Hide file tree

Showing 128 changed files with 538 additions and 580 deletions.
diff --git a/docs/understanding-results.md b/docs/understanding-results.md
@@ -71,7 +71,6 @@ An object containing the results of the audits, keyed by their name.
       "description": "All sites should be protected with HTTPS, even ones that don't handle sensitive data. HTTPS prevents intruders from tampering with or passively listening in on the communications between your app and your users, and is a prerequisite for HTTP/2 and many new web platform APIs. [Learn more](https://developers.google.com/web/tools/lighthouse/audits/https).",
       "score": 0,
       "scoreDisplayMode": "binary",
-      "rawValue": false,
       "displayValue": "1 insecure request found",
       "details": {
         "type": "table",

diff --git a/lighthouse-cli/test/cli/run-test.js b/lighthouse-cli/test/cli/run-test.js
@@ -59,7 +59,7 @@ describe('CLI run', function() {
 
     it('returns results that match the saved results', () => {
       const {lhr} = passedResults;
-      assert.equal(fileResults.audits.viewport.rawValue, false);
+      assert.equal(fileResults.audits.viewport.score, 0);
 
       // passed results match saved results
       assert.strictEqual(fileResults.fetchTime, lhr.fetchTime);

diff --git a/lighthouse-core/audits/accessibility/axe-audit.js b/lighthouse-core/audits/accessibility/axe-audit.js
@@ -34,7 +34,7 @@ class AxeAudit extends Audit {
     const isNotApplicable = notApplicables.find(result => result.id === this.meta.id);
     if (isNotApplicable) {
       return {
-        rawValue: true,
+        score: 1,
         notApplicable: true,
       };
     }
@@ -74,7 +74,7 @@ class AxeAudit extends Audit {
     }
 
     return {
-      rawValue: typeof rule === 'undefined',
+      score: Number(rule === undefined),
       extendedInfo: {
         value: rule,
       },

diff --git a/lighthouse-core/audits/audit.js b/lighthouse-core/audits/audit.js
@@ -89,18 +89,6 @@ class Audit {
     return clampTo2Decimals(score);
   }
 
-  /**
-   * @param {typeof Audit} audit
-   * @param {string} errorMessage
-   * @return {LH.Audit.Result}
-   */
-  static generateErrorAuditResult(audit, errorMessage) {
-    return Audit.generateAuditResult(audit, {
-      rawValue: null,
-      errorMessage,
-    });
-  }
-
   /**
    * @param {LH.Audit.Details.Table['headings']} headings
    * @param {LH.Audit.Details.Table['items']} results
@@ -209,78 +197,87 @@ class Audit {
   }
 
   /**
-   * @param {typeof Audit} audit
-   * @param {LH.Audit.Product} result
-   * @return {{score: number|null, scoreDisplayMode: LH.Audit.ScoreDisplayMode}}
+   * @param {number|null} score
+   * @param {LH.Audit.ScoreDisplayMode} scoreDisplayMode
+   * @param {string} auditId
+   * @return {number|null}
    */
-  static _normalizeAuditScore(audit, result) {
-    // Cast true/false to 1/0
-    let score = result.score === undefined ? Number(result.rawValue) : result.score;
+  static _normalizeAuditScore(score, scoreDisplayMode, auditId) {
+    if (scoreDisplayMode !== Audit.SCORING_MODES.BINARY &&
+        scoreDisplayMode !== Audit.SCORING_MODES.NUMERIC) {
+      return null;
+    }
 
-    if (!Number.isFinite(score)) throw new Error(`Invalid score: ${score}`);
-    if (score > 1) throw new Error(`Audit score for ${audit.meta.id} is > 1`);
-    if (score < 0) throw new Error(`Audit score for ${audit.meta.id} is < 0`);
+    // Otherwise, score must be a number in [0, 1].
+    if (score === null || !Number.isFinite(score)) {
+      throw new Error(`Invalid score for ${auditId}: ${score}`);
+    }
+    if (score > 1) throw new Error(`Audit score for ${auditId} is > 1`);
+    if (score < 0) throw new Error(`Audit score for ${auditId} is < 0`);
 
     score = clampTo2Decimals(score);
 
-    const scoreDisplayMode = audit.meta.scoreDisplayMode || Audit.SCORING_MODES.BINARY;
+    return score;
+  }
 
-    return {
-      score,
-      scoreDisplayMode,
-    };
+  /**
+   * @param {typeof Audit} audit
+   * @param {string} errorMessage
+   * @return {LH.Audit.Result}
+   */
+  static generateErrorAuditResult(audit, errorMessage) {
+    return Audit.generateAuditResult(audit, {
+      score: null,
+      errorMessage,
+    });
   }
 
   /**
    * @param {typeof Audit} audit
-   * @param {LH.Audit.Product} result
+   * @param {LH.Audit.Product} product
    * @return {LH.Audit.Result}
    */
-  static generateAuditResult(audit, result) {
-    if (typeof result.rawValue === 'undefined') {
-      throw new Error('generateAuditResult requires a rawValue');
+  static generateAuditResult(audit, product) {
+    if (product.score === undefined) {
+      throw new Error('generateAuditResult requires a score');
     }
 
-    // TODO(bckenny): cleanup the flow of notApplicable/error/binary/numeric
-    let {score, scoreDisplayMode} = Audit._normalizeAuditScore(audit, result);
+    // Default to binary scoring.
+    let scoreDisplayMode = audit.meta.scoreDisplayMode || Audit.SCORING_MODES.BINARY;
 
-    // If the audit was determined to not apply to the page, set score display mode appropriately
-    if (result.notApplicable) {
+    // But override if product contents require it.
+    if (product.errorMessage) {
+      // Error result.
+      scoreDisplayMode = Audit.SCORING_MODES.ERROR;
+    } else if (product.notApplicable) {
+      // Audit was determined to not apply to the page.
       scoreDisplayMode = Audit.SCORING_MODES.NOT_APPLICABLE;
-      result.rawValue = true;
     }
 
-    if (result.errorMessage) {
-      scoreDisplayMode = Audit.SCORING_MODES.ERROR;
-    }
+    const score = Audit._normalizeAuditScore(product.score, scoreDisplayMode, audit.meta.id);
 
     let auditTitle = audit.meta.title;
     if (audit.meta.failureTitle) {
-      if (Number(score) < Util.PASS_THRESHOLD) {
+      if (score !== null && score < Util.PASS_THRESHOLD) {
         auditTitle = audit.meta.failureTitle;
       }
     }
 
-    if (scoreDisplayMode !== Audit.SCORING_MODES.BINARY &&
-        scoreDisplayMode !== Audit.SCORING_MODES.NUMERIC) {
-      score = null;
-    }
-
     return {
       id: audit.meta.id,
       title: auditTitle,
       description: audit.meta.description,
 
       score,
       scoreDisplayMode,
-      rawValue: result.rawValue,
+      rawValue: product.rawValue,
 
-      displayValue: result.displayValue,
-      explanation: result.explanation,
-      errorMessage: result.errorMessage,
-      warnings: result.warnings,
+      displayValue: product.displayValue,
+      explanation: product.explanation,
+      errorMessage: product.errorMessage,
+      warnings: product.warnings,
 
-      details: result.details,
+      details: product.details,
     };
   }
 }

diff --git a/lighthouse-core/audits/content-width.js b/lighthouse-core/audits/content-width.js
@@ -35,12 +35,12 @@ class ContentWidth extends Audit {
 
     if (IsMobile) {
       return {
-        rawValue: widthsMatch,
+        score: Number(widthsMatch),
         explanation: this.createExplanation(widthsMatch, artifacts.ViewportDimensions),
       };
     } else {
       return {
-        rawValue: true,
+        score: 1,
         notApplicable: true,
       };
     }

diff --git a/lighthouse-core/audits/critical-request-chains.js b/lighthouse-core/audits/critical-request-chains.js
@@ -201,7 +201,7 @@ class CriticalRequestChains extends Audit {
       const longestChain = CriticalRequestChains._getLongestChain(flattenedChains);
 
       return {
-        rawValue: chainCount === 0,
+        score: Number(chainCount === 0),
         notApplicable: chainCount === 0,
         displayValue: chainCount ? str_(UIStrings.displayValue, {itemCount: chainCount}) : '',
         extendedInfo: {

diff --git a/lighthouse-core/audits/deprecations.js b/lighthouse-core/audits/deprecations.js
@@ -61,7 +61,7 @@ class Deprecations extends Audit {
     }
 
     return {
-      rawValue: deprecations.length === 0,
+      score: Number(deprecations.length === 0),
       displayValue,
       extendedInfo: {
         value: deprecations,

diff --git a/lighthouse-core/audits/dobetterweb/appcache-manifest.js b/lighthouse-core/audits/dobetterweb/appcache-manifest.js
@@ -36,7 +36,7 @@ class AppCacheManifestAttr extends Audit {
     const displayValue = usingAppcache ? `Found "${artifacts.AppCacheManifest}"` : '';
 
     return {
-      rawValue: !usingAppcache,
+      score: usingAppcache ? 0 : 1,
       displayValue,
     };
   }

diff --git a/lighthouse-core/audits/dobetterweb/doctype.js b/lighthouse-core/audits/dobetterweb/doctype.js
@@ -30,7 +30,7 @@ class Doctype extends Audit {
   static audit(artifacts) {
     if (!artifacts.Doctype) {
       return {
-        rawValue: false,
+        score: 0,
         explanation: 'Document must contain a doctype',
       };
     }
@@ -42,14 +42,14 @@ class Doctype extends Audit {
 
     if (doctypePublicId !== '') {
       return {
-        rawValue: false,
+        score: 0,
         explanation: 'Expected publicId to be an empty string',
       };
     }
 
     if (doctypeSystemId !== '') {
       return {
-        rawValue: false,
+        score: 0,
         explanation: 'Expected systemId to be an empty string',
       };
     }
@@ -59,11 +59,11 @@ class Doctype extends Audit {
        https://html.spec.whatwg.org/multipage/parsing.html#the-initial-insertion-mode */
     if (doctypeName === 'html') {
       return {
-        rawValue: true,
+        score: 1,
       };
     } else {
       return {
-        rawValue: false,
+        score: 0,
         explanation: 'Doctype name must be the lowercase string `html`',
       };
     }

diff --git a/lighthouse-core/audits/dobetterweb/external-anchors-use-rel-noopener.js b/lighthouse-core/audits/dobetterweb/external-anchors-use-rel-noopener.js
@@ -67,7 +67,7 @@ class ExternalAnchorsUseRelNoopenerAudit extends Audit {
     const details = Audit.makeTableDetails(headings, failingAnchors);
 
     return {
-      rawValue: failingAnchors.length === 0,
+      score: Number(failingAnchors.length === 0),
       extendedInfo: {
         value: failingAnchors,
       },

diff --git a/lighthouse-core/audits/dobetterweb/geolocation-on-start.js b/lighthouse-core/audits/dobetterweb/geolocation-on-start.js
@@ -47,7 +47,7 @@ class GeolocationOnStart extends ViolationAudit {
     const details = ViolationAudit.makeTableDetails(headings, results);
 
     return {
-      rawValue: results.length === 0,
+      score: Number(results.length === 0),
       extendedInfo: {
         value: results,
       },

diff --git a/lighthouse-core/audits/dobetterweb/js-libraries.js b/lighthouse-core/audits/dobetterweb/js-libraries.js
@@ -46,7 +46,7 @@ class JsLibrariesAudit extends Audit {
     const details = Audit.makeTableDetails(headings, libDetails, {});
 
     return {
-      rawValue: true, // Always pass for now.
+      score: 1, // Always pass for now.
       details,
     };
   }

diff --git a/lighthouse-core/audits/dobetterweb/no-document-write.js b/lighthouse-core/audits/dobetterweb/no-document-write.js
@@ -44,7 +44,7 @@ class NoDocWriteAudit extends ViolationAudit {
     const details = ViolationAudit.makeTableDetails(headings, results);
 
     return {
-      rawValue: results.length === 0,
+      score: Number(results.length === 0),
       extendedInfo: {
         value: results,
       },

diff --git a/lighthouse-core/audits/dobetterweb/no-vulnerable-libraries.js b/lighthouse-core/audits/dobetterweb/no-vulnerable-libraries.js
@@ -140,7 +140,7 @@ class NoVulnerableLibrariesAudit extends Audit {
 
     if (!foundLibraries.length) {
       return {
-        rawValue: true,
+        score: 1,
       };
     }
 
@@ -194,7 +194,7 @@ class NoVulnerableLibrariesAudit extends Audit {
     const details = Audit.makeTableDetails(headings, vulnerabilityResults, {});
 
     return {
-      rawValue: totalVulns === 0,
+      score: Number(totalVulns === 0),
       displayValue,
       extendedInfo: {
         jsLibs: libraryVulns,

diff --git a/lighthouse-core/audits/dobetterweb/notification-on-start.js b/lighthouse-core/audits/dobetterweb/notification-on-start.js
@@ -45,7 +45,7 @@ class NotificationOnStart extends ViolationAudit {
     const details = ViolationAudit.makeTableDetails(headings, results);
 
     return {
-      rawValue: results.length === 0,
+      score: Number(results.length === 0),
       extendedInfo: {
         value: results,
       },

diff --git a/lighthouse-core/audits/dobetterweb/password-inputs-can-be-pasted-into.js b/lighthouse-core/audits/dobetterweb/password-inputs-can-be-pasted-into.js
@@ -43,7 +43,7 @@ class PasswordInputsCanBePastedIntoAudit extends Audit {
     ];
 
     return {
-      rawValue: passwordInputsWithPreventedPaste.length === 0,
+      score: Number(passwordInputsWithPreventedPaste.length === 0),
       extendedInfo: {
         value: passwordInputsWithPreventedPaste,
       },

diff --git a/lighthouse-core/audits/dobetterweb/uses-http2.js b/lighthouse-core/audits/dobetterweb/uses-http2.js
@@ -79,7 +79,7 @@ class UsesHTTP2Audit extends Audit {
       const details = Audit.makeTableDetails(headings, resources);
 
       return {
-        rawValue: resources.length === 0,
+        score: Number(resources.length === 0),
         displayValue: displayValue,
         extendedInfo: {
           value: {

diff --git a/lighthouse-core/audits/dobetterweb/uses-passive-event-listeners.js b/lighthouse-core/audits/dobetterweb/uses-passive-event-listeners.js
@@ -45,7 +45,7 @@ class PassiveEventsAudit extends ViolationAudit {
     const details = ViolationAudit.makeTableDetails(headings, results);
 
     return {
-      rawValue: results.length === 0,
+      score: Number(results.length === 0),
       extendedInfo: {
         value: results,
       },

diff --git a/lighthouse-core/audits/final-screenshot.js b/lighthouse-core/audits/final-screenshot.js
@@ -38,7 +38,7 @@ class FinalScreenshot extends Audit {
     }
 
     return {
-      rawValue: true,
+      score: 1,
       details: {
         type: 'screenshot',
         timestamp: finalScreenshot.timestamp,

diff --git a/lighthouse-core/audits/font-display.js b/lighthouse-core/audits/font-display.js
@@ -137,7 +137,6 @@ class FontDisplay extends Audit {
 
     return {
       score: Number(results.length === 0),
-      rawValue: results.length === 0,
       details,
     };
   }

diff --git a/lighthouse-core/audits/image-aspect-ratio.js b/lighthouse-core/audits/image-aspect-ratio.js
@@ -105,7 +105,7 @@ class ImageAspectRatio extends Audit {
     ];
 
     return {
-      rawValue: results.length === 0,
+      score: Number(results.length === 0),
       warnings,
       details: Audit.makeTableDetails(headings, results),
     };

diff --git a/lighthouse-core/audits/is-on-https.js b/lighthouse-core/audits/is-on-https.js
@@ -68,7 +68,7 @@ class HTTPS extends Audit {
       ];
 
       return {
-        rawValue: items.length === 0,
+        score: Number(items.length === 0),
         displayValue,
         extendedInfo: {
           value: items,

diff --git a/lighthouse-core/audits/manual/manual-audit.js b/lighthouse-core/audits/manual/manual-audit.js
@@ -28,7 +28,7 @@ class ManualAudit extends Audit {
    */
   static audit() {
     return {
-      rawValue: false,
+      score: 0,
       // displayValue: '(needs manual verification)'
     };
   }