-
Notifications
You must be signed in to change notification settings - Fork 455
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[tests] Update peer bootstrap none available integration test #3282
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3282 +/- ##
=========================================
- Coverage 72.4% 72.4% -0.1%
=========================================
Files 1099 1099
Lines 101504 101551 +47
=========================================
+ Hits 73579 73608 +29
- Misses 22849 22866 +17
- Partials 5076 5077 +1
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report at Codecov.
|
log.Debug("servers are now up") | ||
|
||
for i, s := range setups { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we run this with a timeout (e.g. waitUntil(isBootstrapped(), TIMEOUT)
)?
Feels like this could be flaky.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
StartServer()
implicitly waits for bootstrap. These asserts are not really needed, added them just to make it clearer what's being tested.
m3/src/dbnode/integration/setup.go
Lines 757 to 760 in 586ccff
waitFn := ts.WaitUntilServerIsUp | |
if waitForBootstrap { | |
waitFn = ts.WaitUntilServerIsBootstrapped | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
# Conflicts: # src/dbnode/integration/peers_bootstrap_none_available_test.go
What this PR does / why we need it:
Updates peer bootstrap none available test to not rely on
NoOpAllBootstrapper
.Special notes for your reviewer:
Does this PR introduce a user-facing and/or backwards incompatible change?:
Does this PR require updating code package or user-facing documentation?: