test: fix unstable unit test TestScaleNode#5366
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 |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization 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)
📝 WalkthroughWalkthroughTestScaleNode now waits for coordinator.bootstrapper state changes after node additions and removals: it polls NodeInitialized for newly added node IDs and verifies removed node IDs are absent. Two local helpers using require.Eventually implement the polling. ChangesBootstrap State Verification in TestScaleNode
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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 docstrings
🧪 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 |
There was a problem hiding this comment.
Code Review
This pull request introduces helper functions requireBootstrapReady and requireBootstrapNodeRemoved in coordinator_test.go to ensure proper synchronization during node scaling tests. These helpers are integrated into TestScaleNode to verify node initialization and removal before checking replication sizes. There are no review comments, and I have no additional feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
What problem does this PR solve?
Issue Number: close #4837
What is changed and how it works?
TestScaleNode no longer assumes that a NodeManager.Tick has already completed all coordinator bootstrap side effects. After adding
nodes, the test now waits until the new nodes are initialized in the coordinator bootstrapper before asserting the 2/2/2 distribution. After removing a node, it waits until that
node has been removed from bootstrap tracking before asserting the 3/3 distribution
Check List
Tests
Questions
Will it cause performance regression or break compatibility?
Do you need to update user documentation, design documentation or monitoring documentation?
Release note
Summary by CodeRabbit