Add a pause to fix flaky getstreamex forwarding test. #262
Conversation
core/node/rpc/service_test.go
Outdated
| // quorum will be achieved and the createUser call will return when a node may not have received the miniblock, | ||
| // and we could occasionally end up with a stream response that does not contain the genesis miniblock. | ||
| // This is why we sleep for a bit before making the stream requests. | ||
| time.Sleep(2 * time.Second) |
There was a problem hiding this comment.
Adding sleep in test quite often makes flake less probably, but doesn't eliminate it completely. But also it slows down test unnecessarily.
I was under the impression that make_miniblock above should be consistent.
If this is not the case, would using Eventually with a bit larger timeout work out better? https://pkg.go.dev/github.com/stretchr/testify/require#Assertions.Eventually
There was a problem hiding this comment.
Let's discuss? To make sure I understand the code.
I think the debug make miniblock method will only make a miniblock and commit it to storage on either a single node, but it won't necessarily require the miniblocks to be replicated synchronously to all nodes fora stream. It calls SetStreamLastMiniblock on a node, which emits an event... but there's no guarantee the event will be processed by all nodes for a stream before the call returns. These tests are set up with a replication factor of 5.
There was a problem hiding this comment.
I see, thanks for clarifying!
In this case it's better to use require.EventuallyXxx than time.Sleep since Eventually will succeed once the condition is true (after tick delay), so test is going to complete faster. With Sleep test is always going to be slow.
Testing again #262 --------- Co-authored-by: Crystal Lemire <[email protected]>
I think inserting a short sleep will fix the flaky getstreamex forwarding test, but if it happens again I will disable it.