diff --git a/.github/workflows/coverage.yml b/.github/workflows/coverage.yml index 2751b7c15..344ac8a18 100644 --- a/.github/workflows/coverage.yml +++ b/.github/workflows/coverage.yml @@ -44,7 +44,7 @@ jobs: steps: - uses: actions/setup-go@v2 with: - go-version: "^1.15.4" + go-version: "1.15" - uses: actions/checkout@v2 - uses: technote-space/get-diff-action@v4 with: @@ -66,7 +66,7 @@ jobs: steps: - uses: actions/setup-go@v2 with: - go-version: "^1.15.4" + go-version: "1.15" - uses: actions/checkout@v2 - uses: technote-space/get-diff-action@v4 with: @@ -78,6 +78,10 @@ jobs: with: name: "${{ github.sha }}-${{ matrix.part }}" if: env.GIT_DIFF + - name: Set up Go + uses: actions/setup-go@v2 + with: + go-version: 1.15 - name: test & coverage report creation run: | cat pkgs.txt.part.${{ matrix.part }} | xargs go test -mod=readonly -timeout 8m -race -coverprofile=${{ matrix.part }}profile.out -covermode=atomic diff --git a/.github/workflows/docker.yml b/.github/workflows/docker.yml index 0a1dab464..a5bfbc861 100644 --- a/.github/workflows/docker.yml +++ b/.github/workflows/docker.yml @@ -16,7 +16,7 @@ jobs: steps: - uses: actions/setup-go@v2 with: - go-version: "^1.15.4" + go-version: "1.15" - uses: actions/checkout@master - name: Prepare id: prep diff --git a/.github/workflows/e2e-nightly.yml b/.github/workflows/e2e-nightly.yml index b21a482c0..c56e41e68 100644 --- a/.github/workflows/e2e-nightly.yml +++ b/.github/workflows/e2e-nightly.yml @@ -18,7 +18,7 @@ jobs: steps: - uses: actions/setup-go@v2 with: - go-version: '^1.15.4' + go-version: '1.15' - uses: actions/checkout@v2 diff --git a/.github/workflows/e2e.yml b/.github/workflows/e2e.yml index 6ac2077ac..ed26718ca 100644 --- a/.github/workflows/e2e.yml +++ b/.github/workflows/e2e.yml @@ -15,7 +15,7 @@ jobs: steps: - uses: actions/setup-go@v2 with: - go-version: '^1.15.4' + go-version: '1.15' - uses: actions/checkout@v2 - uses: technote-space/get-diff-action@v4 with: diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 6411a2d3f..46b55eee2 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -16,7 +16,7 @@ jobs: - uses: actions/setup-go@v2 with: - go-version: '^1.15.4' + go-version: '1.15' - run: echo https://github.com/tendermint/tendermint/blob/${GITHUB_REF#refs/tags/}/CHANGELOG.md#${GITHUB_REF#refs/tags/} > ../release_notes.md diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 56f5ca437..fba5a9913 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -25,7 +25,7 @@ jobs: steps: - uses: actions/setup-go@v2 with: - go-version: "^1.15.4" + go-version: "1.15" - uses: actions/checkout@v2 - uses: technote-space/get-diff-action@v4 with: @@ -57,7 +57,7 @@ jobs: steps: - uses: actions/setup-go@v2 with: - go-version: "^1.15.4" + go-version: "1.15" - uses: actions/checkout@v2 - uses: technote-space/get-diff-action@v4 with: @@ -89,7 +89,7 @@ jobs: steps: - uses: actions/setup-go@v2 with: - go-version: "^1.15.4" + go-version: "1.15" - uses: actions/checkout@v2 - uses: technote-space/get-diff-action@v4 with: @@ -120,7 +120,7 @@ jobs: steps: - uses: actions/setup-go@v2 with: - go-version: "^1.15.4" + go-version: "1.15" - uses: actions/checkout@v2 - uses: technote-space/get-diff-action@v4 with: diff --git a/consensus/invalid_test.go b/consensus/invalid_test.go index 907693c57..5b161dd71 100644 --- a/consensus/invalid_test.go +++ b/consensus/invalid_test.go @@ -19,7 +19,7 @@ import ( func TestReactorInvalidPrecommit(t *testing.T) { N := 4 css, cleanup := randConsensusNet(N, "consensus_reactor_test", newMockTickerFunc(true), newCounter) - defer cleanup() + t.Cleanup(cleanup) for i := 0; i < 4; i++ { ticker := NewTimeoutTicker() @@ -43,7 +43,7 @@ func TestReactorInvalidPrecommit(t *testing.T) { invalidDoPrevoteFunc(t, height, round, byzVal, byzR.Switch, pv) } byzVal.mtx.Unlock() - defer stopConsensusNet(log.TestingLogger(), reactors, eventBuses) + t.Cleanup(func() { stopConsensusNet(log.TestingLogger(), reactors, eventBuses) }) // wait for a bunch of blocks // TODO: make this tighter by ensuring the halt happens by block 2 diff --git a/consensus/mempool_test.go b/consensus/mempool_test.go index db9662acb..c53185048 100644 --- a/consensus/mempool_test.go +++ b/consensus/mempool_test.go @@ -26,7 +26,8 @@ func assertMempool(txn txNotifier) mempl.Mempool { func TestMempoolNoProgressUntilTxsAvailable(t *testing.T) { config := ResetConfig("consensus_mempool_txs_available_test") - defer os.RemoveAll(config.RootDir) + t.Cleanup(func() { _ = os.RemoveAll(config.RootDir) }) + config.Consensus.CreateEmptyBlocks = false state, privVals := randGenesisState(1, false, 10) cs := newStateWithConfig(config, state, privVals[0], NewCounterApplication()) @@ -45,7 +46,7 @@ func TestMempoolNoProgressUntilTxsAvailable(t *testing.T) { func TestMempoolProgressAfterCreateEmptyBlocksInterval(t *testing.T) { config := ResetConfig("consensus_mempool_txs_available_test") - defer os.RemoveAll(config.RootDir) + t.Cleanup(func() { _ = os.RemoveAll(config.RootDir) }) config.Consensus.CreateEmptyBlocksInterval = ensureTimeout state, privVals := randGenesisState(1, false, 10) @@ -63,7 +64,8 @@ func TestMempoolProgressAfterCreateEmptyBlocksInterval(t *testing.T) { func TestMempoolProgressInHigherRound(t *testing.T) { config := ResetConfig("consensus_mempool_txs_available_test") - defer os.RemoveAll(config.RootDir) + t.Cleanup(func() { _ = os.RemoveAll(config.RootDir) }) + config.Consensus.CreateEmptyBlocks = false state, privVals := randGenesisState(1, false, 10) cs := newStateWithConfig(config, state, privVals[0], NewCounterApplication()) diff --git a/consensus/replay_test.go b/consensus/replay_test.go index 2970f15ed..b40d408c2 100644 --- a/consensus/replay_test.go +++ b/consensus/replay_test.go @@ -83,11 +83,11 @@ func startNewStateAndWaitForBlock(t *testing.T, consensusReplayConfig *cfg.Confi err := cs.Start() require.NoError(t, err) - defer func() { + t.Cleanup(func() { if err := cs.Stop(); err != nil { t.Error(err) } - }() + }) // This is just a signal that we haven't halted; its not something contained // in the WAL itself. Assuming the consensus state is running, replay of any @@ -136,10 +136,10 @@ func TestWALCrash(t *testing.T) { 3}, } - for i, tc := range testCases { + for _, tc := range testCases { tc := tc - consensusReplayConfig := ResetConfig(fmt.Sprintf("%s_%d", t.Name(), i)) t.Run(tc.name, func(t *testing.T) { + consensusReplayConfig := ResetConfig(tc.name) crashWALandCheckLiveness(t, consensusReplayConfig, tc.initFn, tc.heightToStop) }) } @@ -658,7 +658,7 @@ func testHandshakeReplay(t *testing.T, config *cfg.Config, nBlocks int, mode uin var genesisState sm.State if testValidatorsChange { testConfig := ResetConfig(fmt.Sprintf("%s_%v_m", t.Name(), mode)) - defer os.RemoveAll(testConfig.RootDir) + t.Cleanup(func() { _ = os.RemoveAll(testConfig.RootDir) }) stateDB = dbm.NewMemDB() genesisState = sim.GenesisState @@ -668,7 +668,7 @@ func testHandshakeReplay(t *testing.T, config *cfg.Config, nBlocks int, mode uin store = newMockBlockStore(config, genesisState.ConsensusParams) } else { // test single node testConfig := ResetConfig(fmt.Sprintf("%s_%v_s", t.Name(), mode)) - defer os.RemoveAll(testConfig.RootDir) + t.Cleanup(func() { _ = os.RemoveAll(testConfig.RootDir) }) walBody, err := WALWithNBlocks(t, numBlocks) require.NoError(t, err) walFile := tempWALWithData(walBody) @@ -885,7 +885,7 @@ func TestHandshakePanicsIfAppReturnsWrongAppHash(t *testing.T) { // - 0x02 // - 0x03 config := ResetConfig("handshake_test_") - defer os.RemoveAll(config.RootDir) + t.Cleanup(func() { os.RemoveAll(config.RootDir) }) privVal := privval.LoadFilePV(config.PrivValidatorKeyFile(), config.PrivValidatorStateFile()) const appVersion = 0x0 pubKey, err := privVal.GetPubKey() @@ -1220,7 +1220,8 @@ func TestHandshakeUpdatesValidators(t *testing.T) { clientCreator := proxy.NewLocalClientCreator(app) config := ResetConfig("handshake_test_") - defer os.RemoveAll(config.RootDir) + t.Cleanup(func() { _ = os.RemoveAll(config.RootDir) }) + privVal := privval.LoadFilePV(config.PrivValidatorKeyFile(), config.PrivValidatorStateFile()) pubKey, err := privVal.GetPubKey() require.NoError(t, err) diff --git a/consensus/wal_test.go b/consensus/wal_test.go index 4ee813609..a64ecc9a5 100644 --- a/consensus/wal_test.go +++ b/consensus/wal_test.go @@ -3,8 +3,6 @@ package consensus import ( "bytes" "crypto/rand" - "io/ioutil" - "os" "path/filepath" // "sync" @@ -27,10 +25,7 @@ const ( ) func TestWALTruncate(t *testing.T) { - walDir, err := ioutil.TempDir("", "wal") - require.NoError(t, err) - defer os.RemoveAll(walDir) - + walDir := t.TempDir() walFile := filepath.Join(walDir, "wal") // this magic number 4K can truncate the content when RotateFile. @@ -45,14 +40,14 @@ func TestWALTruncate(t *testing.T) { wal.SetLogger(log.TestingLogger()) err = wal.Start() require.NoError(t, err) - defer func() { + t.Cleanup(func() { if err := wal.Stop(); err != nil { t.Error(err) } // wait for the wal to finish shutting down so we // can safely remove the directory wal.Wait() - }() + }) // 60 block's size nearly 70K, greater than group's headBuf size(4096 * 10), // when headBuf is full, truncate content will Flush to the file. at this @@ -71,7 +66,7 @@ func TestWALTruncate(t *testing.T) { assert.NoError(t, err, "expected not to err on height %d", h) assert.True(t, found, "expected to find end height for %d", h) assert.NotNil(t, gr) - defer gr.Close() + t.Cleanup(func() { _ = gr.Close() }) dec := NewWALDecoder(gr) msg, err := dec.Decode() @@ -109,23 +104,21 @@ func TestWALEncoderDecoder(t *testing.T) { } func TestWALWrite(t *testing.T) { - walDir, err := ioutil.TempDir("", "wal") - require.NoError(t, err) - defer os.RemoveAll(walDir) + walDir := t.TempDir() walFile := filepath.Join(walDir, "wal") wal, err := NewWAL(walFile) require.NoError(t, err) err = wal.Start() require.NoError(t, err) - defer func() { + t.Cleanup(func() { if err := wal.Stop(); err != nil { t.Error(err) } // wait for the wal to finish shutting down so we // can safely remove the directory wal.Wait() - }() + }) // 1) Write returns an error if msg is too big msg := &BlockPartMessage{ @@ -166,7 +159,7 @@ func TestWALSearchForEndHeight(t *testing.T) { assert.NoError(t, err, "expected not to err on height %d", h) assert.True(t, found, "expected to find end height for %d", h) assert.NotNil(t, gr) - defer gr.Close() + t.Cleanup(func() { _ = gr.Close() }) dec := NewWALDecoder(gr) msg, err := dec.Decode() @@ -177,12 +170,10 @@ func TestWALSearchForEndHeight(t *testing.T) { } func TestWALPeriodicSync(t *testing.T) { - walDir, err := ioutil.TempDir("", "wal") - require.NoError(t, err) - defer os.RemoveAll(walDir) - + walDir := t.TempDir() walFile := filepath.Join(walDir, "wal") wal, err := NewWAL(walFile, autofile.GroupCheckDuration(1*time.Millisecond)) + require.NoError(t, err) wal.SetFlushInterval(walTestFlushInterval) @@ -196,12 +187,12 @@ func TestWALPeriodicSync(t *testing.T) { assert.NotZero(t, wal.Group().Buffered()) require.NoError(t, wal.Start()) - defer func() { + t.Cleanup(func() { if err := wal.Stop(); err != nil { t.Error(err) } wal.Wait() - }() + }) time.Sleep(walTestFlushInterval + (10 * time.Millisecond)) @@ -239,7 +230,6 @@ func nBytes(n int) []byte { func benchmarkWalDecode(b *testing.B, n int) { // registerInterfacesOnce() - buf := new(bytes.Buffer) enc := NewWALEncoder(buf) diff --git a/p2p/conn/connection_test.go b/p2p/conn/connection_test.go index a189e8b89..12f68f437 100644 --- a/p2p/conn/connection_test.go +++ b/p2p/conn/connection_test.go @@ -45,13 +45,12 @@ func createMConnectionWithCallbacks( func TestMConnectionSendFlushStop(t *testing.T) { server, client := NetPipe() - defer server.Close() - defer client.Close() + t.Cleanup(closeAll(t, client, server)) clientConn := createTestMConnection(client) err := clientConn.Start() require.Nil(t, err) - defer clientConn.Stop() // nolint:errcheck // ignore for tests + t.Cleanup(stopAll(t, clientConn)) msg := []byte("abc") assert.True(t, clientConn.Send(0x01, msg)) @@ -83,13 +82,12 @@ func TestMConnectionSendFlushStop(t *testing.T) { func TestMConnectionSend(t *testing.T) { server, client := NetPipe() - defer server.Close() - defer client.Close() + t.Cleanup(closeAll(t, client, server)) mconn := createTestMConnection(client) err := mconn.Start() require.Nil(t, err) - defer mconn.Stop() // nolint:errcheck // ignore for tests + t.Cleanup(stopAll(t, mconn)) msg := []byte("Ant-Man") assert.True(t, mconn.Send(0x01, msg)) @@ -114,8 +112,7 @@ func TestMConnectionSend(t *testing.T) { func TestMConnectionReceive(t *testing.T) { server, client := NetPipe() - defer server.Close() - defer client.Close() + t.Cleanup(closeAll(t, client, server)) receivedCh := make(chan []byte) errorsCh := make(chan interface{}) @@ -128,12 +125,12 @@ func TestMConnectionReceive(t *testing.T) { mconn1 := createMConnectionWithCallbacks(client, onReceive, onError) err := mconn1.Start() require.Nil(t, err) - defer mconn1.Stop() // nolint:errcheck // ignore for tests + t.Cleanup(stopAll(t, mconn1)) mconn2 := createTestMConnection(server) err = mconn2.Start() require.Nil(t, err) - defer mconn2.Stop() // nolint:errcheck // ignore for tests + t.Cleanup(stopAll(t, mconn2)) msg := []byte("Cyclops") assert.True(t, mconn2.Send(0x01, msg)) @@ -150,13 +147,12 @@ func TestMConnectionReceive(t *testing.T) { func TestMConnectionStatus(t *testing.T) { server, client := NetPipe() - defer server.Close() - defer client.Close() + t.Cleanup(closeAll(t, client, server)) mconn := createTestMConnection(client) err := mconn.Start() require.Nil(t, err) - defer mconn.Stop() // nolint:errcheck // ignore for tests + t.Cleanup(stopAll(t, mconn)) status := mconn.Status() assert.NotNil(t, status) @@ -165,8 +161,7 @@ func TestMConnectionStatus(t *testing.T) { func TestMConnectionPongTimeoutResultsInError(t *testing.T) { server, client := net.Pipe() - defer server.Close() - defer client.Close() + t.Cleanup(closeAll(t, client, server)) receivedCh := make(chan []byte) errorsCh := make(chan interface{}) @@ -179,7 +174,7 @@ func TestMConnectionPongTimeoutResultsInError(t *testing.T) { mconn := createMConnectionWithCallbacks(client, onReceive, onError) err := mconn.Start() require.Nil(t, err) - defer mconn.Stop() // nolint:errcheck // ignore for tests + t.Cleanup(stopAll(t, mconn)) serverGotPing := make(chan struct{}) go func() { @@ -204,8 +199,7 @@ func TestMConnectionPongTimeoutResultsInError(t *testing.T) { func TestMConnectionMultiplePongsInTheBeginning(t *testing.T) { server, client := net.Pipe() - defer server.Close() - defer client.Close() + t.Cleanup(closeAll(t, client, server)) receivedCh := make(chan []byte) errorsCh := make(chan interface{}) @@ -218,7 +212,7 @@ func TestMConnectionMultiplePongsInTheBeginning(t *testing.T) { mconn := createMConnectionWithCallbacks(client, onReceive, onError) err := mconn.Start() require.Nil(t, err) - defer mconn.Stop() // nolint:errcheck // ignore for tests + t.Cleanup(stopAll(t, mconn)) // sending 3 pongs in a row (abuse) protoWriter := protoio.NewDelimitedWriter(server) @@ -259,8 +253,7 @@ func TestMConnectionMultiplePongsInTheBeginning(t *testing.T) { func TestMConnectionMultiplePings(t *testing.T) { server, client := net.Pipe() - defer server.Close() - defer client.Close() + t.Cleanup(closeAll(t, client, server)) receivedCh := make(chan []byte) errorsCh := make(chan interface{}) @@ -273,7 +266,7 @@ func TestMConnectionMultiplePings(t *testing.T) { mconn := createMConnectionWithCallbacks(client, onReceive, onError) err := mconn.Start() require.Nil(t, err) - defer mconn.Stop() // nolint:errcheck // ignore for tests + t.Cleanup(stopAll(t, mconn)) // sending 3 pings in a row (abuse) // see https://github.com/tendermint/tendermint/issues/1190 @@ -304,12 +297,10 @@ func TestMConnectionMultiplePings(t *testing.T) { func TestMConnectionPingPongs(t *testing.T) { // check that we are not leaking any go-routines - defer leaktest.CheckTimeout(t, 10*time.Second)() + t.Cleanup(leaktest.CheckTimeout(t, 10*time.Second)) server, client := net.Pipe() - - defer server.Close() - defer client.Close() + t.Cleanup(closeAll(t, client, server)) receivedCh := make(chan []byte) errorsCh := make(chan interface{}) @@ -322,7 +313,7 @@ func TestMConnectionPingPongs(t *testing.T) { mconn := createMConnectionWithCallbacks(client, onReceive, onError) err := mconn.Start() require.Nil(t, err) - defer mconn.Stop() // nolint:errcheck // ignore for tests + t.Cleanup(stopAll(t, mconn)) serverGotPing := make(chan struct{}) go func() { @@ -366,8 +357,7 @@ func TestMConnectionPingPongs(t *testing.T) { func TestMConnectionStopsAndReturnsError(t *testing.T) { server, client := NetPipe() - defer server.Close() - defer client.Close() + t.Cleanup(closeAll(t, client, server)) receivedCh := make(chan []byte) errorsCh := make(chan interface{}) @@ -380,7 +370,7 @@ func TestMConnectionStopsAndReturnsError(t *testing.T) { mconn := createMConnectionWithCallbacks(client, onReceive, onError) err := mconn.Start() require.Nil(t, err) - defer mconn.Stop() // nolint:errcheck // ignore for tests + t.Cleanup(stopAll(t, mconn)) if err := client.Close(); err != nil { t.Error(err) @@ -446,18 +436,7 @@ func TestMConnectionReadErrorBadEncoding(t *testing.T) { _, err := client.Write([]byte{1, 2, 3, 4, 5}) require.NoError(t, err) assert.True(t, expectSend(chOnErr), "badly encoded msgPacket") - - t.Cleanup(func() { - if err := mconnClient.Stop(); err != nil { - t.Log(err) - } - }) - - t.Cleanup(func() { - if err := mconnServer.Stop(); err != nil { - t.Log(err) - } - }) + t.Cleanup(stopAll(t, mconnClient, mconnServer)) } func TestMConnectionReadErrorUnknownChannel(t *testing.T) { @@ -473,18 +452,7 @@ func TestMConnectionReadErrorUnknownChannel(t *testing.T) { // should cause an error assert.True(t, mconnClient.Send(0x02, msg)) assert.True(t, expectSend(chOnErr), "unknown channel") - - t.Cleanup(func() { - if err := mconnClient.Stop(); err != nil { - t.Log(err) - } - }) - - t.Cleanup(func() { - if err := mconnServer.Stop(); err != nil { - t.Log(err) - } - }) + t.Cleanup(stopAll(t, mconnClient, mconnServer)) } func TestMConnectionReadErrorLongMessage(t *testing.T) { @@ -492,8 +460,7 @@ func TestMConnectionReadErrorLongMessage(t *testing.T) { chOnRcv := make(chan struct{}) mconnClient, mconnServer := newClientAndServerConnsForReadErrors(t, chOnErr) - defer mconnClient.Stop() // nolint:errcheck // ignore for tests - defer mconnServer.Stop() // nolint:errcheck // ignore for tests + t.Cleanup(stopAll(t, mconnClient, mconnServer)) mconnServer.onReceive = func(chID byte, msgBytes []byte) { chOnRcv <- struct{}{} @@ -528,8 +495,7 @@ func TestMConnectionReadErrorLongMessage(t *testing.T) { func TestMConnectionReadErrorUnknownMsgType(t *testing.T) { chOnErr := make(chan struct{}) mconnClient, mconnServer := newClientAndServerConnsForReadErrors(t, chOnErr) - defer mconnClient.Stop() // nolint:errcheck // ignore for tests - defer mconnServer.Stop() // nolint:errcheck // ignore for tests + t.Cleanup(stopAll(t, mconnClient, mconnServer)) // send msg with unknown msg type _, err := protoio.NewDelimitedWriter(mconnClient.conn).WriteMsg(&types.Header{ChainID: "x"}) @@ -539,13 +505,12 @@ func TestMConnectionReadErrorUnknownMsgType(t *testing.T) { func TestMConnectionTrySend(t *testing.T) { server, client := NetPipe() - defer server.Close() - defer client.Close() + t.Cleanup(closeAll(t, client, server)) mconn := createTestMConnection(client) err := mconn.Start() require.Nil(t, err) - defer mconn.Stop() // nolint:errcheck // ignore for tests + t.Cleanup(stopAll(t, mconn)) msg := []byte("Semicolon-Woman") resultCh := make(chan string, 2) @@ -587,3 +552,31 @@ func TestConnVectors(t *testing.T) { require.Equal(t, tc.expBytes, hex.EncodeToString(bz), tc.testName) } } + +type stopper interface { + Stop() error +} + +func stopAll(t *testing.T, stoppers ...stopper) func() { + return func() { + for _, s := range stoppers { + if err := s.Stop(); err != nil { + t.Log(err) + } + } + } +} + +type closer interface { + Close() error +} + +func closeAll(t *testing.T, closers ...closer) func() { + return func() { + for _, s := range closers { + if err := s.Close(); err != nil { + t.Log(err) + } + } + } +} diff --git a/p2p/conn/secret_connection_test.go b/p2p/conn/secret_connection_test.go index e787e1348..2bbe31fef 100644 --- a/p2p/conn/secret_connection_test.go +++ b/p2p/conn/secret_connection_test.go @@ -234,7 +234,7 @@ func TestDeriveSecretsAndChallengeGolden(t *testing.T) { if err != nil { log.Fatal(err) } - defer f.Close() + t.Cleanup(closeAll(t, f)) scanner := bufio.NewScanner(f) for scanner.Scan() { line := scanner.Text() @@ -258,8 +258,7 @@ func TestDeriveSecretsAndChallengeGolden(t *testing.T) { func TestNilPubkey(t *testing.T) { var fooConn, barConn = makeKVStoreConnPair() - defer fooConn.Close() - defer barConn.Close() + t.Cleanup(closeAll(t, fooConn, barConn)) var fooPrvKey = ed25519.GenPrivKey() var barPrvKey = privKeyWithNilPubKey{ed25519.GenPrivKey()} @@ -272,8 +271,8 @@ func TestNilPubkey(t *testing.T) { func TestNonEd25519Pubkey(t *testing.T) { var fooConn, barConn = makeKVStoreConnPair() - defer fooConn.Close() - defer barConn.Close() + t.Cleanup(closeAll(t, fooConn, barConn)) + var fooPrvKey = ed25519.GenPrivKey() var barPrvKey = sr25519.GenPrivKey() diff --git a/p2p/pex/addrbook_test.go b/p2p/pex/addrbook_test.go index ad41d5562..21e098499 100644 --- a/p2p/pex/addrbook_test.go +++ b/p2p/pex/addrbook_test.go @@ -22,8 +22,7 @@ import ( // FIXME These tests should not rely on .(*addrBook) assertions func TestAddrBookPickAddress(t *testing.T) { - fname := createTempFileName("addrbook_test") - defer deleteTempFile(fname) + fname := createTempFileName(t, "addrbook_test") // 0 addresses book := NewAddrBook(fname, true) @@ -59,8 +58,7 @@ func TestAddrBookPickAddress(t *testing.T) { } func TestAddrBookSaveLoad(t *testing.T) { - fname := createTempFileName("addrbook_test") - defer deleteTempFile(fname) + fname := createTempFileName(t, "addrbook_test") // 0 addresses book := NewAddrBook(fname, true) @@ -94,9 +92,7 @@ func TestAddrBookSaveLoad(t *testing.T) { } func TestAddrBookLookup(t *testing.T) { - fname := createTempFileName("addrbook_test") - defer deleteTempFile(fname) - + fname := createTempFileName(t, "addrbook_test") randAddrs := randNetAddressPairs(t, 100) book := NewAddrBook(fname, true) @@ -113,9 +109,7 @@ func TestAddrBookLookup(t *testing.T) { } func TestAddrBookPromoteToOld(t *testing.T) { - fname := createTempFileName("addrbook_test") - defer deleteTempFile(fname) - + fname := createTempFileName(t, "addrbook_test") randAddrs := randNetAddressPairs(t, 100) book := NewAddrBook(fname, true) @@ -157,10 +151,9 @@ func TestAddrBookPromoteToOld(t *testing.T) { } func TestAddrBookHandlesDuplicates(t *testing.T) { - fname := createTempFileName("addrbook_test") - defer deleteTempFile(fname) - + fname := createTempFileName(t, "addrbook_test") book := NewAddrBook(fname, true) + book.SetLogger(log.TestingLogger()) randAddrs := randNetAddressPairs(t, 100) @@ -211,9 +204,7 @@ func randIPv4Address(t *testing.T) *p2p.NetAddress { } func TestAddrBookRemoveAddress(t *testing.T) { - fname := createTempFileName("addrbook_test") - defer deleteTempFile(fname) - + fname := createTempFileName(t, "addrbook_test") book := NewAddrBook(fname, true) book.SetLogger(log.TestingLogger()) @@ -232,9 +223,7 @@ func TestAddrBookRemoveAddress(t *testing.T) { func TestAddrBookGetSelectionWithOneMarkedGood(t *testing.T) { // create a book with 10 addresses, 1 good/old and 9 new - book, fname := createAddrBookWithMOldAndNNewAddrs(t, 1, 9) - defer deleteTempFile(fname) - + book, _ := createAddrBookWithMOldAndNNewAddrs(t, 1, 9) addrs := book.GetSelectionWithBias(biasToSelectNewPeers) assert.NotNil(t, addrs) assertMOldAndNNewAddrsInSelection(t, 1, 9, addrs, book) @@ -242,26 +231,20 @@ func TestAddrBookGetSelectionWithOneMarkedGood(t *testing.T) { func TestAddrBookGetSelectionWithOneNotMarkedGood(t *testing.T) { // create a book with 10 addresses, 9 good/old and 1 new - book, fname := createAddrBookWithMOldAndNNewAddrs(t, 9, 1) - defer deleteTempFile(fname) - + book, _ := createAddrBookWithMOldAndNNewAddrs(t, 9, 1) addrs := book.GetSelectionWithBias(biasToSelectNewPeers) assert.NotNil(t, addrs) assertMOldAndNNewAddrsInSelection(t, 9, 1, addrs, book) } func TestAddrBookGetSelectionReturnsNilWhenAddrBookIsEmpty(t *testing.T) { - book, fname := createAddrBookWithMOldAndNNewAddrs(t, 0, 0) - defer deleteTempFile(fname) - + book, _ := createAddrBookWithMOldAndNNewAddrs(t, 0, 0) addrs := book.GetSelectionWithBias(biasToSelectNewPeers) assert.Nil(t, addrs) } func TestAddrBookGetSelection(t *testing.T) { - fname := createTempFileName("addrbook_test") - defer deleteTempFile(fname) - + fname := createTempFileName(t, "addrbook_test") book := NewAddrBook(fname, true) book.SetLogger(log.TestingLogger()) @@ -301,9 +284,7 @@ func TestAddrBookGetSelection(t *testing.T) { func TestAddrBookGetSelectionWithBias(t *testing.T) { const biasTowardsNewAddrs = 30 - fname := createTempFileName("addrbook_test") - defer deleteTempFile(fname) - + fname := createTempFileName(t, "addrbook_test") book := NewAddrBook(fname, true) book.SetLogger(log.TestingLogger()) @@ -384,9 +365,7 @@ func TestAddrBookGetSelectionWithBias(t *testing.T) { } func TestAddrBookHasAddress(t *testing.T) { - fname := createTempFileName("addrbook_test") - defer deleteTempFile(fname) - + fname := createTempFileName(t, "addrbook_test") book := NewAddrBook(fname, true) book.SetLogger(log.TestingLogger()) addr := randIPv4Address(t) @@ -401,6 +380,7 @@ func TestAddrBookHasAddress(t *testing.T) { } func testCreatePrivateAddrs(t *testing.T, numAddrs int) ([]*p2p.NetAddress, []string) { + t.Helper() addrs := make([]*p2p.NetAddress, numAddrs) for i := 0; i < numAddrs; i++ { addrs[i] = randIPv4Address(t) @@ -414,9 +394,7 @@ func testCreatePrivateAddrs(t *testing.T, numAddrs int) ([]*p2p.NetAddress, []st } func TestBanBadPeers(t *testing.T) { - fname := createTempFileName("addrbook_test") - defer deleteTempFile(fname) - + fname := createTempFileName(t, "addrbook_test") book := NewAddrBook(fname, true) book.SetLogger(log.TestingLogger()) @@ -441,9 +419,7 @@ func TestBanBadPeers(t *testing.T) { } func TestAddrBookEmpty(t *testing.T) { - fname := createTempFileName("addrbook_test") - defer deleteTempFile(fname) - + fname := createTempFileName(t, "addrbook_test") book := NewAddrBook(fname, true) book.SetLogger(log.TestingLogger()) // Check that empty book is empty @@ -463,9 +439,7 @@ func TestAddrBookEmpty(t *testing.T) { } func TestPrivatePeers(t *testing.T) { - fname := createTempFileName("addrbook_test") - defer deleteTempFile(fname) - + fname := createTempFileName(t, "addrbook_test") book := NewAddrBook(fname, true) book.SetLogger(log.TestingLogger()) @@ -496,8 +470,7 @@ func testAddrBookAddressSelection(t *testing.T, bookSize int) { dbgStr := fmt.Sprintf("book of size %d (new %d, old %d)", bookSize, nBookNew, nBookOld) // create book and get selection - book, fname := createAddrBookWithMOldAndNNewAddrs(t, nBookOld, nBookNew) - defer deleteTempFile(fname) + book, _ := createAddrBookWithMOldAndNNewAddrs(t, nBookOld, nBookNew) addrs := book.GetSelectionWithBias(biasToSelectNewPeers) assert.NotNil(t, addrs, "%s - expected a non-nil selection", dbgStr) nAddrs := len(addrs) @@ -590,8 +563,7 @@ func TestMultipleAddrBookAddressSelection(t *testing.T) { } func TestAddrBookAddDoesNotOverwriteOldIP(t *testing.T) { - fname := createTempFileName("addrbook_test") - defer deleteTempFile(fname) + fname := createTempFileName(t, "addrbook_test") // This test creates adds a peer to the address book and marks it good // It then attempts to override the peer's IP, by adding a peer with the same ID @@ -717,28 +689,26 @@ func assertMOldAndNNewAddrsInSelection(t *testing.T, m, n int, addrs []*p2p.NetA assert.Equal(t, n, nNew, "new addresses") } -func createTempFileName(prefix string) string { +func createTempFileName(t *testing.T, prefix string) string { + t.Helper() f, err := ioutil.TempFile("", prefix) if err != nil { panic(err) } + fname := f.Name() - err = f.Close() - if err != nil { - panic(err) + if err := f.Close(); err != nil { + t.Fatal(err) } + + t.Cleanup(func() { _ = os.Remove(fname) }) + return fname } -func deleteTempFile(fname string) { - err := os.Remove(fname) - if err != nil { - panic(err) - } -} - func createAddrBookWithMOldAndNNewAddrs(t *testing.T, nOld, nNew int) (book *addrBook, fname string) { - fname = createTempFileName("addrbook_test") + t.Helper() + fname = createTempFileName(t, "addrbook_test") book = NewAddrBook(fname, true).(*addrBook) book.SetLogger(log.TestingLogger()) diff --git a/p2p/pex/pex_reactor_test.go b/p2p/pex/pex_reactor_test.go index 4ed1254ef..aee18bb11 100644 --- a/p2p/pex/pex_reactor_test.go +++ b/p2p/pex/pex_reactor_test.go @@ -3,8 +3,6 @@ package pex import ( "encoding/hex" "fmt" - "io/ioutil" - "os" "path/filepath" "testing" "time" @@ -31,16 +29,14 @@ func init() { } func TestPEXReactorBasic(t *testing.T) { - r, book := createReactor(&ReactorConfig{}) - defer teardownReactor(book) + r, _ := createReactor(t, &ReactorConfig{}) assert.NotNil(t, r) assert.NotEmpty(t, r.GetChannels()) } func TestPEXReactorAddRemovePeer(t *testing.T) { - r, book := createReactor(&ReactorConfig{}) - defer teardownReactor(book) + r, book := createReactor(t, &ReactorConfig{}) size := book.Size() peer := p2p.CreateRandomPeer(false) @@ -73,9 +69,7 @@ func TestPEXReactorRunning(t *testing.T) { switches := make([]*p2p.Switch, N) // directory to store address books - dir, err := ioutil.TempDir("", "pex_reactor") - require.Nil(t, err) - defer os.RemoveAll(dir) + dir := t.TempDir() books := make([]AddrBook, N) logger := log.TestingLogger() @@ -123,9 +117,7 @@ func TestPEXReactorRunning(t *testing.T) { } func TestPEXReactorReceive(t *testing.T) { - r, book := createReactor(&ReactorConfig{}) - defer teardownReactor(book) - + r, book := createReactor(t, &ReactorConfig{}) peer := p2p.CreateRandomPeer(false) // we have to send a request to receive responses @@ -141,9 +133,7 @@ func TestPEXReactorReceive(t *testing.T) { } func TestPEXReactorRequestMessageAbuse(t *testing.T) { - r, book := createReactor(&ReactorConfig{}) - defer teardownReactor(book) - + r, book := createReactor(t, &ReactorConfig{}) sw := createSwitchAndAddReactors(r) sw.SetAddrBook(book) @@ -176,9 +166,7 @@ func TestPEXReactorRequestMessageAbuse(t *testing.T) { } func TestPEXReactorAddrsMessageAbuse(t *testing.T) { - r, book := createReactor(&ReactorConfig{}) - defer teardownReactor(book) - + r, book := createReactor(t, &ReactorConfig{}) sw := createSwitchAndAddReactors(r) sw.SetAddrBook(book) @@ -208,9 +196,7 @@ func TestPEXReactorAddrsMessageAbuse(t *testing.T) { func TestCheckSeeds(t *testing.T) { // directory to store address books - dir, err := ioutil.TempDir("", "pex_reactor") - require.Nil(t, err) - defer os.RemoveAll(dir) + dir := t.TempDir() // 1. test creating peer with no seeds works peerSwitch := testCreateDefaultPeer(dir, 0) @@ -247,19 +233,17 @@ func TestCheckSeeds(t *testing.T) { func TestPEXReactorUsesSeedsIfNeeded(t *testing.T) { // directory to store address books - dir, err := ioutil.TempDir("", "pex_reactor") - require.Nil(t, err) - defer os.RemoveAll(dir) + dir := t.TempDir() // 1. create seed seed := testCreateSeed(dir, 0, []*p2p.NetAddress{}, []*p2p.NetAddress{}) require.Nil(t, seed.Start()) - defer seed.Stop() // nolint:errcheck // ignore for tests + t.Cleanup(func() { _ = seed.Stop() }) // 2. create usual peer with only seed configured. peer := testCreatePeerWithSeed(dir, 1, seed) require.Nil(t, peer.Start()) - defer peer.Stop() // nolint:errcheck // ignore for tests + t.Cleanup(func() { _ = seed.Stop() }) // 3. check that the peer connects to seed immediately assertPeersWithTimeout(t, []*p2p.Switch{peer}, 10*time.Millisecond, 3*time.Second, 1) @@ -267,25 +251,23 @@ func TestPEXReactorUsesSeedsIfNeeded(t *testing.T) { func TestConnectionSpeedForPeerReceivedFromSeed(t *testing.T) { // directory to store address books - dir, err := ioutil.TempDir("", "pex_reactor") - require.Nil(t, err) - defer os.RemoveAll(dir) + dir := t.TempDir() // 1. create peer peerSwitch := testCreateDefaultPeer(dir, 1) require.Nil(t, peerSwitch.Start()) - defer peerSwitch.Stop() // nolint:errcheck // ignore for tests + t.Cleanup(func() { _ = peerSwitch.Stop() }) // 2. Create seed which knows about the peer peerAddr := peerSwitch.NetAddress() seed := testCreateSeed(dir, 2, []*p2p.NetAddress{peerAddr}, []*p2p.NetAddress{peerAddr}) require.Nil(t, seed.Start()) - defer seed.Stop() // nolint:errcheck // ignore for tests + t.Cleanup(func() { _ = seed.Stop() }) // 3. create another peer with only seed configured. secondPeer := testCreatePeerWithSeed(dir, 3, seed) require.Nil(t, secondPeer.Start()) - defer secondPeer.Stop() // nolint:errcheck // ignore for tests + t.Cleanup(func() { _ = secondPeer.Stop() }) // 4. check that the second peer connects to seed immediately assertPeersWithTimeout(t, []*p2p.Switch{secondPeer}, 10*time.Millisecond, 3*time.Second, 1) @@ -296,25 +278,21 @@ func TestConnectionSpeedForPeerReceivedFromSeed(t *testing.T) { func TestPEXReactorSeedMode(t *testing.T) { // directory to store address books - dir, err := ioutil.TempDir("", "pex_reactor") - require.Nil(t, err) - defer os.RemoveAll(dir) + dir := t.TempDir() pexRConfig := &ReactorConfig{SeedMode: true, SeedDisconnectWaitPeriod: 10 * time.Millisecond} - pexR, book := createReactor(pexRConfig) - defer teardownReactor(book) - + pexR, book := createReactor(t, pexRConfig) sw := createSwitchAndAddReactors(pexR) + sw.SetAddrBook(book) - err = sw.Start() - require.NoError(t, err) - defer sw.Stop() // nolint:errcheck // ignore for tests + require.NoError(t, sw.Start()) + t.Cleanup(func() { _ = sw.Stop() }) assert.Zero(t, sw.Peers().Size()) peerSwitch := testCreateDefaultPeer(dir, 1) require.NoError(t, peerSwitch.Start()) - defer peerSwitch.Stop() // nolint:errcheck // ignore for tests + t.Cleanup(func() { _ = peerSwitch.Stop() }) // 1. Test crawlPeers dials the peer pexR.crawlPeers([]*p2p.NetAddress{peerSwitch.NetAddress()}) @@ -335,28 +313,23 @@ func TestPEXReactorSeedMode(t *testing.T) { func TestPEXReactorDoesNotDisconnectFromPersistentPeerInSeedMode(t *testing.T) { // directory to store address books - dir, err := ioutil.TempDir("", "pex_reactor") - require.Nil(t, err) - defer os.RemoveAll(dir) + dir := t.TempDir() pexRConfig := &ReactorConfig{SeedMode: true, SeedDisconnectWaitPeriod: 1 * time.Millisecond} - pexR, book := createReactor(pexRConfig) - defer teardownReactor(book) - + pexR, book := createReactor(t, pexRConfig) sw := createSwitchAndAddReactors(pexR) + sw.SetAddrBook(book) - err = sw.Start() - require.NoError(t, err) - defer sw.Stop() // nolint:errcheck // ignore for tests + require.NoError(t, sw.Start()) + t.Cleanup(func() { _ = sw.Stop() }) assert.Zero(t, sw.Peers().Size()) peerSwitch := testCreateDefaultPeer(dir, 1) require.NoError(t, peerSwitch.Start()) - defer peerSwitch.Stop() // nolint:errcheck // ignore for tests + t.Cleanup(func() { _ = peerSwitch.Stop() }) - err = sw.AddPersistentPeers([]string{peerSwitch.NetAddress().String()}) - require.NoError(t, err) + require.NoError(t, sw.AddPersistentPeers([]string{peerSwitch.NetAddress().String()})) // 1. Test crawlPeers dials the peer pexR.crawlPeers([]*p2p.NetAddress{peerSwitch.NetAddress()}) @@ -373,22 +346,16 @@ func TestPEXReactorDoesNotDisconnectFromPersistentPeerInSeedMode(t *testing.T) { func TestPEXReactorDialsPeerUpToMaxAttemptsInSeedMode(t *testing.T) { // directory to store address books - dir, err := ioutil.TempDir("", "pex_reactor") - require.Nil(t, err) - defer os.RemoveAll(dir) - - pexR, book := createReactor(&ReactorConfig{SeedMode: true}) - defer teardownReactor(book) - + pexR, book := createReactor(t, &ReactorConfig{SeedMode: true}) sw := createSwitchAndAddReactors(pexR) + sw.SetAddrBook(book) // No need to start sw since crawlPeers is called manually here. peer := mock.NewPeer(nil) addr := peer.SocketAddr() - err = book.AddAddress(addr, addr) - require.NoError(t, err) + require.NoError(t, book.AddAddress(addr, addr)) assert.True(t, book.HasAddress(addr)) @@ -409,9 +376,7 @@ func TestPEXReactorSeedModeFlushStop(t *testing.T) { switches := make([]*p2p.Switch, N) // directory to store address books - dir, err := ioutil.TempDir("", "pex_reactor") - require.Nil(t, err) - defer os.RemoveAll(dir) + dir := t.TempDir() books := make([]AddrBook, N) logger := log.TestingLogger() @@ -447,8 +412,7 @@ func TestPEXReactorSeedModeFlushStop(t *testing.T) { reactor := switches[0].Reactors()["pex"].(*Reactor) peerID := switches[1].NodeInfo().ID() - err = switches[1].DialPeerWithAddress(switches[0].NetAddress()) - assert.NoError(t, err) + assert.NoError(t, switches[1].DialPeerWithAddress(switches[0].NetAddress())) // sleep up to a second while waiting for the peer to send us a message. // this isn't perfect since it's possible the peer sends us a msg and we FlushStop @@ -479,9 +443,8 @@ func TestPEXReactorSeedModeFlushStop(t *testing.T) { func TestPEXReactorDoesNotAddPrivatePeersToAddrBook(t *testing.T) { peer := p2p.CreateRandomPeer(false) - pexR, book := createReactor(&ReactorConfig{}) + pexR, book := createReactor(t, &ReactorConfig{}) book.AddPrivateIDs([]string{string(peer.NodeInfo().ID())}) - defer teardownReactor(book) // we have to send a request to receive responses pexR.RequestAddrs(peer) @@ -496,10 +459,9 @@ func TestPEXReactorDoesNotAddPrivatePeersToAddrBook(t *testing.T) { } func TestPEXReactorDialPeer(t *testing.T) { - pexR, book := createReactor(&ReactorConfig{}) - defer teardownReactor(book) - + pexR, book := createReactor(t, &ReactorConfig{}) sw := createSwitchAndAddReactors(pexR) + sw.SetAddrBook(book) peer := mock.NewPeer(nil) @@ -644,13 +606,9 @@ func testCreatePeerWithSeed(dir string, id int, seed *p2p.Switch) *p2p.Switch { return testCreatePeerWithConfig(dir, id, conf) } -func createReactor(conf *ReactorConfig) (r *Reactor, book AddrBook) { +func createReactor(t *testing.T, conf *ReactorConfig) (r *Reactor, book AddrBook) { // directory to store address book - dir, err := ioutil.TempDir("", "pex_reactor") - if err != nil { - panic(err) - } - book = NewAddrBook(filepath.Join(dir, "addrbook.json"), true) + book = NewAddrBook(filepath.Join(t.TempDir(), "addrbook.json"), true) book.SetLogger(log.TestingLogger()) r = NewReactor(book, conf) @@ -658,14 +616,6 @@ func createReactor(conf *ReactorConfig) (r *Reactor, book AddrBook) { return } -func teardownReactor(book AddrBook) { - // FIXME Shouldn't rely on .(*addrBook) assertion - err := os.RemoveAll(filepath.Dir(book.(*addrBook).FilePath())) - if err != nil { - panic(err) - } -} - func createSwitchAndAddReactors(reactors ...p2p.Reactor) *p2p.Switch { sw := p2p.MakeSwitch(cfg, 0, "127.0.0.1", "123.123.123", func(i int, sw *p2p.Switch) *p2p.Switch { return sw }) sw.SetLogger(log.TestingLogger()) @@ -677,7 +627,6 @@ func createSwitchAndAddReactors(reactors ...p2p.Reactor) *p2p.Switch { } func TestPexVectors(t *testing.T) { - addr := tmp2p.NetAddress{ ID: "1", IP: "127.0.0.1", diff --git a/p2p/trust/store_test.go b/p2p/trust/store_test.go index df0f14a04..ecf17dc4a 100644 --- a/p2p/trust/store_test.go +++ b/p2p/trust/store_test.go @@ -5,8 +5,6 @@ package trust import ( "fmt" - "io/ioutil" - "os" "testing" "github.com/stretchr/testify/assert" @@ -17,9 +15,7 @@ import ( ) func TestTrustMetricStoreSaveLoad(t *testing.T) { - dir, err := ioutil.TempDir("", "trust_test") - require.NoError(t, err) - defer os.Remove(dir) + dir := t.TempDir() historyDB, err := dbm.NewDB("trusthistory", "goleveldb", dir) require.NoError(t, err)