polish: show audits with debug string, don't fail loadfast4pwa on net…

…work latencies, works-offline change (#2294)

* polish: show audits with debug string

* more precise offline message

* add back start time check

* remove verify statement

* feedback

* unit tests

* merge fix

lighthouse-core/audits/load-fast-enough-for-pwa.js

@@ -43,7 +43,7 @@ class LoadFastEnough4Pwa extends Audit {
       category: 'PWA',
       name: 'load-fast-enough-for-pwa',
       description: 'Page load is fast enough on 3G',
-      helpText: 'Satisfied if the Time To Interactive duration is shorter than 10 seconds, as defined by the [PWA Baseline Checklist](https://developers.google.com/web/progressive-web-apps/checklist). Network throttling is required (specifically: RTT latencies >= 150 RTT are expected).',
+      helpText: 'Satisfied if First Interactive is less than 10 seconds, as defined by the [PWA Baseline Checklist](https://developers.google.com/web/progressive-web-apps/checklist). Network throttling is required (specifically: RTT latencies >= 150 RTT are expected).',
       requiredArtifacts: ['traces', 'devtoolsLogs']
     };
   }
@@ -112,22 +112,22 @@ class LoadFastEnough4Pwa extends Audit {
           {key: 'latency', itemType: 'text', text: 'Latency (ms)'},
         ], firstRequestLatencies);
 
-        if (!areLatenciesAll3G) {
+        if (!isFast) {
           return {
             rawValue: false,
             // eslint-disable-next-line max-len
-            debugString: `First Interactive was found at ${timeToFirstInteractive.toLocaleString()}, however, the network request latencies were not sufficiently realistic, so the performance measurements cannot be trusted.`,
-            extendedInfo,
-            details
+            debugString: `First Interactive was at ${timeToFirstInteractive.toLocaleString()}. More details in the "Performance" section.`,
+            extendedInfo
           };
         }
 
-        if (!isFast) {
+        if (!areLatenciesAll3G) {
           return {
-            rawValue: false,
+            rawValue: true,
             // eslint-disable-next-line max-len
-            debugString: `Under 3G conditions, First Interactive was at ${timeToFirstInteractive.toLocaleString()}. More details in the "Performance" section.`,
-            extendedInfo
+            debugString: `First Interactive was found at ${timeToFirstInteractive.toLocaleString()}, however, the network request latencies were not sufficiently realistic, so the performance measurements cannot be trusted.`,
+            extendedInfo,
+            details
           };
         }
 

lighthouse-core/audits/works-offline.js

@@ -41,14 +41,16 @@ class WorksOffline extends Audit {
    */
   static audit(artifacts) {
     let debugString;
-    if (!URL.equalWithExcludedFragments(artifacts.URL.initialUrl, artifacts.URL.finalUrl)) {
+    const passed = artifacts.Offline === 200;
+    if (!passed &&
+        !URL.equalWithExcludedFragments(artifacts.URL.initialUrl, artifacts.URL.finalUrl)) {
       debugString = 'WARNING: You may be failing this check because your test URL ' +
           `(${artifacts.URL.initialUrl}) was redirected to "${artifacts.URL.finalUrl}". ` +
           'Try testing the second URL directly.';
     }
 
     return {
-      rawValue: artifacts.Offline === 200,
+      rawValue: passed,
       debugString
     };
   }

lighthouse-core/report/v2/renderer/category-renderer.js

@@ -344,7 +344,8 @@ class CategoryRenderer {
 
     const manualAudits = category.audits.filter(audit => audit.result.manual);
     const nonManualAudits = category.audits.filter(audit => !manualAudits.includes(audit));
-    const passedAudits = nonManualAudits.filter(audit => audit.score === 100);
+    const passedAudits = nonManualAudits.filter(audit => audit.score === 100 &&
+        !audit.result.debugString);
     const nonPassedAudits = nonManualAudits.filter(audit => !passedAudits.includes(audit));
 
     for (const audit of nonPassedAudits) {

lighthouse-core/test/audits/load-fast-enough-for-pwa-test.js

@@ -53,7 +53,7 @@ describe('PWA: load-fast-enough-for-pwa audit', () => {
     });
   });
 
-  it('fails a good TTI value with no throttling', () => {
+  it('warns on a good TTI value with no throttling', () => {
     // latencies are very short
     const mockNetworkRecords = [
       {_timing: {sendEnd: 0, receiveHeadersEnd: 50}, finished: true, _url: 'https://google.com/'},
@@ -62,7 +62,7 @@ describe('PWA: load-fast-enough-for-pwa audit', () => {
       {_timing: {sendEnd: 0, receiveHeadersEnd: 50}, finished: true, _url: 'https://google.com/b'},
     ];
     return FastPWAAudit.audit(generateArtifacts(5000, mockNetworkRecords)).then(result => {
-      assert.equal(result.rawValue, false);
+      assert.equal(result.rawValue, true);
       assert.ok(result.debugString.includes('network request latencies'));
       assert.ok(result.details, 'contains details when latencies were not realistic');
     });

lighthouse-core/test/audits/works-offline-test.js

@@ -35,11 +35,11 @@ describe('Offline: works-offline audit', () => {
 
   it('warns if initial url does not match final url', () => {
     const output = Audit.audit({
-      Offline: 200,
+      Offline: 404,
       URL: {initialUrl: URL, finalUrl: `${URL}/features`}
     });
 
-    assert.equal(output.rawValue, true);
+    assert.equal(output.rawValue, false);
     assert.ok(output.debugString);
   });
 

lighthouse-core/test/report/v2/renderer/category-renderer-test.js

@@ -110,6 +110,20 @@ describe('CategoryRenderer', () => {
     assert.equal(audits.length, category.audits.length, 'renders correct number of audits');
   });
 
+  it('renders audits with debugString as failed', () => {
+    const auditResult = {
+      description: 'Audit',
+      helpText: 'Learn more',
+      debugString: 'It may not have worked!',
+      score: 100,
+    };
+    const audit = {result: auditResult, score: 100};
+    const category = {name: 'Fake', description: '', score: 100, audits: [audit]};
+    const categoryDOM = renderer.render(category, sampleResults.reportGroups);
+    assert.ok(categoryDOM.querySelector('.lh-category > .lh-audit'), 'did not render as failed');
+    assert.ok(categoryDOM.querySelector('.lh-debug'), 'did not render debug message');
+  });
+
   it('renders manual audits if the category contains them', () => {
     const pwaCategory = sampleResults.reportCategories.find(cat => cat.id === 'pwa');
     const categoryDOM = renderer.render(pwaCategory, sampleResults.reportGroups);