event-ruler icon indicating copy to clipboard operation
event-ruler copied to clipboard

Published 20 hours ago • verified publisher icon aws

Make Benchmark Tests Run as part of CI for Pull-Requests

Open baldawar opened this issue 3 years ago • 3 comments

What is your idea?

Right now its kind of annoying to ask for benchmark results as part of each pull-requests. Ideally, we'd have benchmarks run as part of the PR or on-command whenever someone wants it before merging a change.

This would have made the optimization PRs like https://github.com/aws/event-ruler/pull/37 and https://github.com/aws/event-ruler/pull/39 easier to merge in. Also, it reduces one more friction point for any future pull-requests.

Would you be willing to make the change?

Maybe

Additional Context

When we get this working, we should make sure benchmarks are only ever run as PR and not part of regular builds (where something running for 6~7 mins is painfully slow for development cycles).

baldawar avatar Sep 13 '22 05:09 baldawar

I can help with the GHA part of this (including a small tweak to cancel long-running jobs such as benchmarks when new commits are pushed to a PR. Since I'm not an expert in Java benchmarking and the tooling, e.g. which output to use, creating +/- comparisons, etc. it would be great if someone could point me in the right direction so I can create the PR.

embano1 avatar Sep 27 '22 08:09 embano1

Hmm, I agree in principle but I think people shouldn't send us PRs that have big performance penalties because that will never be OK for Ruler. So I'd like to have some mechanism to provide an incentive to make this clear to a contributor. Maybe it's enough to write it into our submission guidelines or template?

timbray avatar Sep 27 '22 22:09 timbray

The default template does ask for benchmarks which we can keep as it. The action here should help ensure as folks push additional updates to their PR, the github actions are good enough.

On steps to implement this, I suspect it'll be like

  • use maven surefire plugin to run a specific test https://maven.apache.org/surefire/maven-surefire-plugin/examples/single-test.html
    • This is the java class that needs to be run https://github.com/aws/event-ruler/blob/main/src/test/software/amazon/event/ruler/Benchmarks.java
  • update https://github.com/aws/event-ruler/blob/main/.github/workflows/CI.yml to setup automation
    • if there's any dependency or maven configuration changes required, this is the file that'll need tinkering https://github.com/aws/event-ruler/blob/main/pom.xml

baldawar avatar Sep 29 '22 23:09 baldawar

closed by https://github.com/aws/event-ruler/pull/178

baldawar avatar Sep 17 '24 00:09 baldawar