-
Notifications
You must be signed in to change notification settings - Fork 436
Add benchmark test run to the CI #3007
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
CC: @kevinjqliu @geruh |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for adding this! Left a few comments inline.
| path: .coverage* | ||
| include-hidden-files: true | ||
|
|
||
| benchmark-test: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we run the benchmarks against main as a baseline in the same job? It would be cool to create a report of all the benchmarks that can be uploaded onto the PR. Maybe we should think of this a bit more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let me try something and add it in a follow up commit
| - name: Install system dependencies | ||
| run: sudo apt-get update && sudo apt-get install -y libkrb5-dev # for kerberos | ||
| - name: Install | ||
| run: make install |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
test_partitioned_write currently prints average runtime but has no threshold or assertion. Worth adding a baseline comparison
| run: sudo apt-get update && sudo apt-get install -y libkrb5-dev # for kerberos | ||
| - name: Install | ||
| run: make install | ||
| - name: Run benchmarks |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like these files are never got produced
Closes #27
Rationale for this change
We have benchmark tests defined here
This PR adds them to the CI as a job to run
Are these changes tested?
Locally executed the make command
Are there any user-facing changes?
No