-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
autonat: fix flaky TestAutoNATPrivate #1581
Conversation
p2p/host/autonat/autonat_test.go
Outdated
@@ -121,8 +122,8 @@ func TestAutoNATPrivate(t *testing.T) { | |||
connect(t, hs, hc) | |||
require.Eventually(t, | |||
func() bool { return an.Status() == network.ReachabilityPrivate }, | |||
2*time.Second, | |||
25*time.Millisecond, | |||
testutils.ScaleDuration(2*time.Second), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any idea why this takes so long? I would've expected 2s to be a long timeout already.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know this doesn't answer your real question (why does this take so long to get to that status). But on my laptop this takes 1 second.
=== RUN TestAutoNATPrivate
Took: 1.028655875s to see NAT status as private
--- PASS: TestAutoNATPrivate (1.03s)
So it makes sense that CI would fail here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's this second:
go-libp2p/p2p/host/autonat/autonat.go
Lines 256 to 262 in e732bda
// Don't look for peers in the peer store more than once per second. | |
if !as.lastProbeTry.IsZero() { | |
backoff := as.lastProbeTry.Add(time.Second) | |
if backoff.After(nextProbe) { | |
nextProbe = backoff | |
} | |
} |
Seems like 20s is way too much on CI. Maybe we can just set to 3s?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's the upper limit, but sure updated :)
* Use expectEvent and scale duration for CI * Use 3 seconds
Fixes #1566 by scaling duration 3/2x. Waits on a channel.