-
Notifications
You must be signed in to change notification settings - Fork 2k
test: do not run criterion benchmarks in no_block_pr #5273
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
These tests have been failing consistently in our CI for as long as I can remember. While each individual test false positive rate is not high, the fact that we run many of them in a multitude of combinations means that at every CI run at least one fails. With this change, we are disabling them in the PR - Optional step. We will still be able to notice performance regressions from the performance tests, so we're not losing much test coverage. We're not completely deleting them because they can be useful for deep dives and for verifying changes in those particular parts of the codebase. Signed-off-by: Riccardo Mancini <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5273 +/- ##
==========================================
+ Coverage 82.84% 82.90% +0.05%
==========================================
Files 250 250
Lines 26967 26967
==========================================
+ Hits 22342 22356 +14
+ Misses 4625 4611 -14
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
Unusually green
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.
I still think we should just delete the python test altogether
Changes
With this change, we are disabling them in the PR - Optional step. We will still be able to notice performance regressions from the performance tests, so we're not losing much test coverage. We're not completely deleting them because they can be useful for deep dives and for verifying changes in those particular parts of the codebase.
Reason
These tests have been failing consistently in our CI for as long as I can remember. While each individual test false positive rate is not high, the fact that we run many of them in a multitude of combinations means that at every CI run at least one fails.
License Acceptance
By submitting this pull request, I confirm that my contribution is made under
the terms of the Apache 2.0 license. For more information on following Developer
Certificate of Origin and signing off your commits, please check
CONTRIBUTING.md
.PR Checklist
tools/devtool checkstyle
to verify that the PR passes theautomated style checks.
how they are solving the problem in a clear and encompassing way.
in the PR.
CHANGELOG.md
.Runbook for Firecracker API changes.
integration tests.
TODO
.rust-vmm
.