core(lhr): remove debugString, add explantion/errorMessage (GoogleChr…

…ome#5132)
kdzwinel · Aug 16, 2018 · a63fb43 · a63fb43
1 parent 6fbf4d0
commit a63fb43
Show file tree

Hide file tree

Showing 63 changed files with 304 additions and 376 deletions.
diff --git a/lighthouse-cli/test/smokehouse/dobetterweb/dbw-expectations.js b/lighthouse-cli/test/smokehouse/dobetterweb/dbw-expectations.js
@@ -47,8 +47,7 @@ module.exports = [
       },
       'external-anchors-use-rel-noopener': {
         score: 0,
-        debugString: 'Lighthouse was unable to determine the destination of some anchor tags. ' +
-                     'If they are not used as hyperlinks, consider removing the _blank target.',
+        warnings: [/Unable to determine/],
         extendedInfo: {
           value: {
             length: 3,
@@ -62,7 +61,7 @@ module.exports = [
       },
       'appcache-manifest': {
         score: 0,
-        debugString: 'Found <html manifest="clock.appcache">.',
+        displayValue: 'Found "clock.appcache"',
       },
       'geolocation-on-start': {
         score: 0,
@@ -105,7 +104,7 @@ module.exports = [
       },
       'no-websql': {
         score: 0,
-        debugString: 'Found database "mydb", version: 1.0.',
+        displayValue: 'Found "mydb" (v1.0)',
       },
       'notification-on-start': {
         score: 0,

diff --git a/lighthouse-cli/test/smokehouse/seo/expectations.js b/lighthouse-cli/test/smokehouse/seo/expectations.js
@@ -101,7 +101,7 @@ module.exports = [
       },
       'font-size': {
         rawValue: false,
-        debugString: 'Text is illegible because of a missing viewport config',
+        explanation: 'Text is illegible because of a missing viewport config',
       },
       'link-text': {
         score: 0,
@@ -138,7 +138,7 @@ module.exports = [
       },
       'canonical': {
         score: 0,
-        debugString: 'Multiple conflicting URLs (https://example.com, https://example.com/)',
+        explanation: 'Multiple conflicting URLs (https://example.com, https://example.com/)',
       },
     },
   },

diff --git a/lighthouse-core/audits/audit.js b/lighthouse-core/audits/audit.js
@@ -91,14 +91,13 @@ class Audit {
 
   /**
    * @param {typeof Audit} audit
-   * @param {string} debugString
+   * @param {string} errorMessage
    * @return {LH.Audit.Result}
    */
-  static generateErrorAuditResult(audit, debugString) {
+  static generateErrorAuditResult(audit, errorMessage) {
     return Audit.generateAuditResult(audit, {
       rawValue: null,
-      error: true,
-      debugString,
+      errorMessage,
     });
   }
 
@@ -159,6 +158,7 @@ class Audit {
       throw new Error('generateAuditResult requires a rawValue');
     }
 
+    // TODO(bckenny): cleanup the flow of notApplicable/error/binary/numeric
     let {score, scoreDisplayMode} = Audit._normalizeAuditScore(audit, result);
 
     // If the audit was determined to not apply to the page, set score display mode appropriately
@@ -167,7 +167,7 @@ class Audit {
       result.rawValue = true;
     }
 
-    if (result.error) {
+    if (result.errorMessage) {
       scoreDisplayMode = Audit.SCORING_MODES.ERROR;
     }
 
@@ -185,10 +185,13 @@ class Audit {
 
     return {
       score,
-      displayValue: result.displayValue || '',
       rawValue: result.rawValue,
-      error: result.error,
-      debugString: result.debugString,
+
+      displayValue: result.displayValue,
+      explanation: result.explanation,
+      errorMessage: result.errorMessage,
+      warnings: result.warnings,
+
       extendedInfo: result.extendedInfo,
       scoreDisplayMode,
       name: audit.meta.name,

diff --git a/lighthouse-core/audits/byte-efficiency/byte-efficiency-audit.js b/lighthouse-core/audits/byte-efficiency/byte-efficiency-audit.js
@@ -165,7 +165,6 @@ class UnusedBytes extends Audit {
    * @return {LH.Audit.Product}
    */
   static createAuditProduct(result, graph, simulator) {
-    const debugString = result.debugString;
     const results = result.results.sort((itemA, itemB) => itemB.wastedBytes - itemA.wastedBytes);
 
     const wastedBytes = results.reduce((sum, item) => sum + item.wastedBytes, 0);
@@ -187,7 +186,8 @@ class UnusedBytes extends Audit {
     const details = Audit.makeTableDetails(result.headings, results, summary);
 
     return {
-      debugString,
+      explanation: result.explanation,
+      warnings: result.warnings,
       displayValue,
       rawValue: wastedMs,
       score: UnusedBytes.scoreForWastedMs(wastedMs),

diff --git a/lighthouse-core/audits/byte-efficiency/offscreen-images.js b/lighthouse-core/audits/byte-efficiency/offscreen-images.js
@@ -113,16 +113,16 @@ class OffscreenImages extends ByteEfficiencyAudit {
     const trace = artifacts.traces[ByteEfficiencyAudit.DEFAULT_PASS];
     const devtoolsLog = artifacts.devtoolsLogs[ByteEfficiencyAudit.DEFAULT_PASS];
 
-    /** @type {string|undefined} */
-    let debugString;
+    /** @type {string[]} */
+    const warnings = [];
     const resultsMap = images.reduce((results, image) => {
       const processed = OffscreenImages.computeWaste(image, viewportDimensions);
       if (processed === null) {
         return results;
       }
 
       if (processed instanceof Error) {
-        debugString = processed.message;
+        warnings.push(processed.message);
         // @ts-ignore TODO(bckenny): Sentry type checking
         Sentry.captureException(processed, {tags: {audit: this.meta.name}, level: 'warning'});
         return results;
@@ -166,7 +166,7 @@ class OffscreenImages extends ByteEfficiencyAudit {
       ];
 
       return {
-        debugString,
+        warnings,
         results,
         headings,
       };

diff --git a/lighthouse-core/audits/byte-efficiency/unminified-javascript.js b/lighthouse-core/audits/byte-efficiency/unminified-javascript.js
@@ -77,7 +77,7 @@ class UnminifiedJavaScript extends ByteEfficiencyAudit {
   static audit_(artifacts, networkRecords) {
     /** @type {Array<LH.Audit.ByteEfficiencyResult>} */
     const results = [];
-    let debugString;
+    const warnings = [];
     for (const requestId of Object.keys(artifacts.Scripts)) {
       const scriptContent = artifacts.Scripts[requestId];
       const networkRecord = networkRecords.find(record => record.requestId === requestId);
@@ -92,13 +92,13 @@ class UnminifiedJavaScript extends ByteEfficiencyAudit {
           !Number.isFinite(result.wastedBytes)) continue;
         results.push(result);
       } catch (err) {
-        debugString = `Unable to process ${networkRecord._url}: ${err.message}`;
+        warnings.push(`Unable to process ${networkRecord._url}: ${err.message}`);
       }
     }
 
     return {
       results,
-      debugString,
+      warnings,
       headings: [
         {key: 'url', itemType: 'url', text: 'URL'},
         {key: 'totalBytes', itemType: 'bytes', displayUnit: 'kb', granularity: 1, text: 'Original'},

diff --git a/lighthouse-core/audits/byte-efficiency/uses-optimized-images.js b/lighthouse-core/audits/byte-efficiency/uses-optimized-images.js
@@ -48,10 +48,10 @@ class UsesOptimizedImages extends ByteEfficiencyAudit {
 
     /** @type {Array<{url: string, fromProtocol: boolean, isCrossOrigin: boolean, totalBytes: number, wastedBytes: number}>} */
     const results = [];
-    const failedImages = [];
+    const warnings = [];
     for (const image of images) {
       if (image.failed) {
-        failedImages.push(image);
+        warnings.push(`Unable to decode ${URL.getURLDisplayName(image.url)}`);
         continue;
       } else if (/(jpeg|bmp)/.test(image.mimeType) === false ||
                  image.originalSize < image.jpegSize + IGNORE_THRESHOLD_IN_BYTES) {
@@ -70,12 +70,6 @@ class UsesOptimizedImages extends ByteEfficiencyAudit {
       });
     }
 
-    let debugString;
-    if (failedImages.length) {
-      const urls = failedImages.map(image => URL.getURLDisplayName(image.url));
-      debugString = `Lighthouse was unable to decode some of your images: ${urls.join(', ')}`;
-    }
-
     const headings = [
       {key: 'url', itemType: 'thumbnail', text: ''},
       {key: 'url', itemType: 'url', text: 'URL'},
@@ -85,7 +79,7 @@ class UsesOptimizedImages extends ByteEfficiencyAudit {
     ];
 
     return {
-      debugString,
+      warnings,
       results,
       headings,
     };

diff --git a/lighthouse-core/audits/byte-efficiency/uses-responsive-images.js b/lighthouse-core/audits/byte-efficiency/uses-responsive-images.js
@@ -80,7 +80,8 @@ class UsesResponsiveImages extends ByteEfficiencyAudit {
     const images = artifacts.ImageUsage;
     const DPR = artifacts.ViewportDimensions.devicePixelRatio;
 
-    let debugString;
+    /** @type {string[]} */
+    const warnings = [];
     /** @type {Map<LH.Audit.ByteEfficiencyResult['url'], LH.Audit.ByteEfficiencyResult>} */
     const resultsMap = new Map();
     images.forEach(image => {
@@ -93,7 +94,7 @@ class UsesResponsiveImages extends ByteEfficiencyAudit {
       if (!processed) return;
 
       if (processed instanceof Error) {
-        debugString = processed.message;
+        warnings.push(processed.message);
         // @ts-ignore TODO(bckenny): Sentry type checking
         Sentry.captureException(processed, {tags: {audit: this.meta.name}, level: 'warning'});
         return;
@@ -118,7 +119,7 @@ class UsesResponsiveImages extends ByteEfficiencyAudit {
     ];
 
     return {
-      debugString,
+      warnings,
       results,
       headings,
     };

diff --git a/lighthouse-core/audits/byte-efficiency/uses-webp-images.js b/lighthouse-core/audits/byte-efficiency/uses-webp-images.js
@@ -48,10 +48,10 @@ class UsesWebPImages extends ByteEfficiencyAudit {
 
     /** @type {Array<{url: string, fromProtocol: boolean, isCrossOrigin: boolean, totalBytes: number, wastedBytes: number}>} */
     const results = [];
-    const failedImages = [];
+    const warnings = [];
     for (const image of images) {
       if (image.failed) {
-        failedImages.push(image);
+        warnings.push(`Unable to decode ${URL.getURLDisplayName(image.url)}`);
         continue;
       } else if (image.originalSize < image.webpSize + IGNORE_THRESHOLD_IN_BYTES) {
         continue;
@@ -69,12 +69,6 @@ class UsesWebPImages extends ByteEfficiencyAudit {
       });
     }
 
-    let debugString;
-    if (failedImages.length) {
-      const urls = failedImages.map(image => URL.getURLDisplayName(image.url));
-      debugString = `Lighthouse was unable to decode some of your images: ${urls.join(', ')}`;
-    }
-
     const headings = [
       {key: 'url', itemType: 'thumbnail', text: ''},
       {key: 'url', itemType: 'url', text: 'URL'},
@@ -84,7 +78,7 @@ class UsesWebPImages extends ByteEfficiencyAudit {
     ];
 
     return {
-      debugString,
+      warnings,
       results,
       headings,
     };

diff --git a/lighthouse-core/audits/content-width.js b/lighthouse-core/audits/content-width.js
@@ -34,7 +34,7 @@ class ContentWidth extends Audit {
 
     return {
       rawValue: widthsMatch,
-      debugString: this.createDebugString(widthsMatch, artifacts.ViewportDimensions),
+      explanation: this.createExplanation(widthsMatch, artifacts.ViewportDimensions),
     };
   }
 
@@ -43,7 +43,7 @@ class ContentWidth extends Audit {
    * @param {LH.Artifacts.ViewportDimensions} artifact
    * @return {string}
    */
-  static createDebugString(match, artifact) {
+  static createExplanation(match, artifact) {
     if (match) {
       return '';
     }

diff --git a/lighthouse-core/audits/dobetterweb/appcache-manifest.js b/lighthouse-core/audits/dobetterweb/appcache-manifest.js
@@ -33,12 +33,11 @@ class AppCacheManifestAttr extends Audit {
    */
   static audit(artifacts) {
     const usingAppcache = artifacts.AppCacheManifest !== null;
-    const debugString = usingAppcache ?
-        `Found <html manifest="${artifacts.AppCacheManifest}">.` : '';
+    const displayValue = usingAppcache ? `Found "${artifacts.AppCacheManifest}"` : '';
 
     return {
       rawValue: !usingAppcache,
-      debugString,
+      displayValue,
     };
   }
 }

diff --git a/lighthouse-core/audits/dobetterweb/external-anchors-use-rel-noopener.js b/lighthouse-core/audits/dobetterweb/external-anchors-use-rel-noopener.js
@@ -29,7 +29,8 @@ class ExternalAnchorsUseRelNoopenerAudit extends Audit {
    * @return {LH.Audit.Product}
    */
   static audit(artifacts) {
-    let debugString;
+    /** @type {string[]} */
+    const warnings = [];
     const pageHost = new URL(artifacts.URL.finalUrl).host;
     // Filter usages to exclude anchors that are same origin
     // TODO: better extendedInfo for anchors with no href attribute:
@@ -40,9 +41,9 @@ class ExternalAnchorsUseRelNoopenerAudit extends Audit {
         try {
           return new URL(anchor.href).host !== pageHost;
         } catch (err) {
-          debugString = 'Lighthouse was unable to determine the destination ' +
-              'of some anchor tags. If they are not used as hyperlinks, ' +
-              'consider removing the _blank target.';
+          // TODO(phulce): make this message better with anchor.outerHTML
+          warnings.push('Unable to determine the destination for anchor tag. ' +
+            'If not used as a hyperlink, consider removing target=_blank.');
           return true;
         }
       })
@@ -75,7 +76,7 @@ class ExternalAnchorsUseRelNoopenerAudit extends Audit {
         value: failingAnchors,
       },
       details,
-      debugString,
+      warnings,
     };
   }
 }

diff --git a/lighthouse-core/audits/dobetterweb/no-mutation-events.js b/lighthouse-core/audits/dobetterweb/no-mutation-events.js
@@ -49,7 +49,8 @@ class NoMutationEventsAudit extends Audit {
    * @return {LH.Audit.Product}
    */
   static audit(artifacts) {
-    let debugString;
+    /** @type {string[]} */
+    const warnings = [];
     const listeners = artifacts.EventListeners;
 
     const results = listeners.filter(loc => {
@@ -58,7 +59,7 @@ class NoMutationEventsAudit extends Audit {
 
       if (!URL.isValid(loc.url) && isMutationEvent) {
         sameHost = true;
-        debugString = URL.INVALID_URL_DEBUG_STRING;
+        warnings.push(URL.INVALID_URL_DEBUG_STRING);
       }
 
       return sameHost && isMutationEvent;
@@ -83,7 +84,7 @@ class NoMutationEventsAudit extends Audit {
         },
       },
       details,
-      debugString,
+      warnings,
     };
   }
 }

diff --git a/lighthouse-core/audits/dobetterweb/no-websql.js b/lighthouse-core/audits/dobetterweb/no-websql.js
@@ -34,12 +34,12 @@ class NoWebSQLAudit extends Audit {
    */
   static audit(artifacts) {
     const db = artifacts.WebSQL;
-    const debugString = (db ?
-        `Found database "${db.name}", version: ${db.version}.` : '');
+    const displayValue = (db ?
+        `Found "${db.name}" (v${db.version})` : '');
 
     return {
       rawValue: !db,
-      debugString,
+      displayValue,
     };
   }
 }