Move date.now and console.time audits to tale formatter

GoogleChrome · Jan 26, 2017 · 112c22a · 112c22a
1 parent e9e0e8e
commit 112c22a
Show file tree

Hide file tree

Showing 7 changed files with 52 additions and 31 deletions.
diff --git a/lighthouse-cli/test/smokehouse/dobetterweb/dbw-expectations.js b/lighthouse-cli/test/smokehouse/dobetterweb/dbw-expectations.js
@@ -64,15 +64,19 @@ module.exports = [
         score: false,
         extendedInfo: {
           value: {
-            length: 3
+            results: {
+              length: 3
+            }
           }
         }
       },
       'no-datenow': {
         score: false,
         extendedInfo: {
           value: {
-            length: 5
+            results: {
+              length: 5
+            }
           }
         }
       },

diff --git a/lighthouse-core/audits/dobetterweb/no-console-time.js b/lighthouse-core/audits/dobetterweb/no-console-time.js
@@ -75,17 +75,16 @@ class NoConsoleTimeAudit extends Audit {
       // warn the user that we don't know what the callsite is.
       debugString = URL.INVALID_URL_DEBUG_STRING;
       return true;
-    }).map(err => {
-      return Object.assign({
-        label: `line: ${err.line}, col: ${err.col}`
-      }, err);
     });
 
     return NoConsoleTimeAudit.generateAuditResult({
       rawValue: results.length === 0,
       extendedInfo: {
-        formatter: Formatter.SUPPORTED_FORMATS.URLLIST,
-        value: results
+        formatter: Formatter.SUPPORTED_FORMATS.TABLE,
+        value: {
+          results,
+          tableHeadings: {url: 'URL', lineCol: 'Line/Col', isEval: 'Eval\'d?'}
+        }
       },
       debugString
     });

diff --git a/lighthouse-core/audits/dobetterweb/no-datenow.js b/lighthouse-core/audits/dobetterweb/no-datenow.js
@@ -75,18 +75,16 @@ class NoDateNowAudit extends Audit {
       // warn the user that we don't know what the callsite is.
       debugString = URL.INVALID_URL_DEBUG_STRING;
       return true;
-    }).map(err => {
-      return Object.assign({
-        label: `line: ${err.line}, col: ${err.col}`,
-        url: err.url
-      }, err);
     });
 
     return NoDateNowAudit.generateAuditResult({
       rawValue: results.length === 0,
       extendedInfo: {
-        formatter: Formatter.SUPPORTED_FORMATS.URLLIST,
-        value: results
+        formatter: Formatter.SUPPORTED_FORMATS.TABLE,
+        value: {
+          results,
+          tableHeadings: {url: 'URL', lineCol: 'Line/Col', isEval: 'Eval\'d?'}
+        }
       },
       debugString
     });

diff --git a/lighthouse-core/formatters/table.js b/lighthouse-core/formatters/table.js
@@ -34,6 +34,11 @@ class Table extends Formatter {
           }
 
           const table = Table.createTable(result.tableHeadings, result.results);
+          const headings = Object.keys(result.tableHeadings).map(key => {
+            return result.tableHeadings[key].toUpperCase();
+          });
+
+          output += `      ${headings.join(' ')}\n`;
 
           table.rows.forEach(row => {
             output += '      ';
@@ -78,13 +83,16 @@ class Table extends Formatter {
 
     const rows = results.map(result => {
       const cols = headingKeys.map(key => {
+        const val = result[key];
         switch (key) {
           case 'code':
-            return '`' + result[key].trim() + '`';
+            return '`' + val.trim() + '`';
           case 'lineCol':
             return `${result.line}:${result.col}`;
+          case 'isEval':
+            return val ? 'yes' : '';
           default:
-            return result[key];
+            return val;
         }
       });
 

diff --git a/lighthouse-core/test/audits/dobetterweb/no-console-time-test.js b/lighthouse-core/test/audits/dobetterweb/no-console-time-test.js
@@ -38,7 +38,7 @@ describe('Page does not use console.time()', () => {
       URL: {finalUrl: URL}
     });
     assert.equal(auditResult.rawValue, true);
-    assert.equal(auditResult.extendedInfo.value.length, 0);
+    assert.equal(auditResult.extendedInfo.value.results.length, 0);
   });
 
   it('passes when console.time() is used on a different origin', () => {
@@ -52,7 +52,7 @@ describe('Page does not use console.time()', () => {
       URL: {finalUrl: URL}
     });
     assert.equal(auditResult.rawValue, true);
-    assert.equal(auditResult.extendedInfo.value.length, 0);
+    assert.equal(auditResult.extendedInfo.value.results.length, 0);
   });
 
   it('fails when console.time() is used on the origin', () => {
@@ -67,7 +67,11 @@ describe('Page does not use console.time()', () => {
       URL: {finalUrl: URL}
     });
     assert.equal(auditResult.rawValue, false);
-    assert.equal(auditResult.extendedInfo.value.length, 2);
+    assert.equal(auditResult.extendedInfo.value.results.length, 2);
+
+    const headings = auditResult.extendedInfo.value.tableHeadings;
+    assert.deepEqual(Object.keys(headings).map(key => headings[key]),
+                     ['URL', 'Line/Col', 'Eval\'d?'], 'table headings are correct and in order');
   });
 
   it('fails when console.time() is used in eval()', () => {
@@ -82,7 +86,7 @@ describe('Page does not use console.time()', () => {
       URL: {finalUrl: URL}
     });
     assert.equal(auditResult.rawValue, false);
-    assert.equal(auditResult.extendedInfo.value.length, 3);
+    assert.equal(auditResult.extendedInfo.value.results.length, 3);
   });
 
   it('includes results when there is no .url', () => {
@@ -97,7 +101,7 @@ describe('Page does not use console.time()', () => {
     });
 
     assert.equal(auditResult.rawValue, false);
-    assert.equal(auditResult.extendedInfo.value.length, 2);
+    assert.equal(auditResult.extendedInfo.value.results.length, 2);
     assert.ok(auditResult.debugString, 'includes debugString');
   });
 });
diff --git a/lighthouse-core/test/audits/dobetterweb/no-datenow-test.js b/lighthouse-core/test/audits/dobetterweb/no-datenow-test.js
@@ -38,7 +38,7 @@ describe('Page does not use Date.now()', () => {
       URL: {finalUrl: URL},
     });
     assert.equal(auditResult.rawValue, true);
-    assert.equal(auditResult.extendedInfo.value.length, 0);
+    assert.equal(auditResult.extendedInfo.value.results.length, 0);
   });
 
   it('passes when Date.now() is used on a different origin', () => {
@@ -52,7 +52,7 @@ describe('Page does not use Date.now()', () => {
       URL: {finalUrl: URL},
     });
     assert.equal(auditResult.rawValue, true);
-    assert.equal(auditResult.extendedInfo.value.length, 0);
+    assert.equal(auditResult.extendedInfo.value.results.length, 0);
   });
 
   it('fails when Date.now() is used on the origin', () => {
@@ -67,7 +67,11 @@ describe('Page does not use Date.now()', () => {
       URL: {finalUrl: URL},
     });
     assert.equal(auditResult.rawValue, false);
-    assert.equal(auditResult.extendedInfo.value.length, 2);
+    assert.equal(auditResult.extendedInfo.value.results.length, 2);
+
+    const headings = auditResult.extendedInfo.value.tableHeadings;
+    assert.deepEqual(Object.keys(headings).map(key => headings[key]),
+                     ['URL', 'Line/Col', 'Eval\'d?'], 'table headings are correct and in order');
   });
 
   it('only passes when has url property', () => {
@@ -82,7 +86,7 @@ describe('Page does not use Date.now()', () => {
     });
 
     assert.equal(auditResult.rawValue, false);
-    assert.equal(auditResult.extendedInfo.value.length, 1);
+    assert.equal(auditResult.extendedInfo.value.results.length, 1);
   });
 
   it('fails when console.time() is used in eval()', () => {
@@ -97,7 +101,7 @@ describe('Page does not use Date.now()', () => {
       URL: {finalUrl: URL}
     });
     assert.equal(auditResult.rawValue, false);
-    assert.equal(auditResult.extendedInfo.value.length, 3);
+    assert.equal(auditResult.extendedInfo.value.results.length, 3);
   });
 
   it('includes results when there is no .url', () => {
@@ -112,7 +116,7 @@ describe('Page does not use Date.now()', () => {
     });
 
     assert.equal(auditResult.rawValue, false);
-    assert.equal(auditResult.extendedInfo.value.length, 2);
+    assert.equal(auditResult.extendedInfo.value.results.length, 2);
     assert.ok(auditResult.debugString, 'includes debugString');
   });
 });
diff --git a/lighthouse-core/test/formatter/table-formatter-test.js b/lighthouse-core/test/formatter/table-formatter-test.js
@@ -24,12 +24,13 @@ const assert = require('assert');
 
 describe('TableFormatter', () => {
   const extendedInfo = {
-    tableHeadings: {url: 'URL', lineCol: 'Line/col', code: 'Snippet'},
+    tableHeadings: {url: 'URL', lineCol: 'Line/col', code: 'Snippet', isEval: 'Eval\'d?'},
     results: [{
       url: 'http://example.com',
       line: 123,
       col: 456,
-      code: 'code snippet'
+      code: 'code snippet',
+      isEval: true
     }]
   };
 
@@ -50,11 +51,14 @@ describe('TableFormatter', () => {
     assert.equal(table.rows[0].cols[0], 'http://example.com');
     assert.equal(table.rows[0].cols[1], '123:456');
     assert.equal(table.rows[0].cols[2], '\`code snippet\`');
+    assert.equal(table.rows[0].cols[3], 'yes');
   });
 
   it('generates valid pretty output', () => {
     const pretty = TableFormatter.getFormatter('pretty');
-    assert.equal(pretty(extendedInfo), '      http://example.com 123:456 \n');
+    const output = pretty(extendedInfo);
+    assert.ok(output.includes('      URL LINE/COL SNIPPET EVAL\'D?\n'), 'prints table headings');
+    assert.ok(output.includes('      http://example.com 123:456 yes \n'), 'prints cells');
   });
 
   it('generates valid html output', () => {