br/pkg/streamhelper: stabilize flaky TestStoreRemoved#67954
br/pkg/streamhelper: stabilize flaky TestStoreRemoved#67954ti-chi-bot[bot] merged 1 commit intopingcap:masterfrom
Conversation
|
@flaky-claw I've received your pull request and will start the review. I'll conduct a thorough review covering code quality, potential issues, and implementation details. ⏳ This process typically takes 10-30 minutes depending on the complexity of the changes. ℹ️ Learn more details on Pantheon AI. |
|
Hi @flaky-claw. Thanks for your PR. PRs from untrusted users cannot be marked as trusted with 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. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdded a helper function Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.11.4)Command failed 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 |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Leavrth, YuJuncen The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/retest |
|
@yinsustart: PRs from untrusted users cannot be marked as trusted with 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. |
|
/check-issue-triage-complete |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #67954 +/- ##
================================================
+ Coverage 77.7953% 78.1620% +0.3666%
================================================
Files 1984 1992 +8
Lines 549728 556091 +6363
================================================
+ Hits 427663 434652 +6989
+ Misses 121145 120027 -1118
- Partials 920 1412 +492
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 #52791
Problem Summary:
Flaky test
TestStoreRemovedinbr/pkg/streamhelperintermittently fails, so this PR stabilizes that path.What changed and how does it work?
Root Cause
TestStoreRemovedwas flaky because it used channel-idle timing (waitPendingEvents) as a proxy for delivery completion, so post-removal events could still be in flight whenDrop()canceled them.Fix
The test needed to wait for the semantic condition it actually cares about, namely that the subscriber has observed full-span coverage at the final post-removal checkpoint.
Verification
Spec:
br/pkg/streamhelper :: TestStoreRemovedtidb.go_flaky.defaultBASELINE_ONLYObserved result:
Gate checklist:
Commands:
go test -json ./br/pkg/streamhelper -run '^TestStoreRemoved$' -count=1go test -json ./br/pkg/streamhelper -count=1make buildCheck List
Tests
Side effects
Documentation
Release note
Please refer to Release Notes Language Style Guide to write a quality release note.
Fixes #52791
Summary by CodeRabbit