test: add MCS controller event assertions in E2E#7313
test: add MCS controller event assertions in E2E#7313Denyme24 wants to merge 2 commits intokarmada-io:masterfrom
Conversation
…ound event Signed-off-by: Denyme24 <namanraj24@outlook.com>
|
@Denyme24: The label(s) DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the robustness of Karmada's MultiClusterService E2E tests by integrating comprehensive event assertions directly into existing workflows. The changes ensure that critical controller events, indicating successful operations and specific failure conditions like Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request is a valuable addition, enhancing the E2E test coverage for MultiClusterService (MCS) controller events. The new tests for success-path events and the deterministic negative case for ClusterNotFound are well-implemented and improve confidence in the MCS controllers' behavior. The overall approach of adding inline event assertions to existing workflows is clean and effective. I have one suggestion to further improve the consistency of the new test helpers.
There was a problem hiding this comment.
Pull request overview
This PR extends the existing MultiClusterService (MCS) E2E flows to also assert that expected Karmada controller events are emitted during successful reconciliation, and adds a deterministic negative case to validate ClusterNotFound warnings.
Changes:
- Add event assertions for
SyncDerivedServiceSucceedin existingServiceImportflows. - Add event assertions for
SyncServiceSucceedandDispatchEndpointSliceSucceedin the CrossClusterMultiClusterServiceflow. - Add a new negative CrossCluster scenario to assert the
ClusterNotFoundwarning event, plus a small helper for event-reason checks.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #7313 +/- ##
=======================================
Coverage 42.03% 42.04%
=======================================
Files 874 874
Lines 53551 53551
=======================================
+ Hits 22511 22516 +5
+ Misses 29349 29346 -3
+ Partials 1691 1689 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Signed-off-by: Denyme24 <namanraj24@outlook.com>
|
hey @XiShanYongYe-Chang, added E2E test coverage for Karmada controller events in the existing MCS test flows. PTAL. |
|
Thanks~ |
What type of PR is this?
/kind cleanup
What this PR does / why we need it:
This PR improves E2E coverage for Karmada controller events in the existing MCS test flows.
as of now, the MCS E2E suite validates functional behavior such as ServiceExport/ServiceImport connectivity, derived service creation, and EndpointSlice propagation, but it does not verify the controller events emitted during those workflows. This PR adds inline event assertions to the current MCS scenarios so we also validate that the expected controller events are produced when reconciliation succeeds.
Specifically, this PR:
SyncDerivedServiceSucceedin the existingServiceImportflowsSyncServiceSucceedandDispatchEndpointSliceSucceedin the existing CrossClusterMultiClusterServiceflowClusterNotFoundwarning event onMultiClusterServiceWe need this to improve E2E event coverage for MCS-related controllers without introducing standalone event-only tests, keeping the assertions close to the user-observable workflows they belong to.
Which issue(s) this PR fixes:
Part of #7252
Special notes for your reviewer:
This patch focuses on deterministic event coverage that fits naturally into the existing MCS E2E flows.
SyncDerivedServiceFailed,SyncServiceFailed, andDispatchEndpointSliceFailedwere not included in this PR because they are harder to trigger reliably in the current E2E environment without introducing artificial failures or making the tests more brittle.APIIncompatiblewas also left out because the E2E clusters are created with a consistent environment, so reproducing an actual API compatibility mismatch is not straightforward in a stable CI-friendly way.To keep this change aligned with the selected inline-assertion approach, this PR adds coverage for the success-path events that are already exercised by existing MCS scenarios, plus a deterministic negative case for
ClusterNotFound.Does this PR introduce a user-facing change?: