update AnchorsWithNoRelNoopener gatherer to use Errors for errors

GoogleChrome · Feb 4, 2017 · 15b3a9e · 15b3a9e
1 parent 1455455
commit 15b3a9e
Show file tree

Hide file tree

Showing 4 changed files with 26 additions and 49 deletions.
diff --git a/lighthouse-core/audits/dobetterweb/external-anchors-use-rel-noopener.js b/lighthouse-core/audits/dobetterweb/external-anchors-use-rel-noopener.js
@@ -42,20 +42,13 @@ class ExternalAnchorsUseRelNoopenerAudit extends Audit {
    * @return {!AuditResult}
    */
   static audit(artifacts) {
-    if (artifacts.AnchorsWithNoRelNoopener === -1) {
-      return ExternalAnchorsUseRelNoopenerAudit.generateAuditResult({
-        rawValue: -1,
-        debugString: 'Unknown error with the AnchorsWithNoRelNoopener gatherer.'
-      });
-    }
-
     let debugString;
     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:
     // https://github.com/GoogleChrome/lighthouse/issues/1233
     // https://github.com/GoogleChrome/lighthouse/issues/1345
-    const failingAnchors = artifacts.AnchorsWithNoRelNoopener.usages
+    const failingAnchors = artifacts.AnchorsWithNoRelNoopener
       .filter(anchor => {
         try {
           return anchor.href === '' || new URL(anchor.href).host !== pageHost;

diff --git a/lighthouse-core/gather/driver.js b/lighthouse-core/gather/driver.js
@@ -533,7 +533,7 @@ class Driver {
 
   /**
    * @param {string} selector Selector to find in the DOM
-   * @return {!Promise<Element[]>} The found elements, or [], resolved in a promise
+   * @return {!Promise<!Array<!Element>>} The found elements, or [], resolved in a promise
    */
   querySelectorAll(selector) {
     return this.sendCommand('DOM.getDocument')

diff --git a/lighthouse-core/gather/gatherers/dobetterweb/anchors-with-no-rel-noopener.js b/lighthouse-core/gather/gatherers/dobetterweb/anchors-with-no-rel-noopener.js
@@ -19,7 +19,10 @@
 const Gatherer = require('../gatherer');
 
 class AnchorsWithNoRelNoopener extends Gatherer {
-
+  /**
+   * @param {!Object} options
+   * @return {!Promise<!Array<{href: string, rel: string, target: string}>>}
+   */
   afterPass(options) {
     const driver = options.driver;
     return driver.querySelectorAll('a[target="_blank"]:not([rel~="noopener"])')
@@ -34,18 +37,13 @@ class AnchorsWithNoRelNoopener extends Gatherer {
         return Promise.all(failingNodes);
       })
       .then(failingNodes => {
-        return {
-          usages: failingNodes.map(node => {
-            return {
-              href: node[0],
-              rel: node[1],
-              target: node[2]
-            };
-          })
-        };
-      })
-      .catch(_ => {
-        return -1;
+        return failingNodes.map(node => {
+          return {
+            href: node[0],
+            rel: node[1],
+            target: node[2]
+          };
+        });
       });
   }
 }

diff --git a/lighthouse-core/test/audits/dobetterweb/external-anchors-use-rel-noopener-test.js b/lighthouse-core/test/audits/dobetterweb/external-anchors-use-rel-noopener-test.js
@@ -24,22 +24,12 @@ const URL = 'https://google.com/test';
 /* eslint-env mocha */
 
 describe('External anchors use rel="noopener"', () => {
-  it('fails when gatherer failed', () => {
-    const auditResult = ExternalAnchorsAudit.audit({
-      AnchorsWithNoRelNoopener: -1
-    });
-    assert.equal(auditResult.rawValue, -1);
-    assert.ok(auditResult.debugString);
-  });
-
   it('passes when links are from same hosts as the page host', () => {
     const auditResult = ExternalAnchorsAudit.audit({
-      AnchorsWithNoRelNoopener: {
-        usages: [
-          {href: 'https://google.com/test'},
-          {href: 'https://google.com/test1'}
-        ]
-      },
+      AnchorsWithNoRelNoopener: [
+        {href: 'https://google.com/test'},
+        {href: 'https://google.com/test1'}
+      ],
       URL: {finalUrl: URL},
     });
     assert.equal(auditResult.rawValue, true);
@@ -48,12 +38,10 @@ describe('External anchors use rel="noopener"', () => {
 
   it('fails when links are from different hosts than the page host', () => {
     const auditResult = ExternalAnchorsAudit.audit({
-      AnchorsWithNoRelNoopener: {
-        usages: [
-          {href: 'https://example.com/test'},
-          {href: 'https://example.com/test1'}
-        ]
-      },
+      AnchorsWithNoRelNoopener: [
+        {href: 'https://example.com/test'},
+        {href: 'https://example.com/test1'}
+      ],
       URL: {finalUrl: URL},
     });
     assert.equal(auditResult.rawValue, false);
@@ -62,13 +50,11 @@ describe('External anchors use rel="noopener"', () => {
 
   it('handles links with no href attribute', () => {
     const auditResult = ExternalAnchorsAudit.audit({
-      AnchorsWithNoRelNoopener: {
-        usages: [
-          {href: ''},
-          {href: 'http://'},
-          {href: 'http:'}
-        ]
-      },
+      AnchorsWithNoRelNoopener: [
+        {href: ''},
+        {href: 'http://'},
+        {href: 'http:'}
+      ],
       URL: {finalUrl: URL},
     });
     assert.equal(auditResult.rawValue, false);