bobheadxi / gobenchdata

📉 Run Go benchmarks, publish results to an interactive web app, and check for performance regressions in your pull requests
https://gobenchdata.bobheadxi.dev
MIT License
142 stars 14 forks source link

checks always compare against first recorded benchmark #70

Closed srenatus closed 1 year ago

srenatus commented 1 year ago

So on every merge, we'll update and commit benchmarks.json in the benchmarks branch of the repo. It works well, I can see one new entry for every merged commit. ✅

For every PR, we're running checks against the recorded benchmark.json file. The checks run. But they always use the first recorded commit for base, which doesn't seem right to me:

📊 Running benchmarks...
detected 2 benchmark suites
successfully output results as json to '/tmp/gobenchdata/benchmarks.json'

📚 Checking out StyraInc/yadda@benchmarks...
branch 'benchmarks' set up to track 'origin/benchmarks'.

🔎 Evaluating results against base runs...
report output written to /tmp/gobenchdata/checks-results.json
Warning: The `set-output` command is deprecated and will be disabled soon. Please upgrade to using Environment Files. For more information see: https://github.blog/changelog/2022-10-11-github-actions-deprecating-save-state-and-set-output-commands/

📝 Generating checks report...
|    |                   BASE                   |                 CURRENT                  | PASSED CHECKS | FAILED CHECKS | TOTAL |
|----|------------------------------------------|------------------------------------------|---------------|---------------|-------|
| ✅ | 2cb3161f3907093c4a5cbd9cf2641b92243051ce | 8f83bf07bd43eb463b5488f30dea5ea5fa098c78 |             3 |             0 |     3 |

Am I holding the tool wrong? I'm quite confused about this one 😅

srenatus commented 1 year ago

To debug this, I've run a git log -1 and cat benchmarks.json in /tmp/build following the gobenchdata checks step; and it showed nothing out of the ordinary -- the latest commit is there and the benchmark.json contains everything.

bobheadxi commented 1 year ago

Shoot, you're right - I double-checked the implementation and I think checks are indeed selecting the oldest run 😬 Fixed here: https://github.com/bobheadxi/gobenchdata/pull/71

srenatus commented 1 year ago

Thank you!

srenatus commented 1 year ago

I take it I'll have to wait for the next two to have this fix in place?

bobheadxi commented 1 year ago

I've cut a release with the fix: https://github.com/bobheadxi/gobenchdata/releases/tag/v1.3.0

srenatus commented 1 year ago

Yay thanks

srenatus commented 1 year ago

Tested, works great. Thanks for the quick turnaround.