-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Generate race unit tests #19078
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?
Generate race unit tests #19078
Conversation
Review ChecklistHello reviewers! 👋 Please follow this checklist when reviewing this Pull Request. General
Tests
Documentation
New flags
If a workflow is added or modified:
Backward compatibility
|
| name: unit_race | ||
| # DO NOT MODIFY: THIS FILE IS GENERATED USING "make generate_ci_workflows" | ||
|
|
||
| name: Unit Test (Race) |
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.
@mhamza15 I think the renaming of the jobs is not intended, but breaks the current configuration of the required status checks.
So either we need to change the repo config (which will then require everyone to merge master into their branches to help them pass the required status check config), or we need to make sure we keep the existing name here.
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.
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.
Ah it matches on the job name, not the workflow name + job name. So this one is fine, the evalengine one needs to be updated.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #19078 +/- ##
==========================================
+ Coverage 69.88% 69.90% +0.02%
==========================================
Files 1610 1612 +2
Lines 215431 215789 +358
==========================================
+ Hits 150551 150850 +299
- Misses 64880 64939 +59 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@mhamza15 one failing CI, but LGTM assuming that is fixed |
mattlord
left a comment
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.
❤️
Add the race unit tests to the workflow generation logic. This will help ensure they stay in sync. As a result, this also fixes the issue that the race unit tests workflow would not fail in CI due to a missing `set -exo pipefail`, allowing the tests to fail but the overall job to still succeed. Signed-off-by: Mohamed Hamza <[email protected]>
Signed-off-by: Mohamed Hamza <[email protected]>
Signed-off-by: Mohamed Hamza <[email protected]>
This reverts commit 06d62bc. Signed-off-by: Mohamed Hamza <[email protected]>
a9d5440 to
c4bc56a
Compare
Signed-off-by: Mohamed Hamza <[email protected]>
Description
The race unit tests workflow would not fail in CI due to a missing
set -exo pipefail, allowing the tests to fail but the overall job to still succeed. This PR adds them to the workflow generator, so that this issue is fixed and so they stay in sync in the future.Going to backport this to release branches as well so that their CI is fixed.
Related Issue(s)
Checklist
Deployment Notes
AI Disclosure