test: stabilize flaky TestResourceManagerClientTestSuite/standalone-resource-manager-with-client-discovery/TestBasicResourceGroupCURD#10935
Conversation
|
[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 |
|
Hi @flaky-claw. Thanks for your PR. I'm waiting for a tikv member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions 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. |
|
Thanks for your pull request. Before we can look at it, you'll need to add a 'DCO signoff' to your commits. 📝 Please follow instructions in the contributing guide to update your commits with the DCO Full details of the Developer Certificate of Origin can be found at developercertificate.org. The list of commits missing DCO signoff:
DetailsInstructions 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. I understand the commands that are listed here. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughTwo immediate assertions in the gRPC resource group CRUD test are replaced with ChangesFix flaky gRPC resource group CRUD test assertions
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #10935 +/- ##
=======================================
Coverage 79.19% 79.20%
=======================================
Files 540 540
Lines 74847 74847
=======================================
+ Hits 59276 59283 +7
+ Misses 11394 11392 -2
+ Partials 4177 4172 -5
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
What problem does this PR solve?
Issue Number: close #10934
Problem Summary:
Flaky test
TestResourceManagerClientTestSuite/standalone-resource-manager-with-client-discovery/TestBasicResourceGroupCURDingithub.com/tikv/pd/tests/integrations/mcs/resourcemanagerintermittently fails, so this PR stabilizes that path.What changed and how does it work?
Root Cause
The test assumed synchronous standalone RM cache visibility after PD-handled resource group metadata writes/deletes.
Fix
Waiting for the cache to reflect accepted modify/delete operations matches the actual contract while preserving exact final assertions.
Verification
Spec:
github.com/tikv/pd/tests/integrations/mcs/resourcemanager :: TestResourceManagerClientTestSuite/standalone-resource-manager-with-client-discovery/TestBasicResourceGroupCURDpd.issue_scoped.v1BASELINE_ONLYbaseline_onlyObserved result:
Required flaky case was not skipped.
target-flaky-corrected passed.
build passed.
Gate checklist:
Commands:
cd tests/integrations && go test -json -tags without_dashboard ./mcs/resourcemanager -run 'TestResourceManagerClientTestSuite/standalone-resource-manager-with-client-discovery/TestBasicResourceGroupCURD' -count=1make buildRelease note
Fixes #10934
Summary by CodeRabbit