From b60f494c1e07a6fdf39863fe05dde56a766b0021 Mon Sep 17 00:00:00 2001 From: Jeromy Date: Mon, 20 Jul 2015 15:46:12 -0700 Subject: [PATCH 1/2] fix race condition in notifications test License: MIT Signed-off-by: Jeromy --- p2p/net/mock/mock_notif_test.go | 41 ++++++++++++++++----------------- 1 file changed, 20 insertions(+), 21 deletions(-) diff --git a/p2p/net/mock/mock_notif_test.go b/p2p/net/mock/mock_notif_test.go index 1ea9df424..fbd197c11 100644 --- a/p2p/net/mock/mock_notif_test.go +++ b/p2p/net/mock/mock_notif_test.go @@ -7,6 +7,7 @@ import ( ma "github.com/ipfs/go-ipfs/Godeps/_workspace/src/github.com/jbenet/go-multiaddr" context "github.com/ipfs/go-ipfs/Godeps/_workspace/src/golang.org/x/net/context" inet "github.com/ipfs/go-ipfs/p2p/net" + peer "github.com/ipfs/go-ipfs/p2p/peer" ) func TestNotifications(t *testing.T) { @@ -42,31 +43,29 @@ func TestNotifications(t *testing.T) { // test everyone got the correct connection opened calls for i, s := range nets { n := notifiees[i] - for _, s2 := range nets { - var actual []inet.Conn - for len(s.ConnsToPeer(s2.LocalPeer())) != len(actual) { - select { - case c := <-n.connected: - actual = append(actual, c) - case <-time.After(timeout): - t.Fatal("timeout") + notifs := make(map[peer.ID]inet.Conn) + for j := 0; j < len(nets)-1; j++ { + select { + case c := <-n.connected: + _, ok := notifs[c.RemotePeer()] + if ok { + t.Fatal("shouldnt have received more than one connection per peer") } + notifs[c.RemotePeer()] = c + case <-time.After(timeout): + t.Fatal("timeout") + } + } + + for p, con := range notifs { + expect := s.ConnsToPeer(p) + if len(expect) != 1 { + t.Fatal("got more than one connection, not supposed to happen") } - expect := s.ConnsToPeer(s2.LocalPeer()) - for _, c1 := range actual { - found := false - for _, c2 := range expect { - if c1 == c2 { - found = true - break - } - } - if !found { - t.Error("connection not found", c1, len(expect), len(actual)) - } + if expect[0] != con { + t.Fatal("got different connection than we expected") } - } } From 47cd70fa7dffc14e2658e55336719b760d5dea42 Mon Sep 17 00:00:00 2001 From: Jeromy Date: Mon, 20 Jul 2015 16:29:33 -0700 Subject: [PATCH 2/2] fix same test in swarm License: MIT Signed-off-by: Jeromy --- p2p/net/mock/mock_notif_test.go | 44 ++++++++++++++++++++----------- p2p/net/swarm/swarm_notif_test.go | 31 ++++++++++++++-------- 2 files changed, 49 insertions(+), 26 deletions(-) diff --git a/p2p/net/mock/mock_notif_test.go b/p2p/net/mock/mock_notif_test.go index fbd197c11..f7a3ddb8a 100644 --- a/p2p/net/mock/mock_notif_test.go +++ b/p2p/net/mock/mock_notif_test.go @@ -43,28 +43,42 @@ func TestNotifications(t *testing.T) { // test everyone got the correct connection opened calls for i, s := range nets { n := notifiees[i] - notifs := make(map[peer.ID]inet.Conn) - for j := 0; j < len(nets)-1; j++ { - select { - case c := <-n.connected: - _, ok := notifs[c.RemotePeer()] - if ok { - t.Fatal("shouldnt have received more than one connection per peer") + notifs := make(map[peer.ID][]inet.Conn) + for j, s2 := range nets { + if i == j { + continue + } + + // this feels a little sketchy, but its probably okay + for len(s.ConnsToPeer(s2.LocalPeer())) != len(notifs[s2.LocalPeer()]) { + select { + case c := <-n.connected: + nfp := notifs[c.RemotePeer()] + notifs[c.RemotePeer()] = append(nfp, c) + case <-time.After(timeout): + t.Fatal("timeout") } - notifs[c.RemotePeer()] = c - case <-time.After(timeout): - t.Fatal("timeout") } } - for p, con := range notifs { + for p, cons := range notifs { expect := s.ConnsToPeer(p) - if len(expect) != 1 { - t.Fatal("got more than one connection, not supposed to happen") + if len(expect) != len(cons) { + t.Fatal("got different number of connections") } - if expect[0] != con { - t.Fatal("got different connection than we expected") + for _, c := range cons { + var found bool + for _, c2 := range expect { + if c == c2 { + found = true + break + } + } + + if !found { + t.Fatal("connection not found!") + } } } } diff --git a/p2p/net/swarm/swarm_notif_test.go b/p2p/net/swarm/swarm_notif_test.go index b44b030d8..9b0e3a9ba 100644 --- a/p2p/net/swarm/swarm_notif_test.go +++ b/p2p/net/swarm/swarm_notif_test.go @@ -8,6 +8,7 @@ import ( context "github.com/ipfs/go-ipfs/Godeps/_workspace/src/golang.org/x/net/context" inet "github.com/ipfs/go-ipfs/p2p/net" + peer "github.com/ipfs/go-ipfs/p2p/peer" ) func TestNotifications(t *testing.T) { @@ -37,35 +38,43 @@ func TestNotifications(t *testing.T) { // test everyone got the correct connection opened calls for i, s := range swarms { n := notifiees[i] - for _, s2 := range swarms { - if s == s2 { + notifs := make(map[peer.ID][]inet.Conn) + for j, s2 := range swarms { + if i == j { continue } - var actual []inet.Conn - for len(s.ConnectionsToPeer(s2.LocalPeer())) != len(actual) { + // this feels a little sketchy, but its probably okay + for len(s.ConnectionsToPeer(s2.LocalPeer())) != len(notifs[s2.LocalPeer()]) { select { case c := <-n.connected: - actual = append(actual, c) + nfp := notifs[c.RemotePeer()] + notifs[c.RemotePeer()] = append(nfp, c) case <-time.After(timeout): t.Fatal("timeout") } } + } - expect := s.ConnectionsToPeer(s2.LocalPeer()) - for _, c1 := range actual { - found := false + for p, cons := range notifs { + expect := s.ConnectionsToPeer(p) + if len(expect) != len(cons) { + t.Fatal("got different number of connections") + } + + for _, c := range cons { + var found bool for _, c2 := range expect { - if c1 == c2 { + if c == c2 { found = true break } } + if !found { - t.Error("connection not found") + t.Fatal("connection not found!") } } - } }