Improve test coverage of pkg/controllers/context #5310
Conversation
karmada-bot
added
kind/failing-test
karmada-bot
added
the
size/L
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #5310 +/- ##
==========================================
+ Coverage 28.36% 28.43% +0.07%
==========================================
Files 632 632
Lines 43774 43810 +36
==========================================
+ Hits 12416 12457 +41
+ Misses 30458 30448 -10
- Partials 900 905 +5
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
Hi @anujagrawal699, it seems that current has more than one commit, and one of them is not a file involved in the current PR description, can you help to remove it? |
|
@XiShanYongYe-Chang The second file was the file added in PR #5301. I was unaware of it being present in the commit because git diff of that file with master was empty. Now should i change the description of this PR or |
|
Let me review #5301 first, then we can come back to this pr. |
|
Hi @anujagrawal699, we can continue this pr, can you help remove the other commit? |
Signed-off-by: Anuj Agrawal <[email protected]>
anujagrawal699
force-pushed
the
improveTestCoverage-pkg/controllers/context
branch
from
560ffb3
to
baff21a
Compare
|
@XiShanYongYe-Chang Thanks, i removed the other commit. Can i add more PR's like this? |
Can wait a bit for me to reivew to finish this pr. |
| @@ -1,5 +1,5 @@ | |||
| /* | |||
| Copyright 2022 The Karmada Authors. | |||
| Copyright 2024 The Karmada Authors. | |||
There was a problem hiding this comment.
This is the creation time of the file. we do not need to change it.
There was a problem hiding this comment.
Thanks, i'll update the commit.
| @@ -34,49 +36,63 @@ func TestContext_IsControllerEnabled(t *testing.T) { | |||
| name: "on by name", | |||
| controllerName: "bravo", | |||
| disabledByDefaultControllers: []string{"delta", "echo"}, | |||
| controllers: []string{"alpha", "bravo", "-charlie"}, // --controllers=alpha,bravo,-charlie | |||
There was a problem hiding this comment.
The comments help us to understand the test better and can be kept.
There was a problem hiding this comment.
@XiShanYongYe-Chang Made the suggested changes. Thanks!
Signed-off-by: Anuj Agrawal <[email protected]>
Signed-off-by: Anuj Agrawal <[email protected]>
karmada-bot
added
size/M
| { | ||
| name: "on by star, not in disabled list", | ||
| controllerName: "foxtrot", | ||
| disabledByDefaultControllers: []string{"delta", "echo"}, | ||
| controllers: []string{"*"}, // --controllers=* | ||
| expected: true, | ||
| }, |
There was a problem hiding this comment.
You can reposition this case so that it is adjacent to similar test cases, making it easier for other users to read and understand.
|
/assign |
Signed-off-by: Anuj Agrawal <[email protected]>
There was a problem hiding this comment.
Thanks for your contribution @anujagrawal699
/lgtm
/approve
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: XiShanYongYe-Chang 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 |
karmada-bot
added
the
approved
|
I was remiss and forgot to tell you to compress all commits into one commit. q_q |
You helped a lot though. Thanks! |
Description:
This PR enhances the test coverage for the pkg/controllers/context package, increasing its test coverage from 42.3% to 100%. The new tests are designed to ensure the functionality and reliability of the package, improving its overall maintainability.
Implemented Tests:
Test Coverage:
Implementing this tests increased it's test coverage from 42.3% to 100%. This can be verified in the file directory i.e. pkg/controllers/context using command
go test -coverprofile=coverage.outWhat type of PR is this?
/kind failing-test
/kind feature
What this PR does / why we need it:
The tests introduced in this PR will:
Improve test coverage of pkg/controllers/context from 42.3% to 100%
Help catch potential issues in future changes
Which issue(s) this PR fixes:
Fixes a part #5236
Special notes for your reviewer:
These commits are part of my introduction to the Karmada project and provide me with hands-on experience in implementing unit tests within the project. I look forward to implement more unit tests in the LFX Mentorship program.
Does this PR introduce a user-facing change?: