From 55b81cc1a1c1c5f382ac6dc6eb6bbc7064b10168 Mon Sep 17 00:00:00 2001 From: Ethan Buchman Date: Mon, 27 Nov 2017 21:48:15 +0000 Subject: [PATCH] address linting FIXMEs --- consensus/mempool_test.go | 10 ++++------ consensus/reactor_test.go | 14 -------------- consensus/replay.go | 4 +--- p2p/switch.go | 3 ++- p2p/switch_test.go | 23 ++++++++++------------- rpc/client/mock/abci_test.go | 30 ++++++++++++------------------ 6 files changed, 29 insertions(+), 55 deletions(-) diff --git a/consensus/mempool_test.go b/consensus/mempool_test.go index 820e3808b..73f676342 100644 --- a/consensus/mempool_test.go +++ b/consensus/mempool_test.go @@ -5,6 +5,8 @@ import ( "testing" "time" + "github.com/stretchr/testify/assert" + abci "github.com/tendermint/abci/types" "github.com/tendermint/tendermint/types" cmn "github.com/tendermint/tmlibs/common" @@ -120,14 +122,10 @@ func TestRmBadTx(t *testing.T) { binary.BigEndian.PutUint64(txBytes, uint64(0)) resDeliver := app.DeliverTx(txBytes) - if resDeliver.Error != nil { - // t.Error(resDeliver.Error()) // FIXME: fails - } + assert.False(t, resDeliver.IsErr(), cmn.Fmt("expected no error. got %v", resDeliver)) resCommit := app.Commit() - if resCommit.Error != nil { - // t.Error(resCommit.Error()) // FIXME: fails - } + assert.False(t, resCommit.IsErr(), cmn.Fmt("expected no error. got %v", resCommit)) emptyMempoolCh := make(chan struct{}) checkTxRespCh := make(chan struct{}) diff --git a/consensus/reactor_test.go b/consensus/reactor_test.go index a2093beb5..458ff2c44 100644 --- a/consensus/reactor_test.go +++ b/consensus/reactor_test.go @@ -389,17 +389,3 @@ func timeoutWaitGroup(t *testing.T, n int, f func(*sync.WaitGroup, int), css []* panic("Timed out waiting for all validators to commit a block") } } - -// XXX: WARNING: this function can halt the consensus as firing events is synchronous. -// Make sure to read off the channels, and in the case of subscribeToEventRespond, to write back on it - -// NOTE: this blocks on receiving a response after the event is consumed -func subscribeToEventRespond(evsw types.EventSwitch, receiver, eventID string) chan interface{} { - // listen for event - ch := make(chan interface{}) - types.AddListenerForEvent(evsw, receiver, eventID, func(data types.TMEventData) { - ch <- data - <-ch - }) - return ch -} diff --git a/consensus/replay.go b/consensus/replay.go index b38dd3f42..349eade04 100644 --- a/consensus/replay.go +++ b/consensus/replay.go @@ -117,13 +117,11 @@ func (cs *ConsensusState) catchupReplay(csHeight int) error { cs.Logger.Error("Replay: wal.group.Search returned EOF", "#ENDHEIGHT", csHeight-1) } else if err != nil { return err - } else { - defer gr.Close() // nolint: errcheck } if !found { return errors.New(cmn.Fmt("Cannot replay height %d. WAL does not contain #ENDHEIGHT for %d.", csHeight, csHeight-1)) } - defer gr.Close() + defer gr.Close() // nolint: errcheck cs.Logger.Info("Catchup by replaying consensus messages", "height", csHeight) diff --git a/p2p/switch.go b/p2p/switch.go index 4e22521ae..bea2ca1bf 100644 --- a/p2p/switch.go +++ b/p2p/switch.go @@ -291,7 +291,8 @@ func (sw *Switch) SetPubKeyFilter(f func(crypto.PubKeyEd25519) error) { func (sw *Switch) startInitPeer(peer *peer) { _, err := peer.Start() // spawn send/recv routines if err != nil { - sw.Logger.Error("Error starting peer", "err", err) + // Should never happen + sw.Logger.Error("Error starting peer", "peer", peer, "err", err) } for _, reactor := range sw.reactors { diff --git a/p2p/switch_test.go b/p2p/switch_test.go index fb179efe1..58ef3e5f0 100644 --- a/p2p/switch_test.go +++ b/p2p/switch_test.go @@ -10,11 +10,12 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + crypto "github.com/tendermint/go-crypto" wire "github.com/tendermint/go-wire" + "github.com/tendermint/tmlibs/log" cfg "github.com/tendermint/tendermint/config" - "github.com/tendermint/tmlibs/log" ) var ( @@ -171,14 +172,12 @@ func TestConnAddrFilter(t *testing.T) { // connect to good peer go func() { - if err := s1.addPeerWithConnection(c1); err != nil { - // t.Error(err) FIXME: fails - } + err := s1.addPeerWithConnection(c1) + assert.NotNil(t, err, "expected err") }() go func() { - if err := s2.addPeerWithConnection(c2); err != nil { - // t.Error(err) FIXME: fails - } + err := s2.addPeerWithConnection(c2) + assert.NotNil(t, err, "expected err") }() assertNoPeersAfterTimeout(t, s1, 400*time.Millisecond) @@ -210,14 +209,12 @@ func TestConnPubKeyFilter(t *testing.T) { // connect to good peer go func() { - if err := s1.addPeerWithConnection(c1); err != nil { - // t.Error(err) FIXME: fails - } + err := s1.addPeerWithConnection(c1) + assert.NotNil(t, err, "expected error") }() go func() { - if err := s2.addPeerWithConnection(c2); err != nil { - // t.Error(err) FIXME: fails - } + err := s2.addPeerWithConnection(c2) + assert.NotNil(t, err, "expected error") }() assertNoPeersAfterTimeout(t, s1, 400*time.Millisecond) diff --git a/rpc/client/mock/abci_test.go b/rpc/client/mock/abci_test.go index d39ec5060..a839b0dd9 100644 --- a/rpc/client/mock/abci_test.go +++ b/rpc/client/mock/abci_test.go @@ -79,6 +79,8 @@ func TestABCIMock(t *testing.T) { func TestABCIRecorder(t *testing.T) { assert, require := assert.New(t), require.New(t) + + // This mock returns errors on everything but Query m := mock.ABCIMock{ Info: mock.Call{Response: abci.ResponseInfo{ Data: "data", @@ -92,15 +94,13 @@ func TestABCIRecorder(t *testing.T) { require.Equal(0, len(r.Calls)) - r.ABCIInfo() _, err := r.ABCIInfo() - if err != nil { - t.Error(err) - } - _, err = r.ABCIQueryWithOptions("path", data.Bytes("data"), client.ABCIQueryOptions{Trusted: false}) - if err != nil { - // t.Errorf(err) FIXME: fails - } + assert.Nil(err, "expected no err on info") + _, err = r.ABCIInfo() + assert.Nil(err, "expected no err on info") + + _, err := r.ABCIQueryWithOptions("path", data.Bytes("data"), client.ABCIQueryOptions{Trusted: false}) + assert.NotNil(err, "expected error on query") require.Equal(2, len(r.Calls)) info := r.Calls[0] @@ -125,20 +125,14 @@ func TestABCIRecorder(t *testing.T) { assert.EqualValues("data", qa.Data) assert.False(qa.Trusted) - // now add some broadcasts + // now add some broadcasts (should all err) txs := []types.Tx{{1}, {2}, {3}} _, err = r.BroadcastTxCommit(txs[0]) - if err != nil { - // t.Error(err) FIXME: fails - } + assert.NotNil(err, "expected err on broadcast") _, err = r.BroadcastTxSync(txs[1]) - if err != nil { - // t.Error(err) FIXME: fails - } + assert.NotNil(err, "expected err on broadcast") _, err = r.BroadcastTxAsync(txs[2]) - if err != nil { - // t.Error(err) FIXME: fails - } + assert.NotNil(err, "expected err on broadcast") require.Equal(5, len(r.Calls))