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
add unit test for dra/manager.go #118711
add unit test for dra/manager.go #118711
Conversation
04fbc4e
to
dec5d40
Compare
/cc @bart0sh @charles-chenzz |
/triage accepted |
dec5d40
to
a06773f
Compare
/cc @pohly |
This is likely to conflict with #119012. I'd prefer to merge that first because it has to be before code freeze, while this test can still be merged before test freeze. |
81bf40c
to
583c446
Compare
Overall approach looks good to me. A couple of suggestions:
|
583c446
to
a09414f
Compare
@bart0sh, I updated the PR to combine all test cases that test one API. I tried to cover as much as error cases as possible. If you do not mind, I'd suggest to to address the review asap to get this merged before the code freeze. I will continue working to increase the code coverage and cover the important error conditions. |
/test pull-kubernetes-node-e2e-crio-dra |
a09414f
to
397ec03
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.
Thank you for the PR. Overall looks very good.
397ec03
to
6d33c35
Compare
Co-Authored-By: charles-chenzz <Rekles666@gmail.com> Signed-off-by: TommyStarK <thomasmilox@gmail.com>
6d33c35
to
0372e4b
Compare
I only did a quick pass, but in general any unit tests are better than none. /lgtm |
LGTM label has been added. Git tree hash: a3ff003a7e9c481c2bfd119711f0b097e3656f96
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: klueska, TommyStarK 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 |
co-author: @charles-chenzz
co-author: @TommyStarK
What type of PR is this?
/kind feature
What this PR does / why we need it:
Add unit test to the DRA manager
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Follow up for #118436
Does this PR introduce a user-facing change?
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: