diff --git a/p2p/pex/addrbook_test.go b/p2p/pex/addrbook_test.go index ad41d5562..8812c9ca2 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) @@ -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()) 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",