core(multi-check): expose manifest checks in details (#5405)

GoogleChrome · Jun 11, 2018 · edbca2a · edbca2a
1 parent ee0c35a
commit edbca2a
Show file tree

Hide file tree

Showing 10 changed files with 116 additions and 32 deletions.
diff --git a/lighthouse-cli/test/smokehouse/pwa-expectations.js b/lighthouse-cli/test/smokehouse/pwa-expectations.js
@@ -5,6 +5,18 @@
  */
 'use strict';
 
+const pwaDetailsExpectations = {
+  isParseFailure: false,
+  hasStartUrl: true,
+  hasIconsAtLeast192px: true,
+  hasIconsAtLeast512px: true,
+  hasPWADisplayValue: true,
+  hasBackgroundColor: true,
+  hasThemeColor: true,
+  hasShortName: true,
+  hasName: true,
+};
+
 /**
  * Expected Lighthouse audit values for various sites with stable(ish) PWA
  * results.
@@ -36,16 +48,16 @@ module.exports = [
         // Ignore speed test; just verify that it ran.
       },
       'webapp-install-banner': {
-        // TODO(phulce): assert the checks when we put them in details
         score: 1,
+        details: {items: [pwaDetailsExpectations]},
       },
       'splash-screen': {
-        // TODO(phulce): assert the checks when we put them in details
         score: 1,
+        details: {items: [pwaDetailsExpectations]},
       },
       'themed-omnibox': {
-        // TODO(phulce): assert the checks when we put them in details
         score: 1,
+        details: {items: [{...pwaDetailsExpectations, themeColor: '#2196F3'}]},
       },
       'content-width': {
         score: 1,
@@ -93,16 +105,16 @@ module.exports = [
         // Ignore speed test; just verify that it ran.
       },
       'webapp-install-banner': {
-        // TODO(phulce): assert the checks when we put them in details
         score: 1,
+        details: {items: [pwaDetailsExpectations]},
       },
       'splash-screen': {
-        // TODO(phulce): assert the checks when we put them in details
         score: 1,
+        details: {items: [pwaDetailsExpectations]},
       },
       'themed-omnibox': {
-        // TODO(phulce): assert the checks when we put them in details
         score: 1,
+        details: {items: [pwaDetailsExpectations]},
       },
       'content-width': {
         score: 1,
@@ -124,3 +136,5 @@ module.exports = [
     },
   },
 ];
+
+module.exports.PWA_DETAILS_EXPECTATIONS = pwaDetailsExpectations;
diff --git a/lighthouse-cli/test/smokehouse/pwa2-expectations.js b/lighthouse-cli/test/smokehouse/pwa2-expectations.js
@@ -5,6 +5,10 @@
  */
 'use strict';
 
+const pwaDetailsExpectations = require('./pwa-expectations').PWA_DETAILS_EXPECTATIONS;
+
+const jakeExpectations = {...pwaDetailsExpectations, hasShortName: false};
+
 /**
  * Expected Lighthouse audit values for various sites with stable(ish) PWA
  * results.
@@ -38,16 +42,16 @@ module.exports = [
         // Ignore speed test; just verify that it ran.
       },
       'webapp-install-banner': {
-        // TODO(phulce): assert the checks when we put them in details
         score: 0,
+        details: {items: [jakeExpectations]},
       },
       'splash-screen': {
-        // TODO(phulce): assert the checks when we put them in details
         score: 1,
+        details: {items: [jakeExpectations]},
       },
       'themed-omnibox': {
-        // TODO(phulce): assert the checks when we put them in details
         score: 1,
+        details: {items: [jakeExpectations]},
       },
       'content-width': {
         score: 1,
@@ -95,16 +99,16 @@ module.exports = [
         // Ignore speed test; just verify that it ran.
       },
       'webapp-install-banner': {
-        // TODO(phulce): assert the checks when we put them in details
         score: 1,
+        details: {items: [pwaDetailsExpectations]},
       },
       'splash-screen': {
-        // TODO(phulce): assert the checks when we put them in details
         score: 1,
+        details: {items: [pwaDetailsExpectations]},
       },
       'themed-omnibox': {
-        // TODO(phulce): assert the checks when we put them in details
         score: 1,
+        details: {items: [pwaDetailsExpectations]},
       },
       'content-width': {
         score: 1,

diff --git a/lighthouse-cli/test/smokehouse/pwa3-expectations.js b/lighthouse-cli/test/smokehouse/pwa3-expectations.js
@@ -5,6 +5,10 @@
  */
 'use strict';
 
+const pwaDetailsExpectations = require('./pwa-expectations').PWA_DETAILS_EXPECTATIONS;
+
+const pwaRocksExpectations = {...pwaDetailsExpectations, hasIconsAtLeast512px: false};
+
 /**
  * Expected Lighthouse audit values for various sites with stable(ish) PWA
  * results.
@@ -36,16 +40,16 @@ module.exports = [
         // Ignore speed test; just verify that it ran .
       },
       'webapp-install-banner': {
-        // TODO(phulce): assert the checks when we put them in details
         score: 1,
+        details: {items: [pwaRocksExpectations]},
       },
       'splash-screen': {
-        // TODO(phulce): assert the checks when we put them in details
         score: 0,
+        details: {items: [pwaRocksExpectations]},
       },
       'themed-omnibox': {
-        // TODO(phulce): assert the checks when we put them in details
         score: 0,
+        details: {items: [pwaRocksExpectations]},
       },
       'content-width': {
         score: 1,

diff --git a/lighthouse-core/audits/multi-check-audit.js b/lighthouse-core/audits/multi-check-audit.js
@@ -25,23 +25,36 @@ class MultiCheckAudit extends Audit {
    * @return {LH.Audit.Product}
    */
   static createAuditProduct(result) {
-    const extendedInfo = {
-      value: result,
+    /** @type {LH.Audit.MultiCheckAuditDetails} */
+    const detailsItem = {
+      ...result,
+      ...result.manifestValues,
+      manifestValues: undefined,
+      warnings: undefined,
+      allChecks: undefined,
     };
 
+    if (result.manifestValues && result.manifestValues.allChecks) {
+      result.manifestValues.allChecks.forEach(check => {
+        detailsItem[check.id] = check.passing;
+      });
+    }
+
+    const details = {items: [detailsItem]};
+
     // If we fail, share the failures
     if (result.failures.length > 0) {
       return {
         rawValue: false,
         explanation: `Failures: ${result.failures.join(',\n')}.`,
-        extendedInfo,
+        details,
       };
     }
 
     // Otherwise, we pass
     return {
       rawValue: true,
-      extendedInfo,
+      details,
       warnings: result.warnings,
     };
   }

diff --git a/lighthouse-core/gather/computed/manifest-values.js b/lighthouse-core/gather/computed/manifest-values.js
@@ -26,7 +26,7 @@ class ManifestValues extends ComputedArtifact {
   /** @typedef {(val: NonNullable<LH.Artifacts.Manifest['value']>) => boolean} Validator */
 
   /**
-   * @return {Array<{id: string, failureText: string, validate: Validator}>}
+   * @return {Array<{id: LH.Artifacts.ManifestValueCheckID, failureText: string, validate: Validator}>}
    */
   static get manifestChecks() {
     return [

diff --git a/lighthouse-core/test/audits/splash-screen-test.js b/lighthouse-core/test/audits/splash-screen-test.js
@@ -62,7 +62,7 @@ describe('PWA: splash screen audit', () => {
       return SplashScreenAudit.audit(artifacts).then(result => {
         assert.strictEqual(result.rawValue, false);
         assert.ok(result.explanation);
-        assert.strictEqual(result.extendedInfo.value.failures.length, 4);
+        assert.strictEqual(result.details.items[0].failures.length, 4);
       });
     });
 

diff --git a/lighthouse-core/test/audits/webapp-install-banner-test.js b/lighthouse-core/test/audits/webapp-install-banner-test.js
@@ -61,7 +61,7 @@ describe('PWA: webapp install banner audit', () => {
       return WebappInstallBannerAudit.audit(artifacts).then(result => {
         assert.strictEqual(result.rawValue, false);
         assert.ok(result.explanation);
-        assert.strictEqual(result.extendedInfo.value.failures.length, 4);
+        assert.strictEqual(result.details.items[0].failures.length, 4);
       });
     });
 
@@ -82,7 +82,7 @@ describe('PWA: webapp install banner audit', () => {
       return WebappInstallBannerAudit.audit(artifacts).then(result => {
         assert.strictEqual(result.rawValue, false);
         assert.ok(result.explanation.includes('start_url'), result.explanation);
-        const failures = result.extendedInfo.value.failures;
+        const failures = result.details.items[0].failures;
         assert.strictEqual(failures.length, 1, failures);
       });
     });
@@ -95,7 +95,7 @@ describe('PWA: webapp install banner audit', () => {
       return WebappInstallBannerAudit.audit(artifacts).then(result => {
         assert.strictEqual(result.rawValue, false);
         assert.ok(result.explanation.includes('short_name'), result.explanation);
-        const failures = result.extendedInfo.value.failures;
+        const failures = result.details.items[0].failures;
         assert.strictEqual(failures.length, 1, failures);
       });
     });
@@ -107,7 +107,7 @@ describe('PWA: webapp install banner audit', () => {
       return WebappInstallBannerAudit.audit(artifacts).then(result => {
         assert.strictEqual(result.rawValue, false);
         assert.ok(result.explanation.includes('name'), result.explanation);
-        const failures = result.extendedInfo.value.failures;
+        const failures = result.details.items[0].failures;
         assert.strictEqual(failures.length, 1, failures);
       });
     });
@@ -119,7 +119,7 @@ describe('PWA: webapp install banner audit', () => {
       return WebappInstallBannerAudit.audit(artifacts).then(result => {
         assert.strictEqual(result.rawValue, false);
         assert.ok(result.explanation.includes('icons'), result.explanation);
-        const failures = result.extendedInfo.value.failures;
+        const failures = result.details.items[0].failures;
         assert.strictEqual(failures.length, 1, failures);
       });
     });
@@ -133,7 +133,7 @@ describe('PWA: webapp install banner audit', () => {
     return WebappInstallBannerAudit.audit(artifacts).then(result => {
       assert.strictEqual(result.rawValue, false);
       assert.ok(result.explanation.includes('service worker'), result.explanation);
-      const failures = result.extendedInfo.value.failures;
+      const failures = result.details.items[0].failures;
       assert.strictEqual(failures.length, 1, failures);
     });
   });
@@ -145,7 +145,7 @@ describe('PWA: webapp install banner audit', () => {
     return WebappInstallBannerAudit.audit(artifacts).then(result => {
       assert.strictEqual(result.rawValue, false);
       assert.ok(result.explanation.includes('start_url'), result.explanation);
-      const failures = result.extendedInfo.value.failures;
+      const failures = result.details.items[0].failures;
       assert.strictEqual(failures.length, 1, failures);
     });
   });

diff --git a/lighthouse-core/test/results/sample_v2.json b/lighthouse-core/test/results/sample_v2.json
@@ -486,7 +486,19 @@
       "score": 0,
       "scoreDisplayMode": "binary",
       "rawValue": false,
-      "explanation": "Failures: No manifest was fetched,\nSite does not register a service worker."
+      "explanation": "Failures: No manifest was fetched,\nSite does not register a service worker.",
+      "details": {
+        "items": [
+          {
+            "failures": [
+              "No manifest was fetched",
+              "Site does not register a service worker"
+            ],
+            "isParseFailure": true,
+            "parseFailureReason": "No manifest was fetched"
+          }
+        ]
+      }
     },
     "splash-screen": {
       "id": "splash-screen",
@@ -495,7 +507,18 @@
       "score": 0,
       "scoreDisplayMode": "binary",
       "rawValue": false,
-      "explanation": "Failures: No manifest was fetched."
+      "explanation": "Failures: No manifest was fetched.",
+      "details": {
+        "items": [
+          {
+            "failures": [
+              "No manifest was fetched"
+            ],
+            "isParseFailure": true,
+            "parseFailureReason": "No manifest was fetched"
+          }
+        ]
+      }
     },
     "themed-omnibox": {
       "id": "themed-omnibox",
@@ -504,7 +527,20 @@
       "score": 0,
       "scoreDisplayMode": "binary",
       "rawValue": false,
-      "explanation": "Failures: No manifest was fetched,\nNo `<meta name=\"theme-color\">` tag found."
+      "explanation": "Failures: No manifest was fetched,\nNo `<meta name=\"theme-color\">` tag found.",
+      "details": {
+        "items": [
+          {
+            "failures": [
+              "No manifest was fetched",
+              "No `<meta name=\"theme-color\">` tag found"
+            ],
+            "themeColor": null,
+            "isParseFailure": true,
+            "parseFailureReason": "No manifest was fetched"
+          }
+        ]
+      }
     },
     "manifest-short-name-length": {
       "id": "manifest-short-name-length",

diff --git a/typings/artifacts.d.ts b/typings/artifacts.d.ts
@@ -307,11 +307,13 @@ declare global {
         bottomUpGroupBy(grouping: string): DevtoolsTimelineModelNode;
       }
 
+      export type ManifestValueCheckID = 'hasStartUrl'|'hasIconsAtLeast192px'|'hasIconsAtLeast512px'|'hasPWADisplayValue'|'hasBackgroundColor'|'hasThemeColor'|'hasShortName'|'hasName'|'shortNameLength';
+
       export interface ManifestValues {
         isParseFailure: boolean;
         parseFailureReason: string | undefined;
         allChecks: {
-          id: string;
+          id: ManifestValueCheckID;
           failureText: string;
           passing: boolean;
         }[];

diff --git a/typings/audit.d.ts b/typings/audit.d.ts
@@ -145,6 +145,17 @@ declare global {
         children: SimpleCriticalRequestNode;
       }
     }
+
+    type MultiCheckAuditP1 = Partial<Record<Artifacts.ManifestValueCheckID, boolean>>;
+    type MultiCheckAuditP2 = Partial<Artifacts.ManifestValues>;
+    interface MultiCheckAuditP3 {
+      failures: Array<string>;
+      warnings?: undefined;
+      manifestValues?: undefined;
+      allChecks?: undefined;
+    }
+
+    export type MultiCheckAuditDetails = MultiCheckAuditP1 & MultiCheckAuditP2 & MultiCheckAuditP3;
   }
 }