Conversation
|
thank you @lanc07 It's late for me right now and the CI seems to be slow today :) I'll look at this the first thing in the morning. Thank you so much. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #514 +/- ##
==========================================
+ Coverage 89.55% 89.57% +0.02%
==========================================
Files 56 56
Lines 4642 4642
==========================================
+ Hits 4157 4158 +1
+ Misses 485 484 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
8691a05 to
784cf98
Compare
| ok = rt.start() | ||
| assert.False(t, ok, "start() should NOT succeed once closed") | ||
| assert.True(t, rt.isRunning(), "should be running") | ||
|
|
||
| select { | ||
| case <-timedOut: | ||
| case <-time.After(ackInterval * 2): | ||
| assert.Fail(t, "should be called once") | ||
| } | ||
|
|
||
| select { | ||
| case <-timedOut: | ||
| assert.Fail(t, "should be called once") | ||
| case <-time.After(ackInterval * 2): | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
This looks like a good direction, thanks for working on moving these timer tests to synctest.
The missing thing is that it needs to lean a bit more on synctest itself rather than replacing the existing Sleep calls with select + time.After. Inside synctest.Test, fake time will advance automatically, and synctest.Wait() gives us the synchronization we need without having to actually wait, so for most of these tests, I think the simpler pattern would be:
time.Sleep(...)
synctest.Wait()
assert.Equal(t, expected, callbacks)to just replace the original sleep and after stopping a timer, we can advance fake time past when it would have fired, call synctest.Wait() for that.
Happy to help and explain more
2 participants
Description
Migrate to synctest to avoid flaky and slow tests