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
[IMPROVED] Fix flaky tests #1564
Conversation
Signed-off-by: Piotr Piotrowski <piotr@synadia.com>
ccc39ed
to
f54e634
Compare
Signed-off-by: Piotr Piotrowski <piotr@synadia.com>
f54e634
to
c3e230f
Compare
Signed-off-by: Piotr Piotrowski <piotr@synadia.com>
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.
Nice improvement of the CI relaiblity. Howeve, I think it would be nice to increase to Durations to make it more durable against slow CI workers.
// we know this request will take longer so extend the timeout | ||
expirationTimer.Reset(100 * time.Millisecond) | ||
|
||
// slower reply which would have hit original timeout | ||
time.Sleep(90 * time.Millisecond) | ||
time.Sleep(70 * time.Millisecond) |
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.
What's the reason for this change here?
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.
This test depends on request either timing out or not depending on the circumstances. The expiration timer is reset in each subscribe callback to 100ms - sleeping for 90ms was making it fail quite often if there the subsequent request had 10ms delay. Now it's 30ms "buffer", which makes it less, but still, flaky...
I get why this test is useful but its tough to fix it entirely while retaining the logic.
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.
Oh, gotcha. That feels pretty flaky no matter what we set here, seeing faster or slower machines running it...
Signed-off-by: Piotr Piotrowski <piotr@synadia.com>
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!
Signed-off-by: Piotr Piotrowski piotr@synadia.com