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
Improve unit test TestZeroRequest #66471
Improve unit test TestZeroRequest #66471
Conversation
/assign @bsalamat |
7ceb726
to
67ed346
Compare
67ed346
to
3d97b10
Compare
/test pull-kubernetes-e2e-kops-aws |
/lgtm |
/cc @bsalamat can you review and approve ? Thanks |
@@ -684,12 +687,12 @@ func TestZeroRequest(t *testing.T) { | |||
} | |||
for _, hp := range list { | |||
if test.name == "test priority of larger pod with machine with zero-request pod" { |
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.
We actually want to get rid of this if
statement here. We should not rely on the name of the test. Please remove this if statement and provide the expected score in the corresponding test case.
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.
@bsalamat Sorry, I thougt the score of the third case varies.
3d97b10
to
5449d15
Compare
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
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bsalamat, islinwb, yastij 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 |
Automatic merge from submit-queue (batch tested with PRs 66291, 66471, 66499). If you want to cherry-pick this change to another branch, please follow the instructions here. |
What this PR does / why we need it:
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close the issue(s) when PR gets merged):Fixes #66468
Special notes for your reviewer:
Release note: