use testing.T.{Cleanup,TempDir}() in tests

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)
@@ -414,9 +393,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 +418,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 +438,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 +469,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 +562,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 +688,24 @@ 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 {
 	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")
+	fname = createTempFileName(t, "addrbook_test")
 
 	book = NewAddrBook(fname, true).(*addrBook)
 	book.SetLogger(log.TestingLogger())

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",