move SW version test to audit to use possibly redirected URL

lighthouse-cli/test/fixtures/smoketest-offline-config.json

@@ -2,7 +2,8 @@
   "passes": [{
     "loadPage": true,
     "gatherers": [
-      "viewport"
+      "viewport",
+      "url"
     ]
   },{
     "loadPage": true,

lighthouse-core/audits/service-worker.js

@@ -17,8 +17,29 @@
 
 'use strict';
 
+const url = require('url');
 const Audit = require('./audit');
 
+/**
+ * @param {string} targetURL
+ * @return {string}
+ */
+function getOrigin(targetURL) {
+  const parsedURL = url.parse(targetURL);
+  return `${parsedURL.protocol}//${parsedURL.hostname}` +
+      (parsedURL.port ? `:${parsedURL.port}` : '');
+}
+
+/**
+ * @param {!Array<!ServiceWorkerVersion>} versions
+ * @param {string} url
+ * @return {(!ServiceWorkerVersion|undefined)}
+ */
+function getActivatedServiceWorker(versions, url) {
+  const origin = getOrigin(url);
+  return versions.find(v => v.status === 'activated' && getOrigin(v.scriptURL) === origin);
+}
+
 class ServiceWorker extends Audit {
   /**
    * @return {!AuditMeta}
@@ -28,7 +49,7 @@ class ServiceWorker extends Audit {
       category: 'Offline',
       name: 'service-worker',
       description: 'Has a registered Service Worker',
-      requiredArtifacts: ['ServiceWorker']
+      requiredArtifacts: ['URL', 'ServiceWorker']
     };
   }
 
@@ -37,9 +58,22 @@ class ServiceWorker extends Audit {
    * @return {!AuditResult}
    */
   static audit(artifacts) {
+    if (!artifacts.ServiceWorker.versions) {
+      // Error in ServiceWorker gatherer.
+      return ServiceWorker.generateAuditResult({
+        rawValue: false,
+        debugString: artifacts.ServiceWorker.debugString
+      });
+    }
+
+    // Find active service worker for this URL. Match against artifacts.URL so
+    // audit accounts for any redirects.
+    const version = getActivatedServiceWorker(artifacts.ServiceWorker.versions, artifacts.URL);
+    const debugString = version ? undefined : 'No active service worker found for this origin.';
+
     return ServiceWorker.generateAuditResult({
-      rawValue: !!artifacts.ServiceWorker.version,
-      debugString: artifacts.ServiceWorker.debugString
+      rawValue: !!version,
+      debugString: debugString
     });
   }
 }

lighthouse-core/closure/typedefs/Audit.js

@@ -39,4 +39,4 @@ AuditMeta.prototype.description;
 AuditMeta.prototype.optimalValue;
 
 /** @type {!Array<string>} */
-AuditMeta.prototype.requiredArtifacts
+AuditMeta.prototype.requiredArtifacts;

lighthouse-core/closure/typedefs/ServiceWorkerArtifact.js

@@ -31,8 +31,8 @@ function ServiceWorkerArtifact() {}
 /** @type {(string|undefined)} */
 ServiceWorkerArtifact.prototype.debugString;
 
-/** @type {(!ServiceWorkerVersion|undefined)} */
-ServiceWorkerArtifact.prototype.version;
+/** @type {(!Array<!ServiceWorkerVersion>|undefined)} */
+ServiceWorkerArtifact.prototype.versions;
 
 /**
  * @struct

lighthouse-core/gather/gatherers/service-worker.js

@@ -19,37 +19,13 @@
 const Gatherer = require('./gatherer');
 
 class ServiceWorker extends Gatherer {
-
-  /**
-   * @param {string} url
-   * @return {string}
-   */
-  static getOrigin(url) {
-    const parsedURL = require('url').parse(url);
-    return `${parsedURL.protocol}//${parsedURL.hostname}` +
-        (parsedURL.port ? `:${parsedURL.port}` : '');
-  }
-
-  /**
-   * @param {!Array<!ServiceWorkerVersion>} versions
-   * @param {string} url
-   * @return {(!ServiceWorkerVersion|undefined)}
-   */
-  static getActivatedServiceWorker(versions, url) {
-    const origin = this.getOrigin(url);
-    return versions.find(v => v.status === 'activated' && this.getOrigin(v.scriptURL) === origin);
-  }
-
   beforePass(options) {
     const driver = options.driver;
     return driver
       .getServiceWorkerVersions()
       .then(data => {
-        const version = ServiceWorker.getActivatedServiceWorker(data.versions, options.url);
-        const debugString = version ? undefined : 'No active service worker found for this origin.';
         return {
-          version,
-          debugString
+          versions: data.versions
         };
       })
       .catch(err => {

lighthouse-core/test/audits/service-worker-test.js

@@ -21,26 +21,50 @@ const Audit = require('../../audits/service-worker.js');
 const assert = require('assert');
 
 describe('Offline: Service Worker audit', () => {
-  it('creates an output when given no Service Worker version', () => {
+  it('reports driver error when given no Service Worker versions', () => {
     const debugString = 'Error string';
     const output = Audit.audit({
       ServiceWorker: {
-        version: undefined,
         debugString
       }
     });
 
     assert.equal(output.score, false);
+    assert.equal(output.rawValue, false);
     assert.equal(output.debugString, debugString);
   });
 
-  it('creates an output when given an array of versions', () => {
+  it('passes when given a matching Service Worker version', () => {
     const output = Audit.audit({
       ServiceWorker: {
-        version: {}
-      }
+        versions: [{
+          status: 'activated',
+          scriptURL: 'https://example.com/sw.js'
+        }]
+      },
+      URL: 'https://example.com'
     });
 
-    return assert.equal(output.score, true);
+    assert.equal(output.score, true);
+    assert.equal(output.rawValue, true);
+  });
+
+  it('discards service worker registrations for other origins', () => {
+    const url = 'https://example.com/';
+    const versions = [{
+      status: 'activated',
+      scriptURL: 'https://other-example.com'
+    }];
+
+    const output = Audit.audit({
+      ServiceWorker: {
+        versions
+      },
+      URL: url
+    });
+
+    assert.equal(output.score, false);
+    assert.equal(output.rawValue, false);
+    assert.ok(output.debugString);
   });
 });

lighthouse-core/test/gather/gatherers/service-worker-test.js

@@ -42,31 +42,7 @@ describe('Service Worker gatherer', () => {
       },
       url
     }).then(_ => {
-      assert(serviceWorkerGatherer.artifact.version);
-      assert.deepEqual(serviceWorkerGatherer.artifact.version, {
-        status: 'activated',
-        scriptURL: url
-      });
-    });
-  });
-
-  it('discards service worker registrations for other origins', () => {
-    const url = 'https://example.com/';
-    const versions = [{
-      status: 'activated',
-      scriptURL: 'https://other-example.com'
-    }];
-
-    return serviceWorkerGatherer.beforePass({
-      driver: {
-        getServiceWorkerVersions() {
-          return Promise.resolve({versions});
-        }
-      },
-      url
-    }).then(_ => {
-      assert.ok(!serviceWorkerGatherer.artifact.version);
-      assert.ok(serviceWorkerGatherer.artifact.debugString);
+      assert.deepEqual(serviceWorkerGatherer.artifact.versions, versions);
     });
   });
 
@@ -78,7 +54,7 @@ describe('Service Worker gatherer', () => {
         }
       }
     }).then(_ => {
-      assert.ok(!serviceWorkerGatherer.artifact.version);
+      assert.ok(!serviceWorkerGatherer.artifact.versions);
       assert.ok(serviceWorkerGatherer.artifact.debugString);
     });
   });