Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Yet another flaky test (or a flaky function!) causing rare failures. #128

Closed
johnsonjh opened this issue Oct 31, 2020 · 6 comments
Closed

Comments

@johnsonjh
Copy link

johnsonjh commented Oct 31, 2020

peer: TestPeerConnection test:

--- FAIL: TestPeerConnection (0.00s)
    peer_test.go:342: Running 2 tests
    peer_test.go:211: testPeer: wrong LastSend - got 2020-10-31 20:16:07 +0000 GMT, want 1970-01-01 00:00:00 +0000 GMT
cd peer && go test -count=10000 . | grep 'FAIL: TestPeerConnection' | wc -l`
90

So, the test passes 99.1% of the time.

Will look into it and see what the source of this failure may be.

Originally posted by @johnsonjh in #127 (comment)

@johnsonjh
Copy link
Author

johnsonjh commented Nov 3, 2020

Test improvements in some of these 42 PR's I currently have pending (and I'm not even sure which ones!) have helped, but not eliminated this particular error.

It was always about ~90 [+/- 15] failures with develop branch as of 75e4cd861bc3f89dd3dce1aa8d8529120a1c3166. After applying all outstanding PR's, it's now ~25 [+/- 5] out of 10,000 test runs, consistently. The results are the same with Go 1.16-pre and Go 1.15.3.

@johnsonjh
Copy link
Author

johnsonjh commented Nov 4, 2020

e99926c#diff-fe29af682b09b1e9bf8169d7c1d2d1f971e70f8f48e7ad4dbb1f624ded93d2f3 is the commit that solved most of the issues - this whole thing is done in a way that's inherently race-prone, but that's seemingly not only the issue here - I'll see if I can find a solution.

@johnsonjh
Copy link
Author

It's a hackjob if workaround, but so is a lot of this code:

» go test -count=25000 .
ok  	github.com/pkt-cash/pktd/peer	41.565s

I'll push the commit into PR #161 to fix it.

@johnsonjh
Copy link
Author

Added the commit to PR #161, so we can close out #128 if it's merged.

@johnsonjh
Copy link
Author

Pretty sure the actual root cause is the race at

github.com/pkt-cash/pktd/addrmgr.(*KnownAddress).LastAttempt()
github.com/pkt-cash/pktd/addrmgr/knownaddress.go:34 +0x5d0

as noted at #170

@johnsonjh
Copy link
Author

johnsonjh commented Nov 18, 2020

@cjdelisle

A proper fix for this test is now pending as PR #212 - if that is accepted and merged, then this issue can be closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant