Skip to content
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

Deflake test #8154

Conversation

@illicitonion
Copy link
Contributor

commented Aug 9, 2019

Narrow the critical window where the test could fail.

Fixes #7101

@illicitonion illicitonion requested a review from blorente Aug 9, 2019

Deflake test
Widen the critical window where the test could fail.

@illicitonion illicitonion force-pushed the twitter:dwagnerhall/deflaking/waits_if_all_unhealthy branch from 5398bc2 to 9904415 Aug 9, 2019

// The serverset is configured to block for 10ms; make sure we waited for at least 9ms for
// stability.
assert!(start.elapsed() > Duration::from_millis(9))
// The serverset is configured to block for 20ms; make sure we waited for at least 10ms for

This comment has been minimized.

Copy link
@pierrechevalier83

pierrechevalier83 Aug 9, 2019

Contributor

I don't see the change that makes this go from 10 to 20 ms. Can you explain what you have in mind?

This comment has been minimized.

Copy link
@illicitonion

illicitonion Aug 9, 2019

Author Contributor

Added comment

@@ -390,7 +390,7 @@ mod tests {
let s = Serverset::new(vec!["good", "bad"], backoff_config).unwrap();
let mut runtime = tokio::runtime::Runtime::new().unwrap();

for _ in 0..2 {
for _ in 0..4 {

This comment has been minimized.

Copy link
@pierrechevalier83

pierrechevalier83 Aug 9, 2019

Contributor

Not sure what this does. Can you explain? (preferentially in the code, maybe by naming this magic number)

This comment has been minimized.

Copy link
@illicitonion

illicitonion Aug 9, 2019

Author Contributor

Added comment

@pierrechevalier83
Copy link
Contributor

left a comment

I need a bit more context to understand this change.

@blorente
Copy link
Contributor

left a comment

Thanks for adding the additional comments :)

@pierrechevalier83
Copy link
Contributor

left a comment

Thanks for the comments and the deflaking 🎉

@stuhood
stuhood approved these changes Aug 9, 2019

@illicitonion illicitonion merged commit a6c88e6 into pantsbuild:master Aug 12, 2019

1 check failed

continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details

@illicitonion illicitonion deleted the twitter:dwagnerhall/deflaking/waits_if_all_unhealthy branch Aug 12, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.