From 86d2db5cc2cc6e5419f70064ea73481ae99351f1 Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Thu, 10 Dec 2015 10:54:10 +0000 Subject: [PATCH 1/3] better emulation of connections in gossip tests real Connections do not process gossip until after AddConnection. mockChannelConnection wasn't behaving in that way, which resulted in some sporadic test failures since it would cause some gossip to get processed too early, which in turn would trigger gc of some of the just-received peers, which we would then find missing in the test assertions. To fix this we introduce a latch on mockChannelConnection which closes when the remote end is ready to receive gossip. Effectively this emulates the entering of LocalConnection.receiveTCP after the call to AddConnection completes. Unfortunately an open latch will cause sendPendingGossip to block. We get around that by abandoning checks in "half-connected" connection states. Fixes #1765. --- mesh/gossip_test.go | 82 ++++++++++++++++++++++++--------------------- 1 file changed, 43 insertions(+), 39 deletions(-) diff --git a/mesh/gossip_test.go b/mesh/gossip_test.go index 31166a90..1993a05b 100644 --- a/mesh/gossip_test.go +++ b/mesh/gossip_test.go @@ -13,7 +13,9 @@ import ( type mockChannelConnection struct { RemoteConnection - dest *Router + router *Router + dest *Router + start chan struct{} } func NewTestRouter(name string) *Router { @@ -24,11 +26,21 @@ func NewTestRouter(name string) *Router { } func (conn *mockChannelConnection) SendProtocolMsg(protocolMsg ProtocolMsg) { + <-conn.start if err := conn.dest.handleGossip(protocolMsg.tag, protocolMsg.msg); err != nil { panic(err) } } +func (conn *mockChannelConnection) Connect() { + conn.router.Ourself.handleAddConnection(conn) + conn.router.Ourself.handleConnectionEstablished(conn) +} + +func (conn *mockChannelConnection) Start() { + close(conn.start) +} + func sendPendingGossip(routers ...*Router) { // Loop until all routers report they didn't send anything for sentSomething := true; sentSomething; { @@ -39,16 +51,24 @@ func sendPendingGossip(routers ...*Router) { } } -func (router *Router) AddTestChannelConnection(r *Router) { +func AddTestGossipConnection(r1, r2 *Router) { + c1 := r1.NewTestChannelConnection(r2) + c2 := r2.NewTestChannelConnection(r1) + c1.Connect() + c2.Start() + c2.Connect() + c1.Start() +} + +func (router *Router) NewTestChannelConnection(r *Router) *mockChannelConnection { fromPeer := NewPeerFrom(router.Ourself.Peer) toPeer := NewPeerFrom(r.Ourself.Peer) r.Peers.FetchWithDefault(fromPeer) // Has side-effect of incrementing refcount toPeer = router.Peers.FetchWithDefault(toPeer) // - conn := &mockChannelConnection{RemoteConnection{router.Ourself.Peer, toPeer, "", false, true}, r} - router.Ourself.handleAddConnection(conn) - router.Ourself.handleConnectionEstablished(conn) + return &mockChannelConnection{ + RemoteConnection{router.Ourself.Peer, toPeer, "", false, true}, router, r, make(chan struct{})} } func (router *Router) DeleteTestChannelConnection(r *Router) { @@ -98,56 +118,42 @@ func TestGossipTopology(t *testing.T) { checkTopology(t, r2, r2.tp()) // Now try adding some connections - r1.AddTestChannelConnection(r2) - sendPendingGossip(r1, r2) - checkTopology(t, r1, r1.tp(r2), r2.tp()) - checkTopology(t, r2, r1.tp(r2), r2.tp()) - r2.AddTestChannelConnection(r1) + AddTestGossipConnection(r1, r2) sendPendingGossip(r1, r2) checkTopology(t, r1, r1.tp(r2), r2.tp(r1)) checkTopology(t, r2, r1.tp(r2), r2.tp(r1)) - // Currently, the connection from 2 to 3 is one-way only - r2.AddTestChannelConnection(r3) + AddTestGossipConnection(r2, r3) sendPendingGossip(r1, r2, r3) - checkTopology(t, r1, r1.tp(r2), r2.tp(r1, r3), r3.tp()) - checkTopology(t, r2, r1.tp(r2), r2.tp(r1, r3), r3.tp()) - // When r2 gossiped to r3, 1 was unreachable from r3 so it got removed from the - // list of peers, but remains referenced in the connection from 1 to 3. - checkTopology(t, r3, r2.tp(r1, r3), r3.tp()) + checkTopology(t, r1, r1.tp(r2), r2.tp(r1, r3), r3.tp(r2)) + checkTopology(t, r2, r1.tp(r2), r2.tp(r1, r3), r3.tp(r2)) + checkTopology(t, r3, r1.tp(r2), r2.tp(r1, r3), r3.tp(r2)) - // Add a connection from 3 to 1 and now r1 is reachable. - r3.AddTestChannelConnection(r1) + AddTestGossipConnection(r3, r1) sendPendingGossip(r1, r2, r3) - checkTopology(t, r1, r1.tp(r2), r2.tp(r1, r3), r3.tp(r1)) - checkTopology(t, r2, r1.tp(r2), r2.tp(r1, r3), r3.tp(r1)) - checkTopology(t, r3, r1.tp(), r2.tp(r1, r3), r3.tp(r1)) - - r1.AddTestChannelConnection(r3) - sendPendingGossip(r1, r2, r3) - checkTopology(t, r1, r1.tp(r2, r3), r2.tp(r1, r3), r3.tp(r1)) - checkTopology(t, r2, r1.tp(r2, r3), r2.tp(r1, r3), r3.tp(r1)) - checkTopology(t, r3, r1.tp(r2, r3), r2.tp(r1, r3), r3.tp(r1)) + checkTopology(t, r1, r1.tp(r2, r3), r2.tp(r1, r3), r3.tp(r1, r2)) + checkTopology(t, r2, r1.tp(r2, r3), r2.tp(r1, r3), r3.tp(r1, r2)) + checkTopology(t, r3, r1.tp(r2, r3), r2.tp(r1, r3), r3.tp(r1, r2)) // Drop the connection from 2 to 3 r2.DeleteTestChannelConnection(r3) sendPendingGossip(r1, r2, r3) - checkTopology(t, r1, r1.tp(r2, r3), r2.tp(r1), r3.tp(r1)) - checkTopology(t, r2, r1.tp(r2, r3), r2.tp(r1), r3.tp(r1)) - checkTopology(t, r3, r1.tp(r2, r3), r2.tp(r1), r3.tp(r1)) + checkTopology(t, r1, r1.tp(r2, r3), r2.tp(r1), r3.tp(r1, r2)) + checkTopology(t, r2, r1.tp(r2, r3), r2.tp(r1), r3.tp(r1, r2)) + checkTopology(t, r3, r1.tp(r2, r3), r2.tp(r1), r3.tp(r1, r2)) // Drop the connection from 1 to 3 r1.DeleteTestChannelConnection(r3) sendPendingGossip(r1, r2, r3) - checkTopology(t, r1, r1.tp(r2), r2.tp(r1), r3.tp(r1)) - checkTopology(t, r2, r1.tp(r2), r2.tp(r1)) + checkTopology(t, r1, r1.tp(r2), r2.tp(r1), r3.tp(r1, r2)) + checkTopology(t, r2, r1.tp(r2), r2.tp(r1), r3.tp(r1, r2)) // r3 still thinks r1 has a connection to it - checkTopology(t, r3, r1.tp(r2, r3), r2.tp(r1), r3.tp(r1)) + checkTopology(t, r3, r1.tp(r2, r3), r2.tp(r1), r3.tp(r1, r2)) // On a timer, r3 will gossip to r1 r3.SendAllGossip() sendPendingGossip(r1, r2, r3) - checkTopology(t, r1, r1.tp(r2), r2.tp(r1), r3.tp(r1)) + checkTopology(t, r1, r1.tp(r2), r2.tp(r1), r3.tp(r1, r2)) } func TestGossipSurrogate(t *testing.T) { @@ -155,10 +161,8 @@ func TestGossipSurrogate(t *testing.T) { r1 := NewTestRouter("01:00:00:01:00:00") r2 := NewTestRouter("02:00:00:02:00:00") r3 := NewTestRouter("03:00:00:03:00:00") - r1.AddTestChannelConnection(r2) - r2.AddTestChannelConnection(r1) - r3.AddTestChannelConnection(r2) - r2.AddTestChannelConnection(r3) + AddTestGossipConnection(r1, r2) + AddTestGossipConnection(r3, r2) sendPendingGossip(r1, r2, r3) checkTopology(t, r1, r1.tp(r2), r2.tp(r1, r3), r3.tp(r2)) checkTopology(t, r2, r1.tp(r2), r2.tp(r1, r3), r3.tp(r2)) From ed14561afdad0e3fb871f5bf86f6c10fcd356c17 Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Tue, 8 Dec 2015 23:21:31 +0000 Subject: [PATCH 2/3] refactor: rename --- mesh/gossip_test.go | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/mesh/gossip_test.go b/mesh/gossip_test.go index 1993a05b..f1782f11 100644 --- a/mesh/gossip_test.go +++ b/mesh/gossip_test.go @@ -11,7 +11,7 @@ import ( // TODO test gossip unicast; atm we only test topology gossip and // surrogates, neither of which employ unicast. -type mockChannelConnection struct { +type mockGossipConnection struct { RemoteConnection router *Router dest *Router @@ -25,19 +25,19 @@ func NewTestRouter(name string) *Router { return router } -func (conn *mockChannelConnection) SendProtocolMsg(protocolMsg ProtocolMsg) { +func (conn *mockGossipConnection) SendProtocolMsg(protocolMsg ProtocolMsg) { <-conn.start if err := conn.dest.handleGossip(protocolMsg.tag, protocolMsg.msg); err != nil { panic(err) } } -func (conn *mockChannelConnection) Connect() { +func (conn *mockGossipConnection) Connect() { conn.router.Ourself.handleAddConnection(conn) conn.router.Ourself.handleConnectionEstablished(conn) } -func (conn *mockChannelConnection) Start() { +func (conn *mockGossipConnection) Start() { close(conn.start) } @@ -52,26 +52,26 @@ func sendPendingGossip(routers ...*Router) { } func AddTestGossipConnection(r1, r2 *Router) { - c1 := r1.NewTestChannelConnection(r2) - c2 := r2.NewTestChannelConnection(r1) + c1 := r1.NewTestGossipConnection(r2) + c2 := r2.NewTestGossipConnection(r1) c1.Connect() c2.Start() c2.Connect() c1.Start() } -func (router *Router) NewTestChannelConnection(r *Router) *mockChannelConnection { +func (router *Router) NewTestGossipConnection(r *Router) *mockGossipConnection { fromPeer := NewPeerFrom(router.Ourself.Peer) toPeer := NewPeerFrom(r.Ourself.Peer) r.Peers.FetchWithDefault(fromPeer) // Has side-effect of incrementing refcount toPeer = router.Peers.FetchWithDefault(toPeer) // - return &mockChannelConnection{ + return &mockGossipConnection{ RemoteConnection{router.Ourself.Peer, toPeer, "", false, true}, router, r, make(chan struct{})} } -func (router *Router) DeleteTestChannelConnection(r *Router) { +func (router *Router) DeleteTestGossipConnection(r *Router) { fromName := router.Ourself.Peer.Name toName := r.Ourself.Peer.Name @@ -136,14 +136,14 @@ func TestGossipTopology(t *testing.T) { checkTopology(t, r3, r1.tp(r2, r3), r2.tp(r1, r3), r3.tp(r1, r2)) // Drop the connection from 2 to 3 - r2.DeleteTestChannelConnection(r3) + r2.DeleteTestGossipConnection(r3) sendPendingGossip(r1, r2, r3) checkTopology(t, r1, r1.tp(r2, r3), r2.tp(r1), r3.tp(r1, r2)) checkTopology(t, r2, r1.tp(r2, r3), r2.tp(r1), r3.tp(r1, r2)) checkTopology(t, r3, r1.tp(r2, r3), r2.tp(r1), r3.tp(r1, r2)) // Drop the connection from 1 to 3 - r1.DeleteTestChannelConnection(r3) + r1.DeleteTestGossipConnection(r3) sendPendingGossip(r1, r2, r3) checkTopology(t, r1, r1.tp(r2), r2.tp(r1), r3.tp(r1, r2)) checkTopology(t, r2, r1.tp(r2), r2.tp(r1), r3.tp(r1, r2)) From ab902b45aa6176f95fe747a6c3fa006946f00826 Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Thu, 10 Dec 2015 10:56:18 +0000 Subject: [PATCH 3/3] refactor: introduce a helper to reduce duplication --- mesh/gossip_test.go | 30 +++++++++++++----------------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/mesh/gossip_test.go b/mesh/gossip_test.go index f1782f11..32ab6472 100644 --- a/mesh/gossip_test.go +++ b/mesh/gossip_test.go @@ -107,12 +107,19 @@ func checkTopology(t *testing.T, router *Router, wantedPeers ...*Peer) { router.Peers.RUnlock() } +func flushAndCheckTopology(t *testing.T, routers []*Router, wantedPeers ...*Peer) { + sendPendingGossip(routers...) + for _, r := range routers { + checkTopology(t, r, wantedPeers...) + } +} + func TestGossipTopology(t *testing.T) { // Create some peers that will talk to each other r1 := NewTestRouter("01:00:00:01:00:00") r2 := NewTestRouter("02:00:00:02:00:00") r3 := NewTestRouter("03:00:00:03:00:00") - + routers := []*Router{r1, r2, r3} // Check state when they have no connections checkTopology(t, r1, r1.tp()) checkTopology(t, r2, r2.tp()) @@ -124,23 +131,14 @@ func TestGossipTopology(t *testing.T) { checkTopology(t, r2, r1.tp(r2), r2.tp(r1)) AddTestGossipConnection(r2, r3) - sendPendingGossip(r1, r2, r3) - checkTopology(t, r1, r1.tp(r2), r2.tp(r1, r3), r3.tp(r2)) - checkTopology(t, r2, r1.tp(r2), r2.tp(r1, r3), r3.tp(r2)) - checkTopology(t, r3, r1.tp(r2), r2.tp(r1, r3), r3.tp(r2)) + flushAndCheckTopology(t, routers, r1.tp(r2), r2.tp(r1, r3), r3.tp(r2)) AddTestGossipConnection(r3, r1) - sendPendingGossip(r1, r2, r3) - checkTopology(t, r1, r1.tp(r2, r3), r2.tp(r1, r3), r3.tp(r1, r2)) - checkTopology(t, r2, r1.tp(r2, r3), r2.tp(r1, r3), r3.tp(r1, r2)) - checkTopology(t, r3, r1.tp(r2, r3), r2.tp(r1, r3), r3.tp(r1, r2)) + flushAndCheckTopology(t, routers, r1.tp(r2, r3), r2.tp(r1, r3), r3.tp(r1, r2)) // Drop the connection from 2 to 3 r2.DeleteTestGossipConnection(r3) - sendPendingGossip(r1, r2, r3) - checkTopology(t, r1, r1.tp(r2, r3), r2.tp(r1), r3.tp(r1, r2)) - checkTopology(t, r2, r1.tp(r2, r3), r2.tp(r1), r3.tp(r1, r2)) - checkTopology(t, r3, r1.tp(r2, r3), r2.tp(r1), r3.tp(r1, r2)) + flushAndCheckTopology(t, routers, r1.tp(r2, r3), r2.tp(r1), r3.tp(r1, r2)) // Drop the connection from 1 to 3 r1.DeleteTestGossipConnection(r3) @@ -161,12 +159,10 @@ func TestGossipSurrogate(t *testing.T) { r1 := NewTestRouter("01:00:00:01:00:00") r2 := NewTestRouter("02:00:00:02:00:00") r3 := NewTestRouter("03:00:00:03:00:00") + routers := []*Router{r1, r2, r3} AddTestGossipConnection(r1, r2) AddTestGossipConnection(r3, r2) - sendPendingGossip(r1, r2, r3) - checkTopology(t, r1, r1.tp(r2), r2.tp(r1, r3), r3.tp(r2)) - checkTopology(t, r2, r1.tp(r2), r2.tp(r1, r3), r3.tp(r2)) - checkTopology(t, r3, r1.tp(r2), r2.tp(r1, r3), r3.tp(r2)) + flushAndCheckTopology(t, routers, r1.tp(r2), r2.tp(r1, r3), r3.tp(r2)) // create a gossiper at either end, but not the middle g1 := newTestGossiper()