pkg/ddl/tests/tiflash: stabilize flaky TestTiFlashProgressAfterAvailableForPartitionTable#67871
pkg/ddl/tests/tiflash: stabilize flaky TestTiFlashProgressAfterAvailableForPartitionTable#67871flaky-claw wants to merge 1 commit into
Conversation
|
Review failed due to infrastructure/execution failure after retries. Please re-trigger review. (Branch ℹ️ Learn more details on Pantheon AI. |
|
[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. 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)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughTest function ChangesTiFlash Progress Test Stabilization
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
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 Tools execution failed with the following error: Failed to run tools: 13 INTERNAL: Received RST_STREAM with code 2 (Internal server error) 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/ddl/tests/tiflash/main_test.go`:
- Around line 28-37: TestMain currently enables intest.InTest,
intest.EnableAssert, and intest.EnableInternalCheck globally which mutates
process-wide test behavior; restrict this to only the tests that need the
TiFlash progress cache synchronization (or add a clear justification comment).
Remove the global enables from TestMain and instead set them inside the specific
test(s) that require them (e.g., in the TiFlash progress-cache flaky test),
wrapping the change in a helper that saves the previous values, sets
intest.InTest/intest.EnableAssert/intest.EnableInternalCheck, runs the test
logic, then restores the original values; alternatively, if keeping in TestMain
is truly required, add a concise comment explaining why the entire package must
have these flags enabled and document the risk. Ensure references: TestMain,
intest.InTest, intest.EnableAssert, intest.EnableInternalCheck, and the TiFlash
progress cache test when applying the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 60c65072-929e-414f-918e-492b3501fcd8
📒 Files selected for processing (2)
pkg/ddl/tests/tiflash/ddl_tiflash_test.gopkg/ddl/tests/tiflash/main_test.go
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #67871 +/- ##
================================================
+ Coverage 76.3085% 76.8585% +0.5499%
================================================
Files 2041 2051 +10
Lines 563503 570742 +7239
================================================
+ Hits 430001 438664 +8663
+ Misses 132586 130387 -2199
- Partials 916 1691 +775
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
f469c33 to
0ce24e7
Compare
|
/retest |
|
@flaky-claw: 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. |
What problem does this PR solve?
Issue Number: close #67563
Problem Summary:
Flaky test
TestTiFlashProgressAfterAvailableForPartitionTableinpkg/ddl/tests/tiflashintermittently fails, so this PR stabilizes that path.What changed and how does it work?
Root Cause
TEST_ISSUE — fixed sleeps assumed async TiFlash progress-cache refresh completed, causing stale-read assertion windows.
Fix
replacing fixed-delay assertions with
require.Eventuallyis necessary to synchronize the test with the real async refresh boundary.Verification
Spec:
pkg/ddl/tests/tiflash :: TestTiFlashProgressAfterAvailableForPartitionTabletidb.go_flaky.defaultBASELINE_ONLYObserved result:
Gate checklist:
Commands:
go test -json ./pkg/ddl/tests/tiflash -run '^TestTiFlashProgressAfterAvailableForPartitionTable$' -count=1go test -json ./pkg/ddl/tests/tiflash -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 #67563
Summary by CodeRabbit