From 8a62a14374da474ec6dfd5ba9121758bb41daeab Mon Sep 17 00:00:00 2001 From: Matt Joiner Date: Tue, 5 Feb 2019 10:35:37 +1100 Subject: [PATCH] Deflake TestFindPeerQuery (#245) * Rework TestFindPeersQuery Unravel the logic and create a minimal test case that isn't flaky. * Use testing.T.Logf * Skip the original test in short mode * Add comments for the arguments to testFindPeerQuery * Qualify aberrant package name * Use redundant package names * gx import testify 1.3.0. --- dht_test.go | 114 +++++++++++++++++++++------------------------- nofile_posix.go | 11 +++++ nofile_windows.go | 5 ++ package.json | 7 +++ 4 files changed, 76 insertions(+), 61 deletions(-) create mode 100644 nofile_posix.go create mode 100644 nofile_windows.go diff --git a/dht_test.go b/dht_test.go index fbea22410c2..2e5196120a8 100644 --- a/dht_test.go +++ b/dht_test.go @@ -5,6 +5,8 @@ import ( "context" "errors" "fmt" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "math/rand" "sort" "strings" @@ -1158,101 +1160,91 @@ func TestClientModeFindPeer(t *testing.T) { } } +func minInt(a, b int) int { + if a < b { + return a + } else { + return b + } +} + +func TestFindPeerQueryMinimal(t *testing.T) { + testFindPeerQuery(t, 2, 22, 11) +} + func TestFindPeerQuery(t *testing.T) { + if testing.Short() { + t.Skip("skipping test in short mode") + } + if curFileLimit() < 1024 { + t.Skip("insufficient file descriptors available") + } + testFindPeerQuery(t, 20, 80, 16) +} + +func testFindPeerQuery(t *testing.T, + bootstrappers, // Number of nodes connected to the querying node + leafs, // Number of nodes that might be connected to from the bootstrappers + bootstrapperLeafConns int, // Number of connections each bootstrapper has to the leaf nodes +) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - nDHTs := 101 - _, allpeers, dhts := setupDHTS(ctx, nDHTs, t) + _, allpeers, dhts := setupDHTS(ctx, 1+bootstrappers+leafs, t) defer func() { - for i := 0; i < nDHTs; i++ { - dhts[i].Close() - defer dhts[i].host.Close() + for _, d := range dhts { + d.Close() + d.host.Close() } }() mrand := rand.New(rand.NewSource(42)) guy := dhts[0] others := dhts[1:] - for i := 0; i < 20; i++ { - for j := 0; j < 16; j++ { // 16, high enough to probably not have any partitions - v := mrand.Intn(80) - connect(t, ctx, others[i], others[20+v]) + for i := 0; i < bootstrappers; i++ { + for j := 0; j < bootstrapperLeafConns; j++ { + v := mrand.Intn(leafs) + connect(t, ctx, others[i], others[bootstrappers+v]) } } - for i := 0; i < 20; i++ { + for i := 0; i < bootstrappers; i++ { connect(t, ctx, guy, others[i]) } + var reachableIds []peer.ID + for i, d := range dhts { + lp := len(d.host.Network().Peers()) + //t.Log(i, lp) + if i != 0 && lp > 0 { + reachableIds = append(reachableIds, allpeers[i]) + } + } + t.Logf("%d reachable ids", len(reachableIds)) + val := "foobar" rtval := kb.ConvertKey(val) rtablePeers := guy.routingTable.NearestPeers(rtval, AlphaValue) - if len(rtablePeers) != 3 { - t.Fatalf("expected 3 peers back from routing table, got %d", len(rtablePeers)) - } + assert.Len(t, rtablePeers, minInt(bootstrappers, AlphaValue)) - netpeers := guy.host.Network().Peers() - if len(netpeers) != 20 { - t.Fatalf("expected 20 peers to be connected, got %d", len(netpeers)) - } - - rtableset := make(map[peer.ID]bool) - for _, p := range rtablePeers { - rtableset[p] = true - } + assert.Len(t, guy.host.Network().Peers(), bootstrappers) out, err := guy.GetClosestPeers(ctx, val) - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) - var notfromrtable int - var count int var outpeers []peer.ID for p := range out { - count++ - if !rtableset[p] { - notfromrtable++ - } outpeers = append(outpeers, p) } - if notfromrtable == 0 { - t.Fatal("got entirely peers from our routing table") - } - - if count != 20 { - t.Fatal("should have only gotten 20 peers from getclosestpeers call") - } - - sort.Sort(peer.IDSlice(allpeers[1:])) sort.Sort(peer.IDSlice(outpeers)) - actualclosest := kb.SortClosestPeers(allpeers[1:], rtval) - exp := actualclosest[:20] + exp := kb.SortClosestPeers(reachableIds, rtval)[:minInt(KValue, len(reachableIds))] + t.Logf("got %d peers", len(outpeers)) got := kb.SortClosestPeers(outpeers, rtval) - diffp := countDiffPeers(exp, got) - if diffp > 0 { - // could be a partition created during setup - t.Fatal("didnt get expected closest peers") - } -} - -func countDiffPeers(a, b []peer.ID) int { - s := make(map[peer.ID]bool) - for _, p := range a { - s[p] = true - } - var out int - for _, p := range b { - if !s[p] { - out++ - } - } - return out + assert.EqualValues(t, exp, got) } func TestFindClosestPeers(t *testing.T) { diff --git a/nofile_posix.go b/nofile_posix.go new file mode 100644 index 00000000000..1b07e318f79 --- /dev/null +++ b/nofile_posix.go @@ -0,0 +1,11 @@ +// +build !windows + +package dht + +import "syscall" + +func curFileLimit() uint64 { + var n syscall.Rlimit + syscall.Getrlimit(syscall.RLIMIT_NOFILE, &n) + return n.Cur +} diff --git a/nofile_windows.go b/nofile_windows.go new file mode 100644 index 00000000000..888aec0c754 --- /dev/null +++ b/nofile_windows.go @@ -0,0 +1,5 @@ +package dht + +func curFileLimit() uint64 { + return 16 * 1024 * 1024 +} diff --git a/package.json b/package.json index b4fa975cb24..fd42a47bd77 100644 --- a/package.json +++ b/package.json @@ -159,6 +159,12 @@ "hash": "QmegQFxhr1J6yZ1vDQuDmJi5jntmj6BL96S11HVtXNCaHb", "name": "go-libp2p-swarm", "version": "3.0.28" + }, + { + "author": "magik6k", + "hash": "QmYNszQ8MWA7o9qhjTZrp6R9DXNn1fvTZufyb6LmKHcgcr", + "name": "testify", + "version": "1.3.0" } ], "gxVersion": "0.4.0", @@ -168,3 +174,4 @@ "releaseCmd": "git commit -a -m \"gx publish $VERSION\"", "version": "4.4.19" } +