-
Notifications
You must be signed in to change notification settings - Fork 116
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
Terminate timeloop faster for m3 #185
Conversation
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.
Thanks!
(you probably need another review though)
Production tests fail with errors that timeLoop goroutine is leaking, investigating, I suspect that production is not always properly discarding tally |
So it seems code is correct, indeed there are a lot of leaking goroutines because tally reporter is not properly closed. So this change potentially can introduce regression for tests which try to detect leaking goroutines. |
Codecov Report
@@ Coverage Diff @@
## master #185 +/- ##
==========================================
+ Coverage 47.31% 47.36% +0.04%
==========================================
Files 64 64
Lines 5932 5937 +5
==========================================
+ Hits 2807 2812 +5
Misses 2685 2685
Partials 440 440
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
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.
Thanks! Just fix lint and then lgtm.
We quickly start and stop process (it is wrapper for another command) and timeloop potentially can delay process shutdown by up-to 100ms (_timeResolution). Instead of just sleeping we as well wait for signal that shutdown is in progress and quickly terminate loop.