Fix flaky registry tests #938
Conversation
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. |
|
|
||
| sub := registry.OnChangedNode(1) | ||
| getCurrentCount := r.CountChannel(sub) | ||
| go func() { |
There was a problem hiding this comment.
there was a race. If this goroutine got the message, it would check the http address and the test would fail. In about 99% of the cases the countChannel would win and this check never got executed.
| require.Equal(t, getCurrentCount(), 1) | ||
| } | ||
|
|
||
| func TestStopOnContextCancel(t *testing.T) { |
There was a problem hiding this comment.
I understand the idea behind the test but since it does not generate new changes after shutdown, it does not test the stop.
Also, we rely on GC to close the channels so we can't even check that the channel got closed (which would probably do some nasty things to CountChannel) while we are holding a pointer to it.,
Fix flaky registry tests by removing TestStopOnContextCancel and modifying TestContractRegistryChangedNodes to use validator functions with CountChannel
TestStopOnContextCanceltest function entirely from contractRegistry_test.goTestContractRegistryChangedNodesin contractRegistry_test.go to use a validator function withCountChannelinstead of a separate goroutine for processing subscription channelsCountChannelfunction in notifier_test.go to accept optional validator functions that validate each value received from the channel before incrementing the count📍Where to Start
Start with the modified
CountChannelfunction in notifier_test.go to understand the new validator functionality, then review the changes toTestContractRegistryChangedNodesin contractRegistry_test.go.Macroscope summarized c8ed51a.