-
Notifications
You must be signed in to change notification settings - Fork 349
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
chore: add resource_pools intg test #9356
Conversation
✅ Deploy Preview for determined-ui canceled.
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #9356 +/- ##
==========================================
+ Coverage 45.14% 45.23% +0.08%
==========================================
Files 1230 1230
Lines 154567 154567
Branches 2404 2405 +1
==========================================
+ Hits 69783 69912 +129
+ Misses 84589 84460 -129
Partials 195 195
Flags with carried forward coverage won't be shown. Click here to find out more. |
9d8069f
to
2dcd1b6
Compare
rp.Schedule() | ||
|
||
require.False(t, rp.reschedule) | ||
go require.True(t, rp.reqList.IsScheduled(allocID)) |
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.
Why is this one a goroutine and not the others? (I apologize, I'm not familiar with this part of the code. I dug into it a bit, but couldn't find anything.)
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 think @carolinaecalderon was trying to fix the race condition. This (surprisingly) tricks the race detector but, of course, doesn't fix it. I think she's working on something else.
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.
yeah, outdated -- I removed the test that was flaky in CI. I created a new ticket (https://hpe-aiatscale.atlassian.net/browse/RM-291) where I'll address this later.
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.
LGTM
Ticket
RM-201
Description
Add intg tests for resource_pool.go
Leave a couple things as TODO for Bradley's refactor PR.
Test Plan
See attached test.
Checklist
docs/release-notes/
.See Release Note for details.