Commit a8f84cd3 authored by Adrian Sutton's avatar Adrian Sutton

op-node/p2p: Fix flaky TestDiscovery

Prevously the test waited for the first two peer connections rather than waiting for the two expected peers to connect.
Commonly the bootnode will connect inbound to peerB at the same time as peerB connects outbound to the bootnode, resulting in two connection events.
parent 6f6a308c
...@@ -32,6 +32,7 @@ require ( ...@@ -32,6 +32,7 @@ require (
github.com/urfave/cli v1.22.9 github.com/urfave/cli v1.22.9
github.com/urfave/cli/v2 v2.17.2-0.20221006022127-8f469abc00aa github.com/urfave/cli/v2 v2.17.2-0.20221006022127-8f469abc00aa
golang.org/x/crypto v0.6.0 golang.org/x/crypto v0.6.0
golang.org/x/exp v0.0.0-20230213192124-5e25df0256eb
golang.org/x/term v0.5.0 golang.org/x/term v0.5.0
) )
...@@ -174,7 +175,6 @@ require ( ...@@ -174,7 +175,6 @@ require (
go.uber.org/fx v1.19.1 // indirect go.uber.org/fx v1.19.1 // indirect
go.uber.org/multierr v1.9.0 // indirect go.uber.org/multierr v1.9.0 // indirect
go.uber.org/zap v1.24.0 // indirect go.uber.org/zap v1.24.0 // indirect
golang.org/x/exp v0.0.0-20230213192124-5e25df0256eb // indirect
golang.org/x/mod v0.8.0 // indirect golang.org/x/mod v0.8.0 // indirect
golang.org/x/net v0.7.0 // indirect golang.org/x/net v0.7.0 // indirect
golang.org/x/sync v0.1.0 // indirect golang.org/x/sync v0.1.0 // indirect
......
...@@ -19,6 +19,7 @@ import ( ...@@ -19,6 +19,7 @@ import (
mocknet "github.com/libp2p/go-libp2p/p2p/net/mock" mocknet "github.com/libp2p/go-libp2p/p2p/net/mock"
tswarm "github.com/libp2p/go-libp2p/p2p/net/swarm/testing" tswarm "github.com/libp2p/go-libp2p/p2p/net/swarm/testing"
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
"golang.org/x/exp/slices"
"github.com/ethereum/go-ethereum/log" "github.com/ethereum/go-ethereum/log"
"github.com/ethereum/go-ethereum/p2p/enode" "github.com/ethereum/go-ethereum/p2p/enode"
...@@ -314,19 +315,22 @@ func TestDiscovery(t *testing.T) { ...@@ -314,19 +315,22 @@ func TestDiscovery(t *testing.T) {
// B and C don't know each other yet, but both have A as a bootnode. // B and C don't know each other yet, but both have A as a bootnode.
// It should only be a matter of time for them to connect, if they discover each other via A. // It should only be a matter of time for them to connect, if they discover each other via A.
var firstPeersOfB []peer.ID timeout := time.After(time.Second * 10)
for i := 0; i < 2; i++ { var peersOfB []peer.ID
// B should be connected to the bootnode (A) it used (it's a valid optimism node to connect to here)
// C should also be connected, although this one might take more time to discover
for !slices.Contains(peersOfB, hostA.ID()) || !slices.Contains(peersOfB, hostC.ID()) {
select { select {
case <-time.After(time.Second * 30): case <-timeout:
t.Fatal("failed to get connection to B in time") var peers []string
for _, id := range peersOfB {
peers = append(peers, id.String())
}
t.Fatalf("timeout reached - expected host A: %v and host C: %v to be in %v", hostA.ID().String(), hostC.ID().String(), peers)
case c := <-connsB: case c := <-connsB:
firstPeersOfB = append(firstPeersOfB, c.RemotePeer()) peersOfB = append(peersOfB, c.RemotePeer())
} }
} }
// B should be connected to the bootnode it used (it's a valid optimism node to connect to here)
require.Contains(t, firstPeersOfB, hostA.ID())
// C should be connected, although this one might take more time to discover
require.Contains(t, firstPeersOfB, hostC.ID())
} }
// Most tests should use mocknets instead of using the actual local host network // Most tests should use mocknets instead of using the actual local host network
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment