audit diff: use good keys for nodes and taptargets (#883)

CONTRIBUTING.md

@@ -46,7 +46,21 @@ yarn install # run in the repo root
 
 When working on the CLI, simply make your changes to `packages/cli` or `packages/utils` files and run `yarn start <LHCI arguments here>`. No build step necessary.
 
-When working on the server, you'll need to start build in watch mode. In one terminal run `yarn build:watch` from the repo root. In another terminal run `yarn start:server`. If you made any changes to the APIs of the server, you will need to restart the `yarn start:server` command, but UI changes should be picked up on refresh without restarting the server process. Once the server is up and running you can fill it with some test data by running `yarn start:seed-database` in another terminal.
+When working on the server, you'll need to build. `yarn build` in root will build the server and viewer. Also, you can use watch mode.
+
+```sh
+yarn build:watch
+yarn start:server
+```
+
+If you made any changes to the APIs of the server, you will need to restart the `yarn start:server` command, but UI changes should be picked up on refresh without restarting the server process. Once the server is up and running you can fill it with some test data with `yarn start:seed-database`.
+
+## Problems?
+
+```sh
+trash node_modules && yarn install
+yarn build
+```
 
 ## Updating the Lighthouse Version
 

packages/server/test/test-utils.js

@@ -279,6 +279,9 @@ module.exports = {
  // FIXME: we're more forgiving in CI where font rendering creates small changes.,
  // failureThreshold: process.env.CI ? 0.005 : 0.001,
  failureThresholdType: 'percent',
+ // On -u dont update files that are already fine. This should match the default but...
+ // we've seen behavior which seemingly conflicts.
+ updatePassedSnapshot: false,
  });
 
  expect.extend({toMatchImageSnapshot});

packages/server/test/ui/storybook.test.jsx

@@ -25,6 +25,8 @@ if (process.env.CI && require('os').platform() !== 'darwin') {
 initStoryshots({
  configPath: path.join(__dirname, '../../.storybook'),
  suite: 'Image Storyshots',
+ // Use a storyKindRegex as an `.only`-like filter on the "Image Storyshots" tests.
+ // storyKindRegex: /Graph/,
  test: imageSnapshot({
  storybookUrl: `http://localhost:${process.env.STORYBOOK_PORT}`,
  beforeScreenshot: async page => {

packages/utils/src/audit-diff-finder.js

@@ -384,9 +384,8 @@ function deepPruneItemForKeySerialization(item) {
 
 /** @param {Record<string, any>} item @return {string} */
 function getItemKey(item) {
- // For most opportunities, diagnostics, etc where 1 row === 1 resource
- if (typeof item.url === 'string' && item.url) return item.url;
- if (typeof item.origin === 'string' && item.origin) return item.origin;
+ // Do most specific checks at the top. most general at bottom..
+ //
  // For sourcemapped opportunities that identify a source location
  const source = item.source;
  if (typeof source === 'string') return source;
@@ -401,6 +400,18 @@ function getItemKey(item) {
  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 node
+ if (typeof item.node?.path === 'string') return item.node.path;
+ // Tap-targets
+ if (
+ typeof item.tapTarget?.path === 'string' &&
+ typeof item.overlappingTarget?.path === 'string'
+ ) {
+ return `${item.tapTarget.path} + ${item.overlappingTarget.path}`;
+ }
+ // For most opportunities, diagnostics, etc where 1 row === 1 resource
+ if (typeof item.url === 'string' && item.url) return item.url;
+ if (typeof item.origin === 'string' && item.origin) return item.origin;
 
  // For everything else, use the entire object, actually works OK on most nodes.
  return JSON.stringify(deepPruneItemForKeySerialization(item));