add test case and nits

GoogleChrome · Jul 19, 2023 · cb73c5b · cb73c5b
1 parent 9a2b5d8
commit cb73c5b
Show file tree

Hide file tree

Showing 2 changed files with 55 additions and 14 deletions.
diff --git a/core/audits/uses-rel-preconnect.js b/core/audits/uses-rel-preconnect.js
@@ -126,8 +126,8 @@ class UsesRelPreconnectAudit extends Audit {
     const devtoolsLog = artifacts.devtoolsLogs[UsesRelPreconnectAudit.DEFAULT_PASS];
     const settings = context.settings;
 
-    let maxWasted = 0;
-    let maxWastedFCP = 0;
+    let maxWastedLcp = 0;
+    let maxWastedFcp = 0;
     /** @type {Array<LH.IcuMessage>} */
     const warnings = [];
 
@@ -225,10 +225,10 @@ class UsesRelPreconnectAudit extends Audit {
         return;
       }
 
-      maxWasted = Math.max(wastedMs, maxWasted);
+      maxWastedLcp = Math.max(wastedMs, maxWastedLcp);
 
       if (fcpGraphURLs.has(firstRecordOfOrigin.url)) {
-        maxWastedFCP = Math.max(wastedMs, maxWasted);
+        maxWastedFcp = Math.max(wastedMs, maxWastedLcp);
       }
       results.push({
         url: securityOrigin,
@@ -253,7 +253,7 @@ class UsesRelPreconnectAudit extends Audit {
         score: 1,
         warnings: preconnectLinks.length >= 3 ?
           [...warnings, str_(UIStrings.tooManyPreconnectLinksWarning)] : warnings,
-        metricSavings: {LCP: maxWasted, FCP: maxWastedFCP},
+        metricSavings: {LCP: maxWastedLcp, FCP: maxWastedFcp},
       };
     }
 
@@ -264,18 +264,18 @@ class UsesRelPreconnectAudit extends Audit {
     ];
 
     const details = Audit.makeOpportunityDetails(headings, results,
-      {overallSavingsMs: maxWasted, sortedBy: ['wastedMs']});
+      {overallSavingsMs: maxWastedLcp, sortedBy: ['wastedMs']});
 
     return {
-      score: ByteEfficiencyAudit.scoreForWastedMs(maxWasted),
-      numericValue: maxWasted,
+      score: ByteEfficiencyAudit.scoreForWastedMs(maxWastedLcp),
+      numericValue: maxWastedLcp,
       numericUnit: 'millisecond',
-      displayValue: maxWasted ?
-        str_(i18n.UIStrings.displayValueMsSavings, {wastedMs: maxWasted}) :
+      displayValue: maxWastedLcp ?
+        str_(i18n.UIStrings.displayValueMsSavings, {wastedMs: maxWastedLcp}) :
         '',
       warnings,
       details,
-      metricSavings: {LCP: maxWasted, FCP: maxWastedFCP},
+      metricSavings: {LCP: maxWastedLcp, FCP: maxWastedFcp},
     };
   }
 }

diff --git a/core/test/audits/uses-rel-preconnect-test.js b/core/test/audits/uses-rel-preconnect-test.js
@@ -17,11 +17,11 @@ const mainResource = {
   priority: 'High',
 };
 
-function buildArtifacts(networkRecords) {
+function buildArtifacts(networkRecords, fcpTs) {
   const trace = createTestTrace({
     timeOrigin: 0,
     largestContentfulPaint: 5000,
-    firstContentfulPaint: 5000,
+    firstContentfulPaint: fcpTs ? fcpTs : 5000,
     topLevelTasks: [{ts: 1000, duration: 50}],
   });
   const devtoolsLog = networkRecordsToDevtoolsLog(networkRecords);
@@ -197,7 +197,8 @@ describe('Performance: uses-rel-preconnect audit', () => {
 
     const artifacts = buildArtifacts(networkRecords);
     const context = {settings: {}, computedCache: new Map()};
-    const {numericValue, details, metricSavings} = await UsesRelPreconnect.audit(artifacts, context);
+    const {numericValue, details, metricSavings} =
+      await UsesRelPreconnect.audit(artifacts, context);
     assert.equal(numericValue, 300);
     assert.equal(details.items.length, 1);
     assert.deepStrictEqual(details.items, [
@@ -353,4 +354,44 @@ describe('Performance: uses-rel-preconnect audit', () => {
     assert.equal(result.score, 1);
     assert.equal(result.warnings.length, 1);
   });
+
+  it(`should have LCP savings and not FCP savings `, async () => {
+    const networkRecords = [
+      mainResource,
+      {
+        url: 'https://cdn.example.com/first',
+        initiator: {},
+        networkRequestTime: 3000,
+        timing: {
+          dnsStart: 300,
+          connectStart: 350,
+          connectEnd: 400,
+        },
+        priority: 'High',
+      },
+      {
+        url: 'https://cdn.example.com/second',
+        initiator: {},
+        networkRequestTime: 4000,
+        networkEndTime: 4500, // ends *before* LCP
+        timing: {
+          dnsStart: 100,
+          connectStart: 200,
+          connectEnd: 300,
+        },
+        priority: 'High',
+      },
+    ];
+
+    const artifacts = buildArtifacts(networkRecords, 4000);
+    const context = {settings: {}, computedCache: new Map()};
+    const {numericValue, details, metricSavings} =
+      await UsesRelPreconnect.audit(artifacts, context);
+    assert.equal(numericValue, 300);
+    assert.equal(details.items.length, 1);
+    assert.deepStrictEqual(details.items, [
+      {url: 'https://cdn.example.com', wastedMs: 300},
+    ]);
+    assert.deepStrictEqual(metricSavings, {LCP: 300, FCP: 0});
+  });
 });