GoogleChrome / lighthouse-ci

Automate running Lighthouse for every commit, viewing the changes, and preventing regressions
Apache License 2.0
6.44k stars 648 forks source link

Include timing details in GitHub Status Check #603

Open ianjmacintosh opened 3 years ago

ianjmacintosh commented 3 years ago

I'd love to see my actual timing numbers (in milliseconds for FCP and TTI) in my GitHub status checks like I can with the categorical scores. I use the Lighthouse CI integration for GitHub to see how my changes affect my site's speed and just recently moved from using the basic "category" assertions to paying specific attention to my FCP and TTI times. I've added warn and error for my short-term goal and my current threshold for acceptable, but I want to see the progress I'm making without fishing through my build output/log in the GitHub Actions screens.

I'd love to be able to configure the message presented by the GitHub status check with richer information than just pass, pass w/warnings, or fail. See screenshot below for an idea of what I've got in mind.

I've considered looking into a bot to add a comment for every build, but that seems noisy. I also considered using a separate Lighthouse server to track my results, but I want the output right in my pull request. Since I could see the category scores, now I've got this idea the same kind of thing is possible for performance timing events.

Current: GitHub Action showing tests passing with warnings

Idea:

GitHub Action showing tests passing 'first-contentful-paint: 2488 (WARN: >2200)
ianjmacintosh commented 3 years ago

I suspect supporting this would require non-trivial changes to https://github.com/GoogleChrome/lighthouse-ci/blob/main/packages/cli/src/upload/upload.js#L246-L306

patrickhulce commented 3 years ago

Thanks for filing @ianjmacintosh! What if LHCI just directly exposed the ability to have a javascript function customize the description? Several people have had several conflicting opinions over the years about what this message should say :)

Usage would look something like...

lighthouserc.js


module.exports = {
  ci: {
    upload: {
      githubStatusDescription({url, assertionResults, state}) {
        return `Whatever you want: ${assertionResults.map(result => /* ... */)}
      }
    }
  }
}
ianjmacintosh commented 3 years ago

@patrickhulce That looks great, I'd love that.

I can easily imagine people getting very opinionated about how people should use the status checks, and they should totally have that conversation within the scope of their own team's implementation. Allowing people to provide their own status-update function seems like a very elegant solution. Thank you, let me know if there's anything I can do to help.

ianjmacintosh commented 3 years ago

@patrickhulce I'm not sure what the enhancement & P2 labels mean, but I'd love to see this feature become real. Would it be cool if I took a crack at implementing it? It might be over my head, but I wanna give it a shot. On the other hand, I don't want to step on anyone's toes. Thoughts?

patrickhulce commented 3 years ago

Feel free to go for it @ianjmacintosh! I imagine the trickiest parts are going to be just documenting it accurately since it will require a javascript config in order to work :)

ianjmacintosh commented 2 years ago

@patrickhulce I'm not sure what the enhancement & P2 labels mean, but I'd love to see this feature become real. Would it be cool if I took a crack at implementing it? It might be over my head, but I wanna give it a shot. On the other hand, I don't want to step on anyone's toes. Thoughts?

I started on this and didn't finish; I'm not familiar with TypeScript and got stumped by it. If anyone is able to implement this feature, please don't let me get in your way!

vivekrpatel8 commented 9 months ago

@patrickhulce Just following up regarding if we can modify the status check descriptions?