google / tachometer

Statistically rigorous benchmark runner for the web
BSD 3-Clause "New" or "Revised" License
663 stars 21 forks source link

Create a Tachometer GitHub Action #142

Open aomarks opened 4 years ago

aomarks commented 4 years ago

We have a somewhat complicated integration that works with Travis or any other CI system. I think that now that GitHub Actions exist, we could have a much simpler workflow instead.

cdata commented 4 years ago

Can you share any additional details about what makes integration complicated today?

aomarks commented 4 years ago

The way we currently do perf regression testing for the lit-element and lit-html repos is by running tachometer piggy-backed on Travis, alongside the unit tests. Tachometer itself then knows how to post its results back using the GitHub API, using credentials for a GitHub "App" so that it shows up as its own result "check" (e.g. https://github.com/Polymer/lit-element/pull/987/checks).

Relevant code:

https://github.com/Polymer/lit-element/blob/master/travis-bench.sh https://github.com/Polymer/lit-element/blob/master/.travis.yml https://github.com/Polymer/tachometer/blob/master/src/github.ts

So, switching to a GitHub Action would be much simpler, since we shouldn't need any of this API integration, credential passing, or Travis-piggybacking. Open question how we display the result table, though. For a GitHub Check, the API lets you post markdown that will show up in the Checks tab. Maybe an Action can also be a check? Maybe it can post a comment? I'm slightly fuzzy on the interaction between Actions and Checks here.

andrewiggins commented 4 years ago

@cdata (mentioning you since you assigned yourself to this) FWIW I built a GitHub action to report tachometer results as a comment in a PR: andrewiggins/tachometer-reporter-action.

I found that trying to use the checks API and actions together is not as valuable since an action job shows up as a check on a PR so the additional check felt redundant.

Anyway, happy to discuss ways to make this more helpful to the community if you are interested

andrewiggins commented 4 years ago

FYI - just added support for multiple measurements per benchmarks in my reporter action, one thing that was notably missing before.