Fixing the flaky middleware unsubscribe test #1173
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1173 +/- ##
==========================================
- Coverage 55.59% 55.57% -0.03%
==========================================
Files 481 481
Lines 29449 29449
==========================================
- Hits 16372 16366 -6
- Misses 10844 10848 +4
- Partials 2233 2235 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
vishalchangrani
force-pushed
the
vishal/flaky_middleware_test
branch
from
f4e7267
to
12e7176
Compare
vishalchangrani
force-pushed
the
vishal/flaky_middleware_test
branch
from
12e7176
to
8e0769a
Compare
|
|
||
| // first test that when both nodes are subscribed to the channel, the target node receives the message | ||
| err := m.mws[first].Publish(message1, testChannel) | ||
| assert.NoError(m.T(), err) | ||
|
|
||
| assert.Eventually(m.T(), func() bool { | ||
| return m.ov[last].AssertCalled(m.T(), "Receive", firstNode, mockery.Anything) |
There was a problem hiding this comment.
AssertCalled calls Fail if the assertion doesn't succeed. This caused the flakiness if the message was not delivered in 1 millisecond before the first call to AssertCalled. Subsequent calls may succeed but by then the test has already been marked as failed :(
|
bors merge |
|
👎 Rejected by too few approved reviews |
There was a problem hiding this comment.
This makes sense!
OK, is this true of other assert calls?
I'm worried about these in our code base:
https://gist.github.com/3eecce8d13fc60e45180039855171324
|
|
||
| // first test that when both nodes are subscribed to the channel, the target node receives the message | ||
| err := m.mws[first].Publish(message1, testChannel) | ||
| assert.NoError(m.T(), err) | ||
|
|
||
| assert.Eventually(m.T(), func() bool { | ||
| return m.ov[last].AssertCalled(m.T(), "Receive", firstNode, mockery.Anything) |
I think so - the |
|
bors merge |
1173: Fixing the flaky middleware unsubscribe test r=vishalchangrani a=vishalchangrani This passes on this branch locally while fails immediately on master: `go test -tags relic -run TestMiddlewareTestSuit ./network/test -v -race -count 10 -failfast` Co-authored-by: vishal <1117327+vishalchangrani@users.noreply.github.com> Co-authored-by: Vishal <1117327+vishalchangrani@users.noreply.github.com>
|
This PR was included in a batch that successfully built, but then failed to merge into master (it was a non-fast-forward update). It will be automatically retried. |
1173: Fixing the flaky middleware unsubscribe test r=vishalchangrani a=vishalchangrani This passes on this branch locally while fails immediately on master: `go test -tags relic -run TestMiddlewareTestSuit ./network/test -v -race -count 10 -failfast` Co-authored-by: vishal <1117327+vishalchangrani@users.noreply.github.com> Co-authored-by: Vishal <1117327+vishalchangrani@users.noreply.github.com>
|
Thanks @vishalchangrani ! |
|
Build failed: |
This passes on this branch locally while fails immediately on master:
go test -tags relic -run TestMiddlewareTestSuit ./network/test -v -race -count 10 -failfast