-
Notifications
You must be signed in to change notification settings - Fork 38.6k
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
Update queueset_test.go for FinalSeats #105592
Update queueset_test.go for FinalSeats #105592
Conversation
@MikeSpreitzer: The label(s) |
78587e6
to
b823846
Compare
/retest |
width uint | ||
// initialSeats is the number of seats this request occupies in the first phase of execution | ||
initialSeats uint | ||
finalSeats uint |
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.
nit: add a comment about final seats too
b823846
to
7879a86
Compare
The force-push to 7879a865a79 is a rebase onto |
7879a86
to
4c82996
Compare
The force-push to 4c8299634a3 clarified the wording in the comment on |
4c82996
to
4c11c30
Compare
The force-push to 4c11c30c2d6 improved debug logging from One is adding the scenario name into
and
The other is making TestSeparations include the test name in the queueSet name. |
Here is a rendering of the queueset_tests produced by commit 4c11c30c2d6a23fb200d0b9c0f3e4f5c9d60cfad . |
@@ -140,12 +142,13 @@ func (uc uniformClient) setSplit() uniformClient { | |||
return uc | |||
} | |||
|
|||
func (uc uniformClient) seats(width uint) uniformClient { | |||
uc.width = width | |||
func (uc uniformClient) width1(seats uint) uniformClient { |
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.
Apparently I didn't save this comment yesterday.
I'm not a huge fan of "width1". Can we maybe call it "initialWidth" or sth?
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.
or initialSeats?
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.
Go won't let us have a method and field with the same name.
Track the introduction of FinalSeats. Give up on calculating expected results for tests with added latency, because I did not find an easy and obvious way to do it.
4c11c30
to
0fc595e
Compare
The force-push to 0fc595e renames the uniformClient method that sets the |
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: MikeSpreitzer, wojtek-t The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/triage accepted |
What type of PR is this?
/kind cleanup
What this PR does / why we need it:
This PR updates
queueset_test.go
to track the introduction of FinalSeats.This PR gives up on calculating expected results for tests with added latency,
because I did not find an easy way to do it.
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing change?
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: