core(webapp-install): simplify start_url warning when no SW is found (G…

…oogleChrome#5067)
kdzwinel · Aug 16, 2018 · 07945b6 · 07945b6
1 parent d04c839
commit 07945b6
Show file tree

Hide file tree

Showing 3 changed files with 42 additions and 26 deletions.
diff --git a/lighthouse-core/audits/webapp-install-banner.js b/lighthouse-core/audits/webapp-install-banner.js
@@ -44,13 +44,12 @@ class WebappInstallBanner extends MultiCheckAudit {
     };
   }
 
-  static assessManifest(artifacts, result) {
-    const {manifestValues, failures} = result;
+  static assessManifest(manifestValues) {
     if (manifestValues.isParseFailure) {
-      failures.push(manifestValues.parseFailureReason);
-      return;
+      return [manifestValues.parseFailureReason];
     }
 
+    const failures = [];
     const bannerCheckIds = [
       'hasName',
       'hasShortName',
@@ -65,40 +64,62 @@ class WebappInstallBanner extends MultiCheckAudit {
           failures.push(item.failureText);
         }
       });
+
+    return failures;
   }
 
 
-  static assessServiceWorker(artifacts, result) {
+  static assessServiceWorker(artifacts) {
+    const failures = [];
     const hasServiceWorker = SWAudit.audit(artifacts).rawValue;
     if (!hasServiceWorker) {
-      result.failures.push('Site does not register a service worker');
+      failures.push('Site does not register a service worker');
     }
+
+    return failures;
   }
 
-  static assessOfflineStartUrl(artifacts, result) {
+  static assessOfflineStartUrl(artifacts) {
+    const failures = [];
+    const warnings = [];
     const hasOfflineStartUrl = artifacts.StartUrl.statusCode === 200;
 
     if (!hasOfflineStartUrl) {
-      result.failures.push('Service worker does not successfully serve the manifest\'s start_url');
-      if (artifacts.StartUrl.debugString) result.failures.push(artifacts.StartUrl.debugString);
+      failures.push('Service worker does not successfully serve the manifest\'s start_url');
+      if (artifacts.StartUrl.debugString) {
+        failures.push(artifacts.StartUrl.debugString);
+      }
     }
 
     if (artifacts.StartUrl.debugString) {
-      result.warnings.push(artifacts.StartUrl.debugString);
+      warnings.push(artifacts.StartUrl.debugString);
     }
+
+    return {failures, warnings};
   }
 
   static audit_(artifacts) {
-    const failures = [];
-    const warnings = [];
+    let offlineFailures = [];
+    let offlineWarnings = [];
 
     return artifacts.requestManifestValues(artifacts.Manifest).then(manifestValues => {
-      const result = {warnings, failures, manifestValues};
-      WebappInstallBanner.assessManifest(artifacts, result);
-      WebappInstallBanner.assessServiceWorker(artifacts, result);
-      WebappInstallBanner.assessOfflineStartUrl(artifacts, result);
+      const manifestFailures = WebappInstallBanner.assessManifest(manifestValues);
+      const swFailures = WebappInstallBanner.assessServiceWorker(artifacts);
+      if (!swFailures.length) {
+        const {failures, warnings} = WebappInstallBanner.assessOfflineStartUrl(artifacts);
+        offlineFailures = failures;
+        offlineWarnings = warnings;
+      }
 
-      return result;
+      return {
+        warnings: offlineWarnings,
+        failures: [
+          ...manifestFailures,
+          ...swFailures,
+          ...offlineFailures,
+        ],
+        manifestValues,
+      };
     });
   }
 }

diff --git a/lighthouse-core/test/audits/webapp-install-banner-test.js b/lighthouse-core/test/audits/webapp-install-banner-test.js
@@ -134,8 +134,7 @@ describe('PWA: webapp install banner audit', () => {
       assert.strictEqual(result.rawValue, false);
       assert.ok(result.debugString.includes('service worker'), result.debugString);
       const failures = result.extendedInfo.value.failures;
-      // start url will be -1 as well so failures will be 2
-      assert.strictEqual(failures.length, 2, 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
@@ -651,17 +651,13 @@
       "score": 0,
       "displayValue": "",
       "rawValue": false,
-      "debugString": "Failures: No manifest was fetched, Site does not register a service worker, Service worker does not successfully serve the manifest's start_url, No start URL to fetch: No usable web app manifest found on page http://localhost:10200/dobetterweb/dbw_tester.html.",
+      "debugString": "Failures: No manifest was fetched, Site does not register a service worker.",
       "extendedInfo": {
         "value": {
-          "warnings": [
-            "No start URL to fetch: No usable web app manifest found on page http://localhost:10200/dobetterweb/dbw_tester.html"
-          ],
+          "warnings": [],
           "failures": [
             "No manifest was fetched",
-            "Site does not register a service worker",
-            "Service worker does not successfully serve the manifest's start_url",
-            "No start URL to fetch: No usable web app manifest found on page http://localhost:10200/dobetterweb/dbw_tester.html"
+            "Site does not register a service worker"
           ],
           "manifestValues": {
             "isParseFailure": true,