From 5329b6255912b9788d9f367b6bebea1cb79ca725 Mon Sep 17 00:00:00 2001
From: Patrick Hulce <patrick.hulce@gmail.com>
Date: Tue, 15 Oct 2019 11:09:04 -0500
Subject: [PATCH] feat(utils): diff finder use non-url item keys

---
 packages/utils/src/audit-diff-finder.js       | 15 ++++
 packages/utils/test/audit-diff-finder.test.js | 75 +++++++++++++++++++
 2 files changed, 90 insertions(+)

diff --git a/packages/utils/src/audit-diff-finder.js b/packages/utils/src/audit-diff-finder.js
index 43a7d258e..63c687752 100644
--- a/packages/utils/src/audit-diff-finder.js
+++ b/packages/utils/src/audit-diff-finder.js
@@ -259,6 +259,21 @@ function replaceNondeterministicStrings(s) {
  * @return {Array<{base?: DetailItemEntry, compare?: DetailItemEntry}>}
  */
 function zipBaseAndCompareItems(baseItems, compareItems) {
+  /** @param {Record<string, any>} item */
+  const getItemKey = item => {
+    // For most opportunities, diagnostics, etc where 1 row === 1 resource
+    if (typeof item.url === 'string') return item.url;
+    // For the pre-grouped audits like resource-summary
+    if (typeof item.label === 'string') return item.label;
+    // For dom-size
+    if (typeof item.statistic === 'string') return item.statistic;
+    // For third-party-summary
+    if (item.entity && typeof item.entity.text === 'string') return item.entity.text;
+
+    // For everything else, use the entire object, actually works OK on most nodes.
+    return JSON.stringify(item);
+  };
+
   const groupedByKey = _.groupIntoMap(
     [
       ...baseItems.map((item, i) => ({item, kind: 'base', index: i})),
diff --git a/packages/utils/test/audit-diff-finder.test.js b/packages/utils/test/audit-diff-finder.test.js
index 731769fa3..c2a71ca70 100644
--- a/packages/utils/test/audit-diff-finder.test.js
+++ b/packages/utils/test/audit-diff-finder.test.js
@@ -519,6 +519,81 @@ describe('#getRowLabel', () => {
   });
 });
 
+describe('#zipBaseAndCompareItems', () => {
+  it('should zip URLs', () => {
+    const base = [{url: 'https://localhost/resource.js', time: 1}];
+    const compare = [{url: 'https://localhost/resource.js', time: 2}];
+    const zipped = zipBaseAndCompareItems(base, compare);
+    expect(zipped).toEqual([
+      {
+        base: {item: base[0], kind: 'base', index: 0},
+        compare: {item: compare[0], kind: 'compare', index: 0},
+      },
+    ]);
+  });
+
+  it('should zip labels', () => {
+    const base = [{label: 'Documents', requestCount: 1}];
+    const compare = [{label: 'Documents', requestCount: 2}];
+    const zipped = zipBaseAndCompareItems(base, compare);
+    expect(zipped).toEqual([
+      {
+        base: {item: base[0], kind: 'base', index: 0},
+        compare: {item: compare[0], kind: 'compare', index: 0},
+      },
+    ]);
+  });
+
+  it('should zip nodes', () => {
+    const base = [{node: {snippet: '<a>Text</a>'}, count: 1}];
+    const compare = [{node: {snippet: '<a>Text</a>'}, count: 1}];
+    const zipped = zipBaseAndCompareItems(base, compare);
+    expect(zipped).toEqual([
+      {
+        base: {item: base[0], kind: 'base', index: 0},
+        compare: {item: compare[0], kind: 'compare', index: 0},
+      },
+    ]);
+  });
+
+  it('should zip multiple items', () => {
+    const base = [
+      {missingInCompare: true},
+      {label: 'Documents', requestCount: 1},
+      {node: {snippet: '<a>Text</a>'}, count: 1},
+      {url: 'https://localhost/resource.12345678.js', time: 1},
+    ];
+    const compare = [
+      {url: 'https://localhost/resource.87654321.js', time: 2},
+      {label: 'Documents', requestCount: 2},
+      {node: {snippet: '<a>Text</a>'}, count: 1},
+      {missingInBase: true},
+    ];
+
+    const zipped = zipBaseAndCompareItems(base, compare);
+    expect(zipped).toEqual([
+      {
+        base: {item: base[0], kind: 'base', index: 0},
+      },
+      {
+        base: {item: base[1], kind: 'base', index: 1},
+        compare: {item: compare[1], kind: 'compare', index: 1},
+      },
+      {
+        base: {item: base[2], kind: 'base', index: 2},
+        compare: {item: compare[2], kind: 'compare', index: 2},
+      },
+      {
+        base: {item: base[3], kind: 'base', index: 3},
+        compare: {item: compare[0], kind: 'compare', index: 0},
+      },
+      {
+        compare: {item: compare[3], kind: 'compare', index: 3},
+      },
+    ]);
+  });
+});
+
 describe('#replaceNondeterministicStrings', () => {
   it('should work on non-replacements', () => {
     expect(replaceNondeterministicStrings('nonsense')).toEqual('nonsense');