pkg/webhook: unit test Multi-Cluster Ingress #5518
Conversation
karmada-bot
added
the
size/L
mohamedawnallah
force-pushed
the
unitTestMultiClusterIngress
branch
from
087ff43 to
1dd8511
Compare
In this commit we introduce unit tests for the `Validation Admission`. These tests ensure correct behavior for various validation scenarios and improve coverage: - Tests the behavior when decoding the request object fails, verifying that admission is denied with an appropriate error message. - Validates that the webhook denies admission when encountering invalid values in the `MultiClusterIngress` spec, ensuring error messaging. - Confirms that valid `MultiClusterIngress` objects are admitted without errors. Signed-off-by: Mohamed Awnallah <[email protected]>
mohamedawnallah
force-pushed
the
unitTestMultiClusterIngress
branch
from
1dd8511 to
d2259d5
Compare
|
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 #5518 +/- ##
==========================================
+ Coverage 31.70% 34.22% +2.52%
==========================================
Files 643 643
Lines 44445 44524 +79
==========================================
+ Hits 14090 15239 +1149
+ Misses 29325 28130 -1195
- Partials 1030 1155 +125
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| @@ -0,0 +1,448 @@ | |||
| /* | |||
| Copyright 2023 The Karmada Authors. | |||
| const ( | ||
| Denied ResponseType = "Denied" | ||
| Allowed ResponseType = "Allowed" | ||
| Errored ResponseType = "Errored" | ||
| ) |
There was a problem hiding this comment.
I see that this is used elsewhere. Would you consider putting it in a public test file?
There was a problem hiding this comment.
@XiShanYongYe-Chang, could we handle this in a separate improvement PR after the webhook PRs are merged? Since these changes are related to those PRs, doing it later will avoid creating additional PRs for both merged and unmerged changes?
| type fakeValidationDecoder struct { | ||
| err error | ||
| obj runtime.Object | ||
| } |
There was a problem hiding this comment.
Can this structure also be used as a public structure?
There was a problem hiding this comment.
@XiShanYongYe-Chang, same could we handle this in a separate improvement PR after the webhook PRs are merged? Since these changes are related to those PRs, doing it later will avoid creating additional PRs for both merged and unmerged changes?
|
[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
In this commit we introduce unit tests for the
Validation Admission. These tests ensure correct behavior for various validation scenarios and improve coverage:MultiClusterIngressspec, ensuring error messaging.MultiClusterIngressobjects are admitted without errors.What type of PR is this?
Which issue(s) this PR fixes:
Part of #5491.
Dependency:
This PR depends on #5520.Does this PR introduce a user-facing change?: