tapchannel: relax timing for quit and cancel tests #1147
Conversation
Pull Request Test Coverage Report for Build 11292969215Details
💛 - Coveralls |
tapchannel/auf_leaf_signer_test.go
Outdated
| @@ -207,7 +207,7 @@ func TestAuxLeafSignerCancelAndQuit(t *testing.T) { | |||
| // Another component could have sent the cancel signal; we'll | |||
| // send that before the quit signal. | |||
| close(cancelChan) | |||
| time.Sleep(time.Millisecond) | |||
| time.Sleep(time.Microsecond * 250) | |||
There was a problem hiding this comment.
Isn't this the opposite of "relaxing" the timing? Since this is now 0.25 milliseconds as before it was 1 millisecond?
There was a problem hiding this comment.
The wording is sort of flipped; the 'timing' here is how long the aux signer will take to skip the remaining sig jobs.
Ideally, the last job is never processed after the quit signal is sent. I didn't account for the fact that either case of the quit-cancel switch may fire, and that jobs will get processed a lot faster once one signal is received.
After adding jobs and decreasing the timeout, I can no longer repro this flake.
There was a problem hiding this comment.
Can we get rid of this sleep?
Actually, yes - I think there doesn't need to be known delay between the cancel and the quit, it was a leftover from fixing the initial issue with these signals.
jharveyb
force-pushed
the
aux_signer_test_race_fixes
branch
2 times, most recently
from
07948f3 to
4f10384
Compare
Testing aux signer quit behavior involved building a job queue and then checking that some portion of the jobs were skipped, and that the last job was not processed. This led to racy behavior if the job queue was too short, or the delay after sending a signal was too long. Using a larger queue and shorter delay reduces the odds of a race here.
jharveyb
force-pushed
the
aux_signer_test_race_fixes
branch
from
4f10384 to
b6a7d60
Compare
Fixes #1143 . I had replicated the original failure locally, race tested this change for 100 iterations locally.