From b9508ffecba0ded9b3a401f1acaba67e273ff053 Mon Sep 17 00:00:00 2001 From: Sean Braithwaite Date: Mon, 27 May 2019 21:44:56 +0200 Subject: [PATCH] [p2p] Peer behaviour test tweaks (#3662) * Peer behaviour test tweaks: Address the remaining test issues mentioned in ##3552 notably: + switch to p2p_test package + Use `Error` instead of `Errorf` when not using formatting + Add expected/got errors to `TestMockPeerBehaviourReporterConcurrency` test * Peer behaviour equal behaviours test + slices of PeerBehaviours should be compared as histograms to ensure they have the same set of PeerBehaviours at the same freequncy. * TestEqualPeerBehaviours: + Add tests for the equivalence between sets of PeerBehaviours --- p2p/peer_behaviour_test.go | 129 ++++++++++++++++++++++++++++--------- 1 file changed, 97 insertions(+), 32 deletions(-) diff --git a/p2p/peer_behaviour_test.go b/p2p/peer_behaviour_test.go index 03ea4969f..1822844d6 100644 --- a/p2p/peer_behaviour_test.go +++ b/p2p/peer_behaviour_test.go @@ -1,58 +1,121 @@ -package p2p +package p2p_test import ( - "net" "sync" "testing" + + "github.com/tendermint/tendermint/p2p" ) // TestMockPeerBehaviour tests the MockPeerBehaviour' ability to store reported // peer behaviour in memory indexed by the peerID func TestMockPeerBehaviourReporter(t *testing.T) { - peer := newMockPeer(net.IP{127, 0, 0, 1}) - pr := NewMockPeerBehaviourReporter() + var peerID p2p.ID = "MockPeer" + pr := p2p.NewMockPeerBehaviourReporter() - behaviours := pr.GetBehaviours(peer.ID()) + behaviours := pr.GetBehaviours(peerID) if len(behaviours) != 0 { - t.Errorf("Expected to have no behaviours reported") + t.Error("Expected to have no behaviours reported") } - pr.Report(peer.ID(), PeerBehaviourBadMessage) - behaviours = pr.GetBehaviours(peer.ID()) + pr.Report(peerID, p2p.PeerBehaviourBadMessage) + behaviours = pr.GetBehaviours(peerID) if len(behaviours) != 1 { - t.Errorf("Expected the peer have one reported behaviour") + t.Error("Expected the peer have one reported behaviour") } - if behaviours[0] != PeerBehaviourBadMessage { - t.Errorf("Expected PeerBehaviourBadMessage to have been reported") + if behaviours[0] != p2p.PeerBehaviourBadMessage { + t.Error("Expected PeerBehaviourBadMessage to have been reported") } } type scriptedBehaviours struct { - PeerID ID - Behaviours []PeerBehaviour + PeerID p2p.ID + Behaviours []p2p.PeerBehaviour } type scriptItem struct { - PeerID ID - Behaviour PeerBehaviour + PeerID p2p.ID + Behaviour p2p.PeerBehaviour } -func equalBehaviours(a []PeerBehaviour, b []PeerBehaviour) bool { - if len(a) != len(b) { +// equalBehaviours returns true if a and b contain the same PeerBehaviours with +// the same freequency and otherwise false. +func equalBehaviours(a []p2p.PeerBehaviour, b []p2p.PeerBehaviour) bool { + aHistogram := map[p2p.PeerBehaviour]int{} + bHistogram := map[p2p.PeerBehaviour]int{} + + for _, behaviour := range a { + aHistogram[behaviour] += 1 + } + + for _, behaviour := range b { + bHistogram[behaviour] += 1 + } + + if len(aHistogram) != len(bHistogram) { return false } - same := make([]PeerBehaviour, len(a)) - for i, aBehaviour := range a { - for _, bBehaviour := range b { - if aBehaviour == bBehaviour { - same[i] = aBehaviour - } + for _, behaviour := range a { + if aHistogram[behaviour] != bHistogram[behaviour] { + return false } } - return len(same) == len(a) + for _, behaviour := range b { + if bHistogram[behaviour] != aHistogram[behaviour] { + return false + } + } + + return true +} + +// TestEqualPeerBehaviours tests that equalBehaviours can tell that two slices +// of peer behaviours can be compared for the behaviours they contain and the +// freequencies that those behaviours occur. +func TestEqualPeerBehaviours(t *testing.T) { + equals := []struct { + left []p2p.PeerBehaviour + right []p2p.PeerBehaviour + }{ + // Empty sets + {[]p2p.PeerBehaviour{}, []p2p.PeerBehaviour{}}, + // Single behaviours + {[]p2p.PeerBehaviour{p2p.PeerBehaviourVote}, []p2p.PeerBehaviour{p2p.PeerBehaviourVote}}, + // Equal Frequencies + {[]p2p.PeerBehaviour{p2p.PeerBehaviourVote, p2p.PeerBehaviourVote}, + []p2p.PeerBehaviour{p2p.PeerBehaviourVote, p2p.PeerBehaviourVote}}, + // Equal frequencies different orders + {[]p2p.PeerBehaviour{p2p.PeerBehaviourVote, p2p.PeerBehaviourBlockPart}, + []p2p.PeerBehaviour{p2p.PeerBehaviourBlockPart, p2p.PeerBehaviourVote}}, + } + + for _, test := range equals { + if !equalBehaviours(test.left, test.right) { + t.Errorf("Expected %#v and %#v to be equal", test.left, test.right) + } + } + + unequals := []struct { + left []p2p.PeerBehaviour + right []p2p.PeerBehaviour + }{ + // Comparing empty sets to non empty sets + {[]p2p.PeerBehaviour{}, []p2p.PeerBehaviour{p2p.PeerBehaviourVote}}, + // Different behaviours + {[]p2p.PeerBehaviour{p2p.PeerBehaviourVote}, []p2p.PeerBehaviour{p2p.PeerBehaviourBlockPart}}, + // Same behaviour with different frequencies + {[]p2p.PeerBehaviour{p2p.PeerBehaviourVote}, + []p2p.PeerBehaviour{p2p.PeerBehaviourVote, p2p.PeerBehaviourVote}}, + } + + for _, test := range unequals { + if equalBehaviours(test.left, test.right) { + t.Errorf("Expected %#v and %#v to be unequal", test.left, test.right) + } + } } // TestPeerBehaviourConcurrency constructs a scenario in which @@ -61,15 +124,15 @@ func equalBehaviours(a []PeerBehaviour, b []PeerBehaviour) bool { // be used within a Reactor Receive method tests to ensure thread safety. func TestMockPeerBehaviourReporterConcurrency(t *testing.T) { behaviourScript := []scriptedBehaviours{ - {"1", []PeerBehaviour{PeerBehaviourVote}}, - {"2", []PeerBehaviour{PeerBehaviourVote, PeerBehaviourVote, PeerBehaviourVote, PeerBehaviourVote}}, - {"3", []PeerBehaviour{PeerBehaviourBlockPart, PeerBehaviourVote, PeerBehaviourBlockPart, PeerBehaviourVote}}, - {"4", []PeerBehaviour{PeerBehaviourVote, PeerBehaviourVote, PeerBehaviourVote, PeerBehaviourVote}}, - {"5", []PeerBehaviour{PeerBehaviourBlockPart, PeerBehaviourVote, PeerBehaviourBlockPart, PeerBehaviourVote}}, + {"1", []p2p.PeerBehaviour{p2p.PeerBehaviourVote}}, + {"2", []p2p.PeerBehaviour{p2p.PeerBehaviourVote, p2p.PeerBehaviourVote, p2p.PeerBehaviourVote, p2p.PeerBehaviourVote}}, + {"3", []p2p.PeerBehaviour{p2p.PeerBehaviourBlockPart, p2p.PeerBehaviourVote, p2p.PeerBehaviourBlockPart, p2p.PeerBehaviourVote}}, + {"4", []p2p.PeerBehaviour{p2p.PeerBehaviourVote, p2p.PeerBehaviourVote, p2p.PeerBehaviourVote, p2p.PeerBehaviourVote}}, + {"5", []p2p.PeerBehaviour{p2p.PeerBehaviourBlockPart, p2p.PeerBehaviourVote, p2p.PeerBehaviourBlockPart, p2p.PeerBehaviourVote}}, } var receiveWg sync.WaitGroup - pr := NewMockPeerBehaviourReporter() + pr := p2p.NewMockPeerBehaviourReporter() scriptItems := make(chan scriptItem) done := make(chan int) numConsumers := 3 @@ -108,8 +171,10 @@ func TestMockPeerBehaviourReporterConcurrency(t *testing.T) { receiveWg.Wait() for _, items := range behaviourScript { - if !equalBehaviours(pr.GetBehaviours(items.PeerID), items.Behaviours) { - t.Errorf("Expected peer %s to have behaved \n", items.PeerID) + reported := pr.GetBehaviours(items.PeerID) + if !equalBehaviours(reported, items.Behaviours) { + t.Errorf("Expected peer %s to have behaved \nExpected: %#v \nGot %#v \n", + items.PeerID, items.Behaviours, reported) } } }