Lighthouse-ci: Proposal: hooks to inspect and modify report data

[shawnbot]

After running into some of the issues described in both #138 and #154, I have a suggestion that might make it easier for folks to work around the lack of built-in support for features like matrix builds (desktop + mobile) and URL determinism, taking the burden of implementing them off you and other contributors:

Lots of other platforms and tools (Rollup is one) offer declarative hooks that allow the configuration to inspect and even modify data (in place) at specific points in the execution lifecycle.

In my specific case, I'm trying to use Lighthouse CI to run scheduled reports on a list of URLs that either aren't under our control or have content that isn't managed in GitHub. Currently, builds for which the git SHA hasn't changed (which will be typical because the actions are run on a schedule) fail because the hashes are the same.

I'm going to gloss _very_ heavily over the implementation details because I haven't looked at how builds are scheduled or structured, but hopefully you'll get where I'm going. Some examples:

• Before running the builds, a hook could modify the list of builds to run to include desktop runs for each, e.g.

  module.exports = {
    ci: {
      hooks: {
        beforeRunBuilds ({ builds }) {
          for (const { hash, settings, ...build } of builds) {
            builds.push({
              ...build,
              hash: `${hash}-desktop`,
              settings: {
                ...settings,
                emulatedFormFactor: 'desktop'
              }
            })
          }
        }
      }
    }
  }
  

• Before the builds are uploaded, a hook could modify the hashes to ensure that they're unique (in the case of my scheduled runs):

  const date = (new Date()).d.toLocaleDateString()
  module.exports = {
    ci: {
      hooks: {
        beforeUploadBuild ({ build }) {
          build.hash = `${build.hash}@${date}`
        }
      }
    }
  }
  

• Or there could even be a hash hook to modify the data used to generate the hash for each build:

  const date = (new Date()).d.toLocaleDateString()
  module.exports = {
    ci: {
      hooks: {
        beforeHash ({ hash }) {
          // assuming a hash object that's JSON.stringify()'d before being hashed into a string
          hash.date = date
        }
      }
    }
  }
  

If you're uncomfortable with hooks living in the configuration, plugins might be a good place to enable them. That would also make it easier for folks to distribute reusable packages that do very specific things: enable desktop + mobile builds, or further expand a matrix of testing environments; ensure that build hashes involve the current date, time, or some other changing context.

LHCI is such a great tool, and I would love to help with this if I can. Thanks for considering it!

[patrickhulce]

Thanks for filing @shawnbot! I like this idea a lot to enable the community to find solutions to these problems that we might not have time to solve properly in LHCI itself 👍

There are a few challenges with the way you've outlined it here but I think we can work around them. I'll circle back around later with my full thoughts :)

[shawnbot]

@patrickhulce Thanks! After some digging, it looks like I might just be able to set LHCI_BUILD_CONTEXT__CURRENT_HASH to, say, the head SHA + a timestamp and that would solve the problem of duplicate builds for scheduled runs. Does that sound right?

https://github.com/GoogleChrome/lighthouse-ci/blob/f9b23c281eb71ffaf11ceb2f6b409025226af502/packages/utils/test/build-context.test.js#L52-L53

[patrickhulce]

For the specific problem of duplicate hashes, yes but as a broader solution to the "use desktop settings for this run" problem I like the hooks :)

[patrickhulce]

I think we need three primary hooks to make these use cases work.

1. beforeCollect where LHCI would provide an array of objects that each represent a single run of Lighthouse on a specific URL with given settings. The hook could manipulate the settings of those objects or append new runs to be collected for ultimate flexibility.
2. beforeUploadBuild where LHCI would provide the build object. The hook could manipulate the properties of that object
3. beforeUploadRuns where LHCI would provide an array of run objects. The hook could manipulate the properties of those runs or split/append new ones as necessary.

[shawnbot]

@patrickhulce Fantastic. Like I said, I'm interested in helping! Can you gut check where I might stub out a beforeCollect hook? I was looking at the collect runCommand() function, but maybe it could be done in startServerAndDetermineUrls() instead? For instance:

async function startServerAndDetermineUrls(options) {
- const urlsAsArray = Array.isArray(options.url) ? options.url : options.url ? [options.url] : [];
+ let urlsAsArray = Array.isArray(options.url) ? options.url : options.url ? [options.url] : [];
  if (!options.staticDistDir) {
    if (!urlsAsArray.length) throw new Error(`No URLs provided to collect`);

    let close = async () => undefined;
    if (options.startServerCommand) {
      const regexPattern = new RegExp(options.startServerReadyPattern, 'i');
      const {child, patternMatch, stdout, stderr} = await runCommandAndWaitForPattern(
        options.startServerCommand,
        regexPattern,
        {timeout: options.startServerReadyTimeout}
      );
      process.stdout.write(`Started a web server with "${options.startServerCommand}"...\n`);
      close = () => killProcessTree(child.pid);

      if (!patternMatch) {
        // This `message` variable is only for readability.
        const message = `Ensure the server prints a pattern that matches ${regexPattern} when it is ready.\n`;
        process.stdout.write(`WARNING: Timed out waiting for the server to start listening.\n`);
        process.stdout.write(`         ${message}`);
        if (process.env.CI) process.stderr.write(`\nServer Output:\n${stdout}\n${stderr}\n`);
      }
    }
+
+   if (typeof options.beforeCollect === 'function') {
+     // allow the array to either be modified by reference or returned
+     urlsAsArray = options.beforeCollect(urlsAsArray) || urlsAsArray;
+   }

    return {
      urls: urlsAsArray,
      close,
    };
  }

[patrickhulce]

That's the rough area we'll need for beforeCollect but unfortunately it's the one that requires the most internal restructuring. (We'll need to create the idea of a collection object that contains the URL, settings, etc which currently exists only implicitly)

2 and 3 might be easier to get started with contributing if you want to start there? :)