Improved test coverage for workloadrebalancer controller #5606
Conversation
karmada-bot
added
the
kind/failing-test
karmada-bot
added
kind/feature
|
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 #5606 +/- ##
==========================================
+ Coverage 35.01% 35.19% +0.17%
==========================================
Files 645 645
Lines 44861 44869 +8
==========================================
+ Hits 15708 15791 +83
+ Misses 27938 27846 -92
- Partials 1215 1232 +17
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
/cc @chaosi-zju |
| name: "reconcile cluster-wide resource rebalancer", | ||
| req: controllerruntime.Request{ | ||
| NamespacedName: types.NamespacedName{Name: clusterRebalancer.Name}, | ||
| }, | ||
| existObjects: []client.Object{clusterDeploy, clusterBinding, clusterRebalancer}, | ||
| existObjsWithStatus: []client.Object{clusterRebalancer}, | ||
| wantStatus: appsv1alpha1.WorkloadRebalancerStatus{ | ||
| ObservedWorkloads: []appsv1alpha1.ObservedWorkload{ | ||
| { | ||
| Workload: clusterDeployObj, | ||
| Result: appsv1alpha1.RebalanceSuccessful, | ||
| }, |
There was a problem hiding this comment.
thanks, but this case is not suitable, since deploy is not cluster-wide resource, and it will never generate clusterResourceBinding. you'd better use other resource like clusterrole~
There was a problem hiding this comment.
Thanks i'll work on it.
anujagrawal699
force-pushed
the
addedTests-controllers/workloadrebalancer
branch
from
ca3ee3f
to
48158a0
Compare
|
@chaosi-zju Please take a look. |
| @@ -139,6 +140,19 @@ var ( | |||
| }, | |||
| }, | |||
| } | |||
|
|
|||
| clusterRole = &rbacv1.ClusterRole{ | |||
| ObjectMeta: metav1.ObjectMeta{Name: "test-cluster-role"}, | |||
There was a problem hiding this comment.
This is a cluster wide resource, it is not suitable to use fixed simple names, for the purpose of preventing from affecting other cases. So it is better to name with a random suffix.
BTW, I'd much appreciate it if you can also rename above deployment with a random suffix too~
Signed-off-by: Anuj Agrawal <anujagrawal380@gmail.com> Changed the resource to clusterrole Signed-off-by: Anuj Agrawal <anujagrawal380@gmail.com> Added tests for workloadrebalancer controller Signed-off-by: Anuj Agrawal <anujagrawal380@gmail.com>
anujagrawal699
force-pushed
the
addedTests-controllers/workloadrebalancer
branch
from
48158a0
to
9bb8b5b
Compare
|
/lgtm |
|
[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
Description:
This PR adds a new test case for cluster-wide resources and refactors the existing test structure for the WorkloadRebalancer controller. These changes aim to improve code coverage and reduce cyclomatic complexity.
Changes:
Added a new test case for cluster-wide resource rebalancing:
Refactored the TestRebalancerController_Reconcile function:
Rationale:
Test Coverage:
pkg/controllers/workloadrebalancer : 63.01% to 75,.4%
What type of PR is this?
/kind failing-test
/kind feature
Which issue(s) this PR fixes:
Fixes a part of #5470
Does this PR introduce a user-facing change?: