From 1cd7e3aee62a4882331a79c75d14feffb13148e8 Mon Sep 17 00:00:00 2001 From: Eng Zer Jun Date: Mon, 15 Aug 2022 21:04:39 +0800 Subject: [PATCH 01/23] aliasmgr: use `T.TempDir` to create temporary test directory Signed-off-by: Eng Zer Jun --- aliasmgr/aliasmgr_test.go | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/aliasmgr/aliasmgr_test.go b/aliasmgr/aliasmgr_test.go index 291acd9c3..9ae7f82df 100644 --- a/aliasmgr/aliasmgr_test.go +++ b/aliasmgr/aliasmgr_test.go @@ -1,9 +1,7 @@ package aliasmgr import ( - "io/ioutil" "math/rand" - "os" "path/filepath" "testing" @@ -18,11 +16,7 @@ func TestAliasStorePeerAlias(t *testing.T) { t.Parallel() // Create the backend database and use this to create the aliasStore. - dbDir, err := ioutil.TempDir("", "aliasStore") - require.NoError(t, err) - defer os.RemoveAll(dbDir) - - dbPath := filepath.Join(dbDir, "testdb") + dbPath := filepath.Join(t.TempDir(), "testdb") db, err := kvdb.Create( kvdb.BoltBackendName, dbPath, true, kvdb.DefaultDBTimeout, ) @@ -51,11 +45,7 @@ func TestAliasStoreRequest(t *testing.T) { t.Parallel() // Create the backend database and use this to create the aliasStore. - dbDir, err := ioutil.TempDir("", "aliasStore") - require.NoError(t, err) - defer os.RemoveAll(dbDir) - - dbPath := filepath.Join(dbDir, "testdb") + dbPath := filepath.Join(t.TempDir(), "testdb") db, err := kvdb.Create( kvdb.BoltBackendName, dbPath, true, kvdb.DefaultDBTimeout, ) From 994f4d67a15afb6a69e1594293eacc56025c4892 Mon Sep 17 00:00:00 2001 From: Eng Zer Jun Date: Mon, 15 Aug 2022 21:04:51 +0800 Subject: [PATCH 02/23] cert: use `T.TempDir` to create temporary test directory Signed-off-by: Eng Zer Jun --- cert/selfsigned_test.go | 22 ++++++---------------- 1 file changed, 6 insertions(+), 16 deletions(-) diff --git a/cert/selfsigned_test.go b/cert/selfsigned_test.go index dd9953e2a..3f5e694b2 100644 --- a/cert/selfsigned_test.go +++ b/cert/selfsigned_test.go @@ -1,7 +1,6 @@ package cert_test import ( - "io/ioutil" "testing" "time" @@ -21,16 +20,13 @@ var ( // TestIsOutdatedCert checks that we'll consider the TLS certificate outdated // if the ip addresses or dns names don't match. func TestIsOutdatedCert(t *testing.T) { - tempDir, err := ioutil.TempDir("", "certtest") - if err != nil { - t.Fatal(err) - } + tempDir := t.TempDir() certPath := tempDir + "/tls.cert" keyPath := tempDir + "/tls.key" // Generate TLS files with two extra IPs and domains. - err = cert.GenCertPair( + err := cert.GenCertPair( "lnd autogenerated cert", certPath, keyPath, extraIPs[:2], extraDomains[:2], false, testTLSCertDuration, ) @@ -76,16 +72,13 @@ func TestIsOutdatedCert(t *testing.T) { // nor dulicates in the lists, matter for whether we consider the certificate // outdated. func TestIsOutdatedPermutation(t *testing.T) { - tempDir, err := ioutil.TempDir("", "certtest") - if err != nil { - t.Fatal(err) - } + tempDir := t.TempDir() certPath := tempDir + "/tls.cert" keyPath := tempDir + "/tls.key" // Generate TLS files from the IPs and domains. - err = cert.GenCertPair( + err := cert.GenCertPair( "lnd autogenerated cert", certPath, keyPath, extraIPs[:], extraDomains[:], false, testTLSCertDuration, ) @@ -143,16 +136,13 @@ func TestIsOutdatedPermutation(t *testing.T) { // TestTLSDisableAutofill checks that setting the --tlsdisableautofill flag // does not add interface ip addresses or hostnames to the cert. func TestTLSDisableAutofill(t *testing.T) { - tempDir, err := ioutil.TempDir("", "certtest") - if err != nil { - t.Fatal(err) - } + tempDir := t.TempDir() certPath := tempDir + "/tls.cert" keyPath := tempDir + "/tls.key" // Generate TLS files with two extra IPs and domains and no interface IPs. - err = cert.GenCertPair( + err := cert.GenCertPair( "lnd autogenerated cert", certPath, keyPath, extraIPs[:2], extraDomains[:2], true, testTLSCertDuration, ) From 712177ee03ccb60f7659d9bcef9a2784a368b8cd Mon Sep 17 00:00:00 2001 From: Eng Zer Jun Date: Mon, 15 Aug 2022 21:05:01 +0800 Subject: [PATCH 03/23] chainntnfs: use `T.TempDir` to create temporary test directory Signed-off-by: Eng Zer Jun --- chainntnfs/bitcoindnotify/bitcoind_test.go | 9 +++++---- chainntnfs/btcdnotify/btcd_test.go | 9 +++++---- chainntnfs/height_hint_cache_test.go | 9 +++++---- chainntnfs/test/test_interface.go | 6 +----- chainntnfs/test_utils.go | 17 ++++------------- 5 files changed, 20 insertions(+), 30 deletions(-) diff --git a/chainntnfs/bitcoindnotify/bitcoind_test.go b/chainntnfs/bitcoindnotify/bitcoind_test.go index 39a29e9b3..63b2c44dc 100644 --- a/chainntnfs/bitcoindnotify/bitcoind_test.go +++ b/chainntnfs/bitcoindnotify/bitcoind_test.go @@ -5,7 +5,6 @@ package bitcoindnotify import ( "bytes" - "io/ioutil" "testing" "time" @@ -35,10 +34,12 @@ var ( func initHintCache(t *testing.T) *chainntnfs.HeightHintCache { t.Helper() - tempDir, err := ioutil.TempDir("", "kek") - require.NoError(t, err, "unable to create temp dir") - db, err := channeldb.Open(tempDir) + db, err := channeldb.Open(t.TempDir()) require.NoError(t, err, "unable to create db") + t.Cleanup(func() { + require.NoError(t, db.Close()) + }) + testCfg := chainntnfs.CacheConfig{ QueryDisable: false, } diff --git a/chainntnfs/btcdnotify/btcd_test.go b/chainntnfs/btcdnotify/btcd_test.go index 4ff3b89d4..2db472556 100644 --- a/chainntnfs/btcdnotify/btcd_test.go +++ b/chainntnfs/btcdnotify/btcd_test.go @@ -5,7 +5,6 @@ package btcdnotify import ( "bytes" - "io/ioutil" "testing" "github.com/btcsuite/btcd/chaincfg/chainhash" @@ -33,10 +32,12 @@ var ( func initHintCache(t *testing.T) *chainntnfs.HeightHintCache { t.Helper() - tempDir, err := ioutil.TempDir("", "kek") - require.NoError(t, err, "unable to create temp dir") - db, err := channeldb.Open(tempDir) + db, err := channeldb.Open(t.TempDir()) require.NoError(t, err, "unable to create db") + t.Cleanup(func() { + require.NoError(t, db.Close()) + }) + testCfg := chainntnfs.CacheConfig{ QueryDisable: false, } diff --git a/chainntnfs/height_hint_cache_test.go b/chainntnfs/height_hint_cache_test.go index f984b871d..e97a3d8ef 100644 --- a/chainntnfs/height_hint_cache_test.go +++ b/chainntnfs/height_hint_cache_test.go @@ -2,7 +2,6 @@ package chainntnfs import ( "bytes" - "io/ioutil" "testing" "github.com/btcsuite/btcd/chaincfg/chainhash" @@ -24,13 +23,15 @@ func initHintCache(t *testing.T) *HeightHintCache { func initHintCacheWithConfig(t *testing.T, cfg CacheConfig) *HeightHintCache { t.Helper() - tempDir, err := ioutil.TempDir("", "kek") - require.NoError(t, err, "unable to create temp dir") - db, err := channeldb.Open(tempDir) + db, err := channeldb.Open(t.TempDir()) require.NoError(t, err, "unable to create db") hintCache, err := NewHeightHintCache(cfg, db.Backend) require.NoError(t, err, "unable to create hint cache") + t.Cleanup(func() { + require.NoError(t, db.Close()) + }) + return hintCache } diff --git a/chainntnfs/test/test_interface.go b/chainntnfs/test/test_interface.go index 3f81e77de..cf1aa8b9a 100644 --- a/chainntnfs/test/test_interface.go +++ b/chainntnfs/test/test_interface.go @@ -6,7 +6,6 @@ package chainntnfstest import ( "bytes" "fmt" - "io/ioutil" "log" "sync" "testing" @@ -1833,10 +1832,7 @@ func TestInterfaces(t *testing.T, targetBackEnd string) { } // Initialize a height hint cache for each notifier. - tempDir, err := ioutil.TempDir("", "channeldb") - if err != nil { - t.Fatalf("unable to create temp dir: %v", err) - } + tempDir := t.TempDir() db, err := channeldb.Open(tempDir) if err != nil { t.Fatalf("unable to create db: %v", err) diff --git a/chainntnfs/test_utils.go b/chainntnfs/test_utils.go index 2898b30cc..2040593ae 100644 --- a/chainntnfs/test_utils.go +++ b/chainntnfs/test_utils.go @@ -6,9 +6,7 @@ package chainntnfs import ( "errors" "fmt" - "io/ioutil" "math/rand" - "os" "os/exec" "path/filepath" "testing" @@ -196,8 +194,7 @@ func NewBitcoindBackend(t *testing.T, minerAddr string, txindex, t.Helper() - tempBitcoindDir, err := ioutil.TempDir("", "bitcoind") - require.NoError(t, err, "unable to create temp dir") + tempBitcoindDir := t.TempDir() rpcPort := rand.Intn(65536-1024) + 1024 zmqBlockHost := "ipc:///" + tempBitcoindDir + "/blocks.socket" @@ -220,7 +217,6 @@ func NewBitcoindBackend(t *testing.T, minerAddr string, txindex, bitcoind := exec.Command("bitcoind", args...) if err := bitcoind.Start(); err != nil { - os.RemoveAll(tempBitcoindDir) t.Fatalf("unable to start bitcoind: %v", err) } @@ -251,7 +247,8 @@ func NewBitcoindBackend(t *testing.T, minerAddr string, txindex, } var conn *chain.BitcoindConn - err = wait.NoError(func() error { + err := wait.NoError(func() error { + var err error conn, err = chain.NewBitcoindConn(cfg) if err != nil { return err @@ -262,7 +259,6 @@ func NewBitcoindBackend(t *testing.T, minerAddr string, txindex, if err != nil { bitcoind.Process.Kill() bitcoind.Wait() - os.RemoveAll(tempBitcoindDir) t.Fatalf("unable to establish connection to bitcoind: %v", err) } @@ -270,7 +266,6 @@ func NewBitcoindBackend(t *testing.T, minerAddr string, txindex, conn.Stop() bitcoind.Process.Kill() bitcoind.Wait() - os.RemoveAll(tempBitcoindDir) } } @@ -279,15 +274,13 @@ func NewBitcoindBackend(t *testing.T, minerAddr string, txindex, func NewNeutrinoBackend(t *testing.T, minerAddr string) (*neutrino.ChainService, func()) { t.Helper() - spvDir, err := ioutil.TempDir("", "neutrino") - require.NoError(t, err, "unable to create temp dir") + spvDir := t.TempDir() dbName := filepath.Join(spvDir, "neutrino.db") spvDatabase, err := walletdb.Create( "bdb", dbName, true, kvdb.DefaultDBTimeout, ) if err != nil { - os.RemoveAll(spvDir) t.Fatalf("unable to create walletdb: %v", err) } @@ -301,7 +294,6 @@ func NewNeutrinoBackend(t *testing.T, minerAddr string) (*neutrino.ChainService, } spvNode, err := neutrino.NewChainService(spvConfig) if err != nil { - os.RemoveAll(spvDir) spvDatabase.Close() t.Fatalf("unable to create neutrino: %v", err) } @@ -316,6 +308,5 @@ func NewNeutrinoBackend(t *testing.T, minerAddr string) (*neutrino.ChainService, return spvNode, func() { spvNode.Stop() spvDatabase.Close() - os.RemoveAll(spvDir) } } From 519df2a15d3dbf6d5617418dab2da54137cb392a Mon Sep 17 00:00:00 2001 From: Eng Zer Jun Date: Mon, 15 Aug 2022 21:05:15 +0800 Subject: [PATCH 04/23] chanbackup: use `T.TempDir` to create temporary test directory Signed-off-by: Eng Zer Jun --- chanbackup/backupfile_test.go | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/chanbackup/backupfile_test.go b/chanbackup/backupfile_test.go index 4992b1c68..14747f47b 100644 --- a/chanbackup/backupfile_test.go +++ b/chanbackup/backupfile_test.go @@ -52,9 +52,7 @@ func assertFileDeleted(t *testing.T, filePath string) { func TestUpdateAndSwap(t *testing.T) { t.Parallel() - tempTestDir, err := ioutil.TempDir("", "") - require.NoError(t, err, "unable to make temp dir") - defer os.Remove(tempTestDir) + tempTestDir := t.TempDir() testCases := []struct { fileName string @@ -205,10 +203,13 @@ func TestExtractMulti(t *testing.T) { packedMulti := PackedMulti(b.Bytes()) // Finally, we'll make a new temporary file, then write out the packed - // multi directly to to it. - tempFile, err := ioutil.TempFile("", "") + // multi directly to it. + tempFile, err := os.CreateTemp("", "") require.NoError(t, err, "unable to create temp file") - defer os.Remove(tempFile.Name()) + t.Cleanup(func() { + require.NoError(t, tempFile.Close()) + require.NoError(t, os.Remove(tempFile.Name())) + }) _, err = tempFile.Write(packedMulti) require.NoError(t, err, "unable to write temp file") From dd07cb850d968fe65ca2a9ce53c6ae5ed41bd1a6 Mon Sep 17 00:00:00 2001 From: Eng Zer Jun Date: Mon, 15 Aug 2022 21:05:24 +0800 Subject: [PATCH 05/23] channeldb: use `T.TempDir` to create temporary test directory Signed-off-by: Eng Zer Jun --- channeldb/db_test.go | 10 +- channeldb/forwarding_package_test.go | 9 +- channeldb/graph_test.go | 132 ++++++------------- channeldb/meta_test.go | 8 +- channeldb/migration30/iterator_test.go | 15 +-- channeldb/migration30/migration_test.go | 3 +- channeldb/migration_01_to_11/channel_test.go | 30 ++--- channeldb/migration_01_to_11/meta_test.go | 3 +- channeldb/migtest/migtest.go | 35 +++-- 9 files changed, 79 insertions(+), 166 deletions(-) diff --git a/channeldb/db_test.go b/channeldb/db_test.go index 0a6f2a9ff..adf95d79f 100644 --- a/channeldb/db_test.go +++ b/channeldb/db_test.go @@ -1,11 +1,9 @@ package channeldb import ( - "io/ioutil" "math" "math/rand" "net" - "os" "path/filepath" "reflect" "testing" @@ -32,9 +30,7 @@ func TestOpenWithCreate(t *testing.T) { // First, create a temporary directory to be used for the duration of // this test. - tempDirName, err := ioutil.TempDir("", "channeldb") - require.NoError(t, err, "unable to create temp dir") - defer os.RemoveAll(tempDirName) + tempDirName := t.TempDir() // Next, open thereby creating channeldb for the first time. dbPath := filepath.Join(tempDirName, "cdb") @@ -70,9 +66,7 @@ func TestWipe(t *testing.T) { // First, create a temporary directory to be used for the duration of // this test. - tempDirName, err := ioutil.TempDir("", "channeldb") - require.NoError(t, err, "unable to create temp dir") - defer os.RemoveAll(tempDirName) + tempDirName := t.TempDir() // Next, open thereby creating channeldb for the first time. dbPath := filepath.Join(tempDirName, "cdb") diff --git a/channeldb/forwarding_package_test.go b/channeldb/forwarding_package_test.go index c8977c8ea..1aabe7614 100644 --- a/channeldb/forwarding_package_test.go +++ b/channeldb/forwarding_package_test.go @@ -2,7 +2,6 @@ package channeldb_test import ( "bytes" - "io/ioutil" "path/filepath" "runtime" "testing" @@ -843,13 +842,7 @@ func loadFwdPkgs(t *testing.T, db kvdb.Backend, // provided path is an empty, it will create a temp dir/file to use. func makeFwdPkgDB(t *testing.T, path string) kvdb.Backend { // nolint:unparam if path == "" { - var err error - path, err = ioutil.TempDir("", "fwdpkgdb") - if err != nil { - t.Fatalf("unable to create temp path: %v", err) - } - - path = filepath.Join(path, "fwdpkg.db") + path = filepath.Join(t.TempDir(), "fwdpkg.db") } bdb, err := kvdb.Create( diff --git a/channeldb/graph_test.go b/channeldb/graph_test.go index f2536855e..35c7050eb 100644 --- a/channeldb/graph_test.go +++ b/channeldb/graph_test.go @@ -7,11 +7,9 @@ import ( "errors" "fmt" "image/color" - "io/ioutil" "math" prand "math/rand" "net" - "os" "reflect" "runtime" "sync" @@ -53,27 +51,18 @@ var ( testPub = route.Vertex{2, 202, 4} ) -// MakeTestGraph creates a new instance of the ChannelGraph for testing -// purposes. A callback which cleans up the created temporary directories is -// also returned and intended to be executed after the test completes. -func MakeTestGraph(modifiers ...OptionModifier) (*ChannelGraph, func(), error) { - // First, create a temporary directory to be used for the duration of - // this test. - tempDirName, err := ioutil.TempDir("", "channelgraph") - if err != nil { - return nil, nil, err - } - +// MakeTestGraph creates a new instance of the ChannelGraph for testing purposes. +func MakeTestGraph(t testing.TB, modifiers ...OptionModifier) (*ChannelGraph, error) { opts := DefaultOptions() for _, modifier := range modifiers { modifier(&opts) } // Next, create channelgraph for the first time. - backend, backendCleanup, err := kvdb.GetTestBackend(tempDirName, "cgr") + backend, backendCleanup, err := kvdb.GetTestBackend(t.TempDir(), "cgr") if err != nil { backendCleanup() - return nil, nil, err + return nil, err } graph, err := NewChannelGraph( @@ -83,17 +72,15 @@ func MakeTestGraph(modifiers ...OptionModifier) (*ChannelGraph, func(), error) { ) if err != nil { backendCleanup() - _ = os.RemoveAll(tempDirName) - return nil, nil, err + return nil, err } - cleanUp := func() { + t.Cleanup(func() { _ = backend.Close() backendCleanup() - _ = os.RemoveAll(tempDirName) - } + }) - return graph, cleanUp, nil + return graph, nil } func createLightningNode(db kvdb.Backend, priv *btcec.PrivateKey) (*LightningNode, error) { @@ -127,8 +114,7 @@ func createTestVertex(db kvdb.Backend) (*LightningNode, error) { func TestNodeInsertionAndDeletion(t *testing.T) { t.Parallel() - graph, cleanUp, err := MakeTestGraph() - defer cleanUp() + graph, err := MakeTestGraph(t) require.NoError(t, err, "unable to make test database") // We'd like to test basic insertion/deletion for vertexes from the @@ -189,8 +175,7 @@ func TestNodeInsertionAndDeletion(t *testing.T) { func TestPartialNode(t *testing.T) { t.Parallel() - graph, cleanUp, err := MakeTestGraph() - defer cleanUp() + graph, err := MakeTestGraph(t) require.NoError(t, err, "unable to make test database") // We want to be able to insert nodes into the graph that only has the @@ -247,8 +232,7 @@ func TestPartialNode(t *testing.T) { func TestAliasLookup(t *testing.T) { t.Parallel() - graph, cleanUp, err := MakeTestGraph() - defer cleanUp() + graph, err := MakeTestGraph(t) require.NoError(t, err, "unable to make test database") // We'd like to test the alias index within the database, so first @@ -287,9 +271,8 @@ func TestAliasLookup(t *testing.T) { func TestSourceNode(t *testing.T) { t.Parallel() - graph, cleanUp, err := MakeTestGraph() + graph, err := MakeTestGraph(t) require.NoError(t, err, "unable to make test database") - defer cleanUp() // We'd like to test the setting/getting of the source node, so we // first create a fake node to use within the test. @@ -320,8 +303,7 @@ func TestSourceNode(t *testing.T) { func TestEdgeInsertionDeletion(t *testing.T) { t.Parallel() - graph, cleanUp, err := MakeTestGraph() - defer cleanUp() + graph, err := MakeTestGraph(t) require.NoError(t, err, "unable to make test database") // We'd like to test the insertion/deletion of edges, so we create two @@ -445,8 +427,7 @@ func createEdge(height, txIndex uint32, txPosition uint16, outPointIndex uint32, func TestDisconnectBlockAtHeight(t *testing.T) { t.Parallel() - graph, cleanUp, err := MakeTestGraph() - defer cleanUp() + graph, err := MakeTestGraph(t) require.NoError(t, err, "unable to make test database") sourceNode, err := createTestVertex(graph.db) @@ -717,8 +698,7 @@ func createChannelEdge(db kvdb.Backend, node1, node2 *LightningNode) (*ChannelEd func TestEdgeInfoUpdates(t *testing.T) { t.Parallel() - graph, cleanUp, err := MakeTestGraph() - defer cleanUp() + graph, err := MakeTestGraph(t) require.NoError(t, err, "unable to make test database") // We'd like to test the update of edges inserted into the database, so @@ -1022,8 +1002,7 @@ func newEdgePolicy(chanID uint64, db kvdb.Backend, func TestGraphTraversal(t *testing.T) { t.Parallel() - graph, cleanUp, err := MakeTestGraph() - defer cleanUp() + graph, err := MakeTestGraph(t) require.NoError(t, err, "unable to make test database") // We'd like to test some of the graph traversal capabilities within @@ -1110,8 +1089,7 @@ func TestGraphTraversal(t *testing.T) { func TestGraphTraversalCacheable(t *testing.T) { t.Parallel() - graph, cleanUp, err := MakeTestGraph() - defer cleanUp() + graph, err := MakeTestGraph(t) require.NoError(t, err, "unable to make test database") // We'd like to test some of the graph traversal capabilities within @@ -1174,8 +1152,7 @@ func TestGraphTraversalCacheable(t *testing.T) { func TestGraphCacheTraversal(t *testing.T) { t.Parallel() - graph, cleanUp, err := MakeTestGraph() - defer cleanUp() + graph, err := MakeTestGraph(t) require.NoError(t, err) // We'd like to test some of the graph traversal capabilities within @@ -1412,8 +1389,7 @@ func assertChanViewEqualChanPoints(t *testing.T, a []EdgePoint, b []*wire.OutPoi func TestGraphPruning(t *testing.T) { t.Parallel() - graph, cleanUp, err := MakeTestGraph() - defer cleanUp() + graph, err := MakeTestGraph(t) require.NoError(t, err, "unable to make test database") sourceNode, err := createTestVertex(graph.db) @@ -1603,8 +1579,7 @@ func TestGraphPruning(t *testing.T) { func TestHighestChanID(t *testing.T) { t.Parallel() - graph, cleanUp, err := MakeTestGraph() - defer cleanUp() + graph, err := MakeTestGraph(t) require.NoError(t, err, "unable to make test database") // If we don't yet have any channels in the database, then we should @@ -1666,8 +1641,7 @@ func TestHighestChanID(t *testing.T) { func TestChanUpdatesInHorizon(t *testing.T) { t.Parallel() - graph, cleanUp, err := MakeTestGraph() - defer cleanUp() + graph, err := MakeTestGraph(t) require.NoError(t, err, "unable to make test database") // If we issue an arbitrary query before any channel updates are @@ -1826,8 +1800,7 @@ func TestChanUpdatesInHorizon(t *testing.T) { func TestNodeUpdatesInHorizon(t *testing.T) { t.Parallel() - graph, cleanUp, err := MakeTestGraph() - defer cleanUp() + graph, err := MakeTestGraph(t) require.NoError(t, err, "unable to make test database") startTime := time.Unix(1234, 0) @@ -1943,8 +1916,7 @@ func TestNodeUpdatesInHorizon(t *testing.T) { func TestFilterKnownChanIDs(t *testing.T) { t.Parallel() - graph, cleanUp, err := MakeTestGraph() - defer cleanUp() + graph, err := MakeTestGraph(t) require.NoError(t, err, "unable to make test database") // If we try to filter out a set of channel ID's before we even know of @@ -2050,8 +2022,7 @@ func TestFilterKnownChanIDs(t *testing.T) { func TestFilterChannelRange(t *testing.T) { t.Parallel() - graph, cleanUp, err := MakeTestGraph() - defer cleanUp() + graph, err := MakeTestGraph(t) require.NoError(t, err, "unable to make test database") // We'll first populate our graph with two nodes. All channels created @@ -2174,8 +2145,7 @@ func TestFilterChannelRange(t *testing.T) { func TestFetchChanInfos(t *testing.T) { t.Parallel() - graph, cleanUp, err := MakeTestGraph() - defer cleanUp() + graph, err := MakeTestGraph(t) require.NoError(t, err, "unable to make test database") // We'll first populate our graph with two nodes. All channels created @@ -2284,8 +2254,7 @@ func TestFetchChanInfos(t *testing.T) { func TestIncompleteChannelPolicies(t *testing.T) { t.Parallel() - graph, cleanUp, err := MakeTestGraph() - defer cleanUp() + graph, err := MakeTestGraph(t) require.NoError(t, err, "unable to make test database") // Create two nodes. @@ -2384,8 +2353,7 @@ func TestIncompleteChannelPolicies(t *testing.T) { func TestChannelEdgePruningUpdateIndexDeletion(t *testing.T) { t.Parallel() - graph, cleanUp, err := MakeTestGraph() - defer cleanUp() + graph, err := MakeTestGraph(t) require.NoError(t, err, "unable to make test database") sourceNode, err := createTestVertex(graph.db) @@ -2528,8 +2496,7 @@ func TestChannelEdgePruningUpdateIndexDeletion(t *testing.T) { func TestPruneGraphNodes(t *testing.T) { t.Parallel() - graph, cleanUp, err := MakeTestGraph() - defer cleanUp() + graph, err := MakeTestGraph(t) require.NoError(t, err, "unable to make test database") // We'll start off by inserting our source node, to ensure that it's @@ -2601,8 +2568,7 @@ func TestPruneGraphNodes(t *testing.T) { func TestAddChannelEdgeShellNodes(t *testing.T) { t.Parallel() - graph, cleanUp, err := MakeTestGraph() - defer cleanUp() + graph, err := MakeTestGraph(t) require.NoError(t, err, "unable to make test database") // To start, we'll create two nodes, and only add one of them to the @@ -2643,8 +2609,7 @@ func TestAddChannelEdgeShellNodes(t *testing.T) { func TestNodePruningUpdateIndexDeletion(t *testing.T) { t.Parallel() - graph, cleanUp, err := MakeTestGraph() - defer cleanUp() + graph, err := MakeTestGraph(t) require.NoError(t, err, "unable to make test database") // We'll first populate our graph with a single node that will be @@ -2703,8 +2668,7 @@ func TestNodeIsPublic(t *testing.T) { // We'll need to create a separate database and channel graph for each // participant to replicate real-world scenarios (private edges being in // some graphs but not others, etc.). - aliceGraph, cleanUp, err := MakeTestGraph() - defer cleanUp() + aliceGraph, err := MakeTestGraph(t) require.NoError(t, err, "unable to make test database") aliceNode, err := createTestVertex(aliceGraph.db) require.NoError(t, err, "unable to create test node") @@ -2712,8 +2676,7 @@ func TestNodeIsPublic(t *testing.T) { t.Fatalf("unable to set source node: %v", err) } - bobGraph, cleanUp, err := MakeTestGraph() - defer cleanUp() + bobGraph, err := MakeTestGraph(t) require.NoError(t, err, "unable to make test database") bobNode, err := createTestVertex(bobGraph.db) require.NoError(t, err, "unable to create test node") @@ -2721,8 +2684,7 @@ func TestNodeIsPublic(t *testing.T) { t.Fatalf("unable to set source node: %v", err) } - carolGraph, cleanUp, err := MakeTestGraph() - defer cleanUp() + carolGraph, err := MakeTestGraph(t) require.NoError(t, err, "unable to make test database") carolNode, err := createTestVertex(carolGraph.db) require.NoError(t, err, "unable to create test node") @@ -2841,9 +2803,8 @@ func TestNodeIsPublic(t *testing.T) { func TestDisabledChannelIDs(t *testing.T) { t.Parallel() - graph, cleanUp, err := MakeTestGraph() + graph, err := MakeTestGraph(t) require.NoError(t, err, "unable to make test database") - defer cleanUp() // Create first node and add it to the graph. node1, err := createTestVertex(graph.db) @@ -2925,8 +2886,7 @@ func TestDisabledChannelIDs(t *testing.T) { func TestEdgePolicyMissingMaxHtcl(t *testing.T) { t.Parallel() - graph, cleanUp, err := MakeTestGraph() - defer cleanUp() + graph, err := MakeTestGraph(t) require.NoError(t, err, "unable to make test database") // We'd like to test the update of edges inserted into the database, so @@ -3087,8 +3047,7 @@ func TestGraphZombieIndex(t *testing.T) { t.Parallel() // We'll start by creating our test graph along with a test edge. - graph, cleanUp, err := MakeTestGraph() - defer cleanUp() + graph, err := MakeTestGraph(t) require.NoError(t, err, "unable to create test database") node1, err := createTestVertex(graph.db) @@ -3274,9 +3233,8 @@ func TestLightningNodeSigVerification(t *testing.T) { } // Create a LightningNode from the same private key. - graph, cleanUp, err := MakeTestGraph() + graph, err := MakeTestGraph(t) require.NoError(t, err, "unable to make test database") - defer cleanUp() node, err := createLightningNode(graph.db, priv) require.NoError(t, err, "unable to create node") @@ -3318,9 +3276,8 @@ func TestComputeFee(t *testing.T) { func TestBatchedAddChannelEdge(t *testing.T) { t.Parallel() - graph, cleanUp, err := MakeTestGraph() + graph, err := MakeTestGraph(t) require.Nil(t, err) - defer cleanUp() sourceNode, err := createTestVertex(graph.db) require.Nil(t, err) @@ -3400,9 +3357,8 @@ func TestBatchedAddChannelEdge(t *testing.T) { func TestBatchedUpdateEdgePolicy(t *testing.T) { t.Parallel() - graph, cleanUp, err := MakeTestGraph() + graph, err := MakeTestGraph(t) require.Nil(t, err) - defer cleanUp() // We'd like to test the update of edges inserted into the database, so // we create two vertexes to connect. @@ -3458,9 +3414,8 @@ func TestBatchedUpdateEdgePolicy(t *testing.T) { // BenchmarkForEachChannel is a benchmark test that measures the number of // allocations and the total memory consumed by the full graph traversal. func BenchmarkForEachChannel(b *testing.B) { - graph, cleanUp, err := MakeTestGraph() + graph, err := MakeTestGraph(b) require.Nil(b, err) - defer cleanUp() const numNodes = 100 const numChannels = 4 @@ -3518,8 +3473,7 @@ func BenchmarkForEachChannel(b *testing.B) { // TestGraphCacheForEachNodeChannel tests that the ForEachNodeChannel method // works as expected, and is able to handle nil self edges. func TestGraphCacheForEachNodeChannel(t *testing.T) { - graph, cleanUp, err := MakeTestGraph() - defer cleanUp() + graph, err := MakeTestGraph(t) require.NoError(t, err) // Unset the channel graph cache to simulate the user running with the @@ -3559,9 +3513,7 @@ func TestGraphCacheForEachNodeChannel(t *testing.T) { func TestGraphLoading(t *testing.T) { // First, create a temporary directory to be used for the duration of // this test. - tempDirName, err := ioutil.TempDir("", "channelgraph") - require.NoError(t, err) - defer os.RemoveAll(tempDirName) + tempDirName := t.TempDir() // Next, create the graph for the first time. backend, backendCleanup, err := kvdb.GetTestBackend(tempDirName, "cgr") diff --git a/channeldb/meta_test.go b/channeldb/meta_test.go index 830cee5ba..5b1e45cc7 100644 --- a/channeldb/meta_test.go +++ b/channeldb/meta_test.go @@ -2,8 +2,6 @@ package channeldb import ( "bytes" - "io/ioutil" - "os" "testing" "github.com/go-errors/errors" @@ -422,11 +420,7 @@ func TestMigrationWithoutErrors(t *testing.T) { func TestMigrationReversion(t *testing.T) { t.Parallel() - tempDirName, err := ioutil.TempDir("", "channeldb") - defer func() { - os.RemoveAll(tempDirName) - }() - require.NoError(t, err, "unable to create temp dir") + tempDirName := t.TempDir() backend, cleanup, err := kvdb.GetTestBackend(tempDirName, "cdb") require.NoError(t, err, "unable to get test db backend") diff --git a/channeldb/migration30/iterator_test.go b/channeldb/migration30/iterator_test.go index 5025d417c..8d2206eb5 100644 --- a/channeldb/migration30/iterator_test.go +++ b/channeldb/migration30/iterator_test.go @@ -52,8 +52,7 @@ func TestLocateChanBucket(t *testing.T) { t.Parallel() // Create test database. - cdb, cleanUp, err := migtest.MakeDB() - defer cleanUp() + cdb, err := migtest.MakeDB(t) require.NoError(t, err) // Create a test channel. @@ -138,8 +137,7 @@ func TestFindNextMigrateHeight(t *testing.T) { t.Parallel() // Create test database. - cdb, cleanUp, err := migtest.MakeDB() - defer cleanUp() + cdb, err := migtest.MakeDB(t) require.NoError(t, err) // tester is a helper closure that finds the next migration height. @@ -307,8 +305,7 @@ func TestIterator(t *testing.T) { t.Parallel() // Create test database. - cdb, cleanUp, err := migtest.MakeDB() - defer cleanUp() + cdb, err := migtest.MakeDB(t) require.NoError(t, err) // exitKey is used to signal exit when hitting this key. @@ -383,8 +380,7 @@ func TestIterateBuckets(t *testing.T) { t.Parallel() // Create test database. - cdb, cleanUp, err := migtest.MakeDB() - defer cleanUp() + cdb, err := migtest.MakeDB(t) require.NoError(t, err) // Create three test channels. @@ -659,8 +655,7 @@ func TestLocalNextUpdateNum(t *testing.T) { for _, tc := range testCases { // Create a test database. - cdb, cleanUp, err := migtest.MakeDB() - defer cleanUp() + cdb, err := migtest.MakeDB(t) require.NoError(t, err) tc := tc diff --git a/channeldb/migration30/migration_test.go b/channeldb/migration30/migration_test.go index 573598ec3..c748ebac0 100644 --- a/channeldb/migration30/migration_test.go +++ b/channeldb/migration30/migration_test.go @@ -164,8 +164,7 @@ func TestValidateMigration(t *testing.T) { tc := tc // Create a test db. - cdb, cleanUp, err := migtest.MakeDB() - defer cleanUp() + cdb, err := migtest.MakeDB(t) require.NoError(t, err, "failed to create test db") t.Run(tc.name, func(t *testing.T) { diff --git a/channeldb/migration_01_to_11/channel_test.go b/channeldb/migration_01_to_11/channel_test.go index ff17b113f..76a16c7d0 100644 --- a/channeldb/migration_01_to_11/channel_test.go +++ b/channeldb/migration_01_to_11/channel_test.go @@ -2,9 +2,8 @@ package migration_01_to_11 import ( "bytes" - "io/ioutil" "math/rand" - "os" + "testing" "github.com/btcsuite/btcd/btcec/v2" "github.com/btcsuite/btcd/btcutil" @@ -63,29 +62,18 @@ var ( privKey, pubKey = btcec.PrivKeyFromBytes(key[:]) ) -// makeTestDB creates a new instance of the ChannelDB for testing purposes. A -// callback which cleans up the created temporary directories is also returned -// and intended to be executed after the test completes. -func makeTestDB() (*DB, func(), error) { - // First, create a temporary directory to be used for the duration of - // this test. - tempDirName, err := ioutil.TempDir("", "channeldb") +// makeTestDB creates a new instance of the ChannelDB for testing purposes. +func makeTestDB(t *testing.T) (*DB, error) { + // Create channeldb for the first time. + cdb, err := Open(t.TempDir()) if err != nil { - return nil, nil, err + return nil, err } - - // Next, create channeldb for the first time. - cdb, err := Open(tempDirName) - if err != nil { - return nil, nil, err - } - - cleanUp := func() { + t.Cleanup(func() { cdb.Close() - os.RemoveAll(tempDirName) - } + }) - return cdb, cleanUp, nil + return cdb, nil } func createTestChannelState(cdb *DB) (*OpenChannel, error) { diff --git a/channeldb/migration_01_to_11/meta_test.go b/channeldb/migration_01_to_11/meta_test.go index a655f1cde..513b90d56 100644 --- a/channeldb/migration_01_to_11/meta_test.go +++ b/channeldb/migration_01_to_11/meta_test.go @@ -12,8 +12,7 @@ import ( func applyMigration(t *testing.T, beforeMigration, afterMigration func(d *DB), migrationFunc migration, shouldFail bool) { - cdb, cleanUp, err := makeTestDB() - defer cleanUp() + cdb, err := makeTestDB(t) if err != nil { t.Fatal(err) } diff --git a/channeldb/migtest/migtest.go b/channeldb/migtest/migtest.go index 317312842..65e36308f 100644 --- a/channeldb/migtest/migtest.go +++ b/channeldb/migtest/migtest.go @@ -2,37 +2,38 @@ package migtest import ( "fmt" - "io/ioutil" "os" "testing" "github.com/lightningnetwork/lnd/kvdb" + "github.com/stretchr/testify/require" ) -// MakeDB creates a new instance of the ChannelDB for testing purposes. A -// callback which cleans up the created temporary directories is also returned -// and intended to be executed after the test completes. -func MakeDB() (kvdb.Backend, func(), error) { +// MakeDB creates a new instance of the ChannelDB for testing purposes. +func MakeDB(t testing.TB) (kvdb.Backend, error) { // Create temporary database for mission control. - file, err := ioutil.TempFile("", "*.db") + file, err := os.CreateTemp("", "*.db") if err != nil { - return nil, nil, err + return nil, err } dbPath := file.Name() + t.Cleanup(func() { + require.NoError(t, file.Close()) + require.NoError(t, os.Remove(dbPath)) + }) + db, err := kvdb.Open( kvdb.BoltBackendName, dbPath, true, kvdb.DefaultDBTimeout, ) if err != nil { - return nil, nil, err + return nil, err } + t.Cleanup(func() { + require.NoError(t, db.Close()) + }) - cleanUp := func() { - db.Close() - os.RemoveAll(dbPath) - } - - return db, cleanUp, nil + return db, nil } // ApplyMigration is a helper test function that encapsulates the general steps @@ -43,8 +44,7 @@ func ApplyMigration(t *testing.T, t.Helper() - cdb, cleanUp, err := MakeDB() - defer cleanUp() + cdb, err := MakeDB(t) if err != nil { t.Fatal(err) } @@ -94,8 +94,7 @@ func ApplyMigrationWithDb(t testing.TB, beforeMigration, afterMigration, t.Helper() - cdb, cleanUp, err := MakeDB() - defer cleanUp() + cdb, err := MakeDB(t) if err != nil { t.Fatal(err) } From 5d5ad9ce6505c4a5f710dfaa40d8cba51feca328 Mon Sep 17 00:00:00 2001 From: Eng Zer Jun Date: Mon, 15 Aug 2022 21:05:33 +0800 Subject: [PATCH 06/23] cluster: use `T.TempDir` to create temporary test directory Signed-off-by: Eng Zer Jun --- cluster/etcd_elector_test.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/cluster/etcd_elector_test.go b/cluster/etcd_elector_test.go index c10137045..59cd25606 100644 --- a/cluster/etcd_elector_test.go +++ b/cluster/etcd_elector_test.go @@ -5,7 +5,6 @@ package cluster import ( "context" - "io/ioutil" "os" "runtime/pprof" "sync" @@ -41,8 +40,7 @@ func TestEtcdElector(t *testing.T) { guard := GuardTimeout(t, 5*time.Second) defer guard() - tmpDir, err := ioutil.TempDir("", "etcd") - require.NoError(t, err, "unable to create temp dir") + tmpDir := t.TempDir() etcdCfg, cleanup, err := etcd.NewEmbeddedEtcdInstance(tmpDir, 0, 0, "") require.NoError(t, err) From d1541d662853156b575b34b028cf383d196bc4c5 Mon Sep 17 00:00:00 2001 From: Eng Zer Jun Date: Mon, 15 Aug 2022 21:05:40 +0800 Subject: [PATCH 07/23] contractcourt: use `T.TempDir` to create temporary test directory Signed-off-by: Eng Zer Jun --- contractcourt/breacharbiter_test.go | 46 +++------------ contractcourt/briefcase_test.go | 74 +++++++++--------------- contractcourt/chain_arbitrator_test.go | 17 +----- contractcourt/chain_watcher_test.go | 33 ++++------- contractcourt/channel_arbitrator_test.go | 9 +-- contractcourt/utils_test.go | 33 ++++------- 6 files changed, 60 insertions(+), 152 deletions(-) diff --git a/contractcourt/breacharbiter_test.go b/contractcourt/breacharbiter_test.go index be2e27805..f50669cd4 100644 --- a/contractcourt/breacharbiter_test.go +++ b/contractcourt/breacharbiter_test.go @@ -10,10 +10,8 @@ import ( "encoding/binary" "fmt" "io" - "io/ioutil" "math/rand" "net" - "os" "reflect" "sync" "testing" @@ -639,25 +637,13 @@ func TestMockRetributionStore(t *testing.T) { } } -func makeTestChannelDB() (*channeldb.DB, func(), error) { - // First, create a temporary directory to be used for the duration of - // this test. - tempDirName, err := ioutil.TempDir("", "channeldb") +func makeTestChannelDB(t *testing.T) (*channeldb.DB, error) { + db, err := channeldb.Open(t.TempDir()) if err != nil { - return nil, nil, err + return nil, err } - cleanUp := func() { - os.RemoveAll(tempDirName) - } - - db, err := channeldb.Open(tempDirName) - if err != nil { - cleanUp() - return nil, nil, err - } - - return db, cleanUp, nil + return db, nil } // TestChannelDBRetributionStore instantiates a retributionStore backed by a @@ -670,12 +656,11 @@ func TestChannelDBRetributionStore(t *testing.T) { t.Run( "channeldbDBRetributionStore."+test.name, func(tt *testing.T) { - db, cleanUp, err := makeTestChannelDB() + db, err := makeTestChannelDB(t) if err != nil { t.Fatalf("unable to open channeldb: %v", err) } defer db.Close() - defer cleanUp() restartDb := func() RetributionStorer { // Close and reopen channeldb @@ -976,7 +961,7 @@ func initBreachedState(t *testing.T) (*BreachArbiter, // Create a pair of channels using a notifier that allows us to signal // a spend of the funding transaction. Alice's channel will be the on // observing a breach. - alice, bob, cleanUpChans, err := createInitChannels(1) + alice, bob, cleanUpChans, err := createInitChannels(t, 1) require.NoError(t, err, "unable to create test channels") // Instantiate a breach arbiter to handle the breach of alice's channel. @@ -2177,8 +2162,7 @@ func createTestArbiter(t *testing.T, contractBreaches chan *ContractBreachEvent, // createInitChannels creates two initialized test channels funded with 10 BTC, // with 5 BTC allocated to each side. Within the channel, Alice is the // initiator. -func createInitChannels(revocationWindow int) (*lnwallet.LightningChannel, *lnwallet.LightningChannel, func(), error) { - +func createInitChannels(t *testing.T, revocationWindow int) (*lnwallet.LightningChannel, *lnwallet.LightningChannel, func(), error) { aliceKeyPriv, aliceKeyPub := btcec.PrivKeyFromBytes( channels.AlicesPrivKey, ) @@ -2285,22 +2269,12 @@ func createInitChannels(revocationWindow int) (*lnwallet.LightningChannel, *lnwa return nil, nil, nil, err } - alicePath, err := ioutil.TempDir("", "alicedb") + dbAlice, err := channeldb.Open(t.TempDir()) if err != nil { return nil, nil, nil, err } - dbAlice, err := channeldb.Open(alicePath) - if err != nil { - return nil, nil, nil, err - } - - bobPath, err := ioutil.TempDir("", "bobdb") - if err != nil { - return nil, nil, nil, err - } - - dbBob, err := channeldb.Open(bobPath) + dbBob, err := channeldb.Open(t.TempDir()) if err != nil { return nil, nil, nil, err } @@ -2418,8 +2392,6 @@ func createInitChannels(revocationWindow int) (*lnwallet.LightningChannel, *lnwa cleanUpFunc := func() { dbBob.Close() dbAlice.Close() - os.RemoveAll(bobPath) - os.RemoveAll(alicePath) } // Now that the channel are open, simulate the start of a session by diff --git a/contractcourt/briefcase_test.go b/contractcourt/briefcase_test.go index 1c780443b..8bc0209f1 100644 --- a/contractcourt/briefcase_test.go +++ b/contractcourt/briefcase_test.go @@ -2,9 +2,7 @@ package contractcourt import ( "crypto/rand" - "io/ioutil" prand "math/rand" - "os" "reflect" "testing" "time" @@ -146,36 +144,28 @@ var ( } ) -func makeTestDB() (kvdb.Backend, func(), error) { - // First, create a temporary directory to be used for the duration of - // this test. - tempDirName, err := ioutil.TempDir("", "arblog") - if err != nil { - return nil, nil, err - } - +func makeTestDB(t *testing.T) (kvdb.Backend, error) { db, err := kvdb.Create( - kvdb.BoltBackendName, tempDirName+"/test.db", true, + kvdb.BoltBackendName, t.TempDir()+"/test.db", true, kvdb.DefaultDBTimeout, ) if err != nil { - return nil, nil, err + return nil, err } - cleanUp := func() { + t.Cleanup(func() { db.Close() - os.RemoveAll(tempDirName) - } + }) - return db, cleanUp, nil + return db, nil } -func newTestBoltArbLog(chainhash chainhash.Hash, - op wire.OutPoint) (ArbitratorLog, func(), error) { +func newTestBoltArbLog(t *testing.T, chainhash chainhash.Hash, + op wire.OutPoint) (ArbitratorLog, error) { - testDB, cleanUp, err := makeTestDB() + testDB, err := makeTestDB(t) if err != nil { - return nil, nil, err + return nil, err } testArbCfg := ChannelArbitratorConfig{ @@ -187,10 +177,10 @@ func newTestBoltArbLog(chainhash chainhash.Hash, } testLog, err := newBoltArbitratorLog(testDB, testArbCfg, chainhash, op) if err != nil { - return nil, nil, err + return nil, err } - return testLog, cleanUp, err + return testLog, err } func randOutPoint() wire.OutPoint { @@ -304,11 +294,10 @@ func TestContractInsertionRetrieval(t *testing.T) { // First, we'll create a test instance of the ArbitratorLog // implementation backed by boltdb. - testLog, cleanUp, err := newTestBoltArbLog( - testChainHash, testChanPoint1, + testLog, err := newTestBoltArbLog( + t, testChainHash, testChanPoint1, ) require.NoError(t, err, "unable to create test log") - defer cleanUp() // The log created, we'll create a series of resolvers, each properly // implementing the ContractResolver interface. @@ -432,11 +421,10 @@ func TestContractResolution(t *testing.T) { // First, we'll create a test instance of the ArbitratorLog // implementation backed by boltdb. - testLog, cleanUp, err := newTestBoltArbLog( - testChainHash, testChanPoint1, + testLog, err := newTestBoltArbLog( + t, testChainHash, testChanPoint1, ) require.NoError(t, err, "unable to create test log") - defer cleanUp() // We'll now create a timeout resolver that we'll be using for the // duration of this test. @@ -486,11 +474,10 @@ func TestContractSwapping(t *testing.T) { // First, we'll create a test instance of the ArbitratorLog // implementation backed by boltdb. - testLog, cleanUp, err := newTestBoltArbLog( - testChainHash, testChanPoint1, + testLog, err := newTestBoltArbLog( + t, testChainHash, testChanPoint1, ) require.NoError(t, err, "unable to create test log") - defer cleanUp() // We'll create two resolvers, a regular timeout resolver, and the // contest resolver that eventually turns into the timeout resolver. @@ -543,11 +530,10 @@ func TestContractResolutionsStorage(t *testing.T) { // First, we'll create a test instance of the ArbitratorLog // implementation backed by boltdb. - testLog, cleanUp, err := newTestBoltArbLog( - testChainHash, testChanPoint1, + testLog, err := newTestBoltArbLog( + t, testChainHash, testChanPoint1, ) require.NoError(t, err, "unable to create test log") - defer cleanUp() // With the test log created, we'll now craft a contact resolution that // will be using for the duration of this test. @@ -659,11 +645,10 @@ func TestContractResolutionsStorage(t *testing.T) { func TestStateMutation(t *testing.T) { t.Parallel() - testLog, cleanUp, err := newTestBoltArbLog( - testChainHash, testChanPoint1, + testLog, err := newTestBoltArbLog( + t, testChainHash, testChanPoint1, ) require.NoError(t, err, "unable to create test log") - defer cleanUp() // The default state of an arbitrator should be StateDefault. arbState, err := testLog.CurrentState(nil) @@ -707,17 +692,15 @@ func TestScopeIsolation(t *testing.T) { // We'll create two distinct test logs. Each log will have a unique // scope key, and therefore should be isolated from the other on disk. - testLog1, cleanUp1, err := newTestBoltArbLog( - testChainHash, testChanPoint1, + testLog1, err := newTestBoltArbLog( + t, testChainHash, testChanPoint1, ) require.NoError(t, err, "unable to create test log") - defer cleanUp1() - testLog2, cleanUp2, err := newTestBoltArbLog( - testChainHash, testChanPoint2, + testLog2, err := newTestBoltArbLog( + t, testChainHash, testChanPoint2, ) require.NoError(t, err, "unable to create test log") - defer cleanUp2() // We'll now update the current state of both the logs to a unique // state. @@ -754,11 +737,10 @@ func TestScopeIsolation(t *testing.T) { func TestCommitSetStorage(t *testing.T) { t.Parallel() - testLog, cleanUp, err := newTestBoltArbLog( - testChainHash, testChanPoint1, + testLog, err := newTestBoltArbLog( + t, testChainHash, testChanPoint1, ) require.NoError(t, err, "unable to create test log") - defer cleanUp() activeHTLCs := []channeldb.HTLC{ { diff --git a/contractcourt/chain_arbitrator_test.go b/contractcourt/chain_arbitrator_test.go index 9eb2b90fd..4b491c8e4 100644 --- a/contractcourt/chain_arbitrator_test.go +++ b/contractcourt/chain_arbitrator_test.go @@ -1,9 +1,7 @@ package contractcourt import ( - "io/ioutil" "net" - "os" "testing" "github.com/btcsuite/btcd/chaincfg/chainhash" @@ -22,13 +20,7 @@ import ( func TestChainArbitratorRepublishCloses(t *testing.T) { t.Parallel() - tempPath, err := ioutil.TempDir("", "testdb") - if err != nil { - t.Fatal(err) - } - defer os.RemoveAll(tempPath) - - db, err := channeldb.Open(tempPath) + db, err := channeldb.Open(t.TempDir()) if err != nil { t.Fatal(err) } @@ -143,12 +135,7 @@ func TestChainArbitratorRepublishCloses(t *testing.T) { func TestResolveContract(t *testing.T) { t.Parallel() - // To start with, we'll create a new temp DB for the duration of this - // test. - tempPath, err := ioutil.TempDir("", "testdb") - require.NoError(t, err, "unable to make temp dir") - defer os.RemoveAll(tempPath) - db, err := channeldb.Open(tempPath) + db, err := channeldb.Open(t.TempDir()) require.NoError(t, err, "unable to open db") defer db.Close() diff --git a/contractcourt/chain_watcher_test.go b/contractcourt/chain_watcher_test.go index 6e9b0acd6..1dc738274 100644 --- a/contractcourt/chain_watcher_test.go +++ b/contractcourt/chain_watcher_test.go @@ -200,27 +200,19 @@ type dlpTestCase struct { // state) are returned. func executeStateTransitions(t *testing.T, htlcAmount lnwire.MilliSatoshi, aliceChannel, bobChannel *lnwallet.LightningChannel, - numUpdates uint8) ([]*channeldb.OpenChannel, func(), error) { + numUpdates uint8) ([]*channeldb.OpenChannel, error) { // We'll make a copy of the channel state before each transition. var ( - chanStates []*channeldb.OpenChannel - cleanupFuncs []func() + chanStates []*channeldb.OpenChannel ) - cleanAll := func() { - for _, f := range cleanupFuncs { - f() - } - } - - state, f, err := copyChannelState(aliceChannel.State()) + state, err := copyChannelState(t, aliceChannel.State()) if err != nil { - return nil, nil, err + return nil, err } chanStates = append(chanStates, state) - cleanupFuncs = append(cleanupFuncs, f) for i := 0; i < int(numUpdates); i++ { addFakeHTLC( @@ -229,21 +221,18 @@ func executeStateTransitions(t *testing.T, htlcAmount lnwire.MilliSatoshi, err := lnwallet.ForceStateTransition(aliceChannel, bobChannel) if err != nil { - cleanAll() - return nil, nil, err + return nil, err } - state, f, err := copyChannelState(aliceChannel.State()) + state, err := copyChannelState(t, aliceChannel.State()) if err != nil { - cleanAll() - return nil, nil, err + return nil, err } chanStates = append(chanStates, state) - cleanupFuncs = append(cleanupFuncs, f) } - return chanStates, cleanAll, nil + return chanStates, nil } // TestChainWatcherDataLossProtect tests that if we've lost data (and are @@ -278,7 +267,7 @@ func TestChainWatcherDataLossProtect(t *testing.T) { // new HTLC to add to the commitment, and then lock in a state // transition. const htlcAmt = 1000 - states, cleanStates, err := executeStateTransitions( + states, err := executeStateTransitions( t, htlcAmt, aliceChannel, bobChannel, testCase.BroadcastStateNum, ) @@ -287,7 +276,6 @@ func TestChainWatcherDataLossProtect(t *testing.T) { "transition: %v", err) return false } - defer cleanStates() // We'll use the state this test case wants Alice to start at. aliceChanState := states[testCase.NumUpdates] @@ -455,7 +443,7 @@ func TestChainWatcherLocalForceCloseDetect(t *testing.T) { // get more coverage of various state hint encodings beyond 0 // and 1. const htlcAmt = 1000 - states, cleanStates, err := executeStateTransitions( + states, err := executeStateTransitions( t, htlcAmt, aliceChannel, bobChannel, numUpdates, ) if err != nil { @@ -463,7 +451,6 @@ func TestChainWatcherLocalForceCloseDetect(t *testing.T) { "transition: %v", err) return false } - defer cleanStates() // We'll use the state this test case wants Alice to start at. aliceChanState := states[localState] diff --git a/contractcourt/channel_arbitrator_test.go b/contractcourt/channel_arbitrator_test.go index 5f789f121..c8f7e6e2b 100644 --- a/contractcourt/channel_arbitrator_test.go +++ b/contractcourt/channel_arbitrator_test.go @@ -3,8 +3,6 @@ package contractcourt import ( "errors" "fmt" - "io/ioutil" - "os" "path/filepath" "reflect" "sort" @@ -406,11 +404,7 @@ func createTestChannelArbitrator(t *testing.T, log ArbitratorLog, var cleanUp func() if log == nil { - dbDir, err := ioutil.TempDir("", "chanArb") - if err != nil { - return nil, err - } - dbPath := filepath.Join(dbDir, "testdb") + dbPath := filepath.Join(t.TempDir(), "testdb") db, err := kvdb.Create( kvdb.BoltBackendName, dbPath, true, kvdb.DefaultDBTimeout, @@ -427,7 +421,6 @@ func createTestChannelArbitrator(t *testing.T, log ArbitratorLog, } cleanUp = func() { db.Close() - os.RemoveAll(dbDir) } log = &testArbLog{ diff --git a/contractcourt/utils_test.go b/contractcourt/utils_test.go index 0023402c1..13dfd45a3 100644 --- a/contractcourt/utils_test.go +++ b/contractcourt/utils_test.go @@ -3,7 +3,6 @@ package contractcourt import ( "fmt" "io" - "io/ioutil" "os" "path/filepath" "runtime/pprof" @@ -52,47 +51,35 @@ func copyFile(dest, src string) error { } // copyChannelState copies the OpenChannel state by copying the database and -// creating a new struct from it. The copied state and a cleanup function are -// returned. -func copyChannelState(state *channeldb.OpenChannel) ( - *channeldb.OpenChannel, func(), error) { +// creating a new struct from it. The copied state is returned. +func copyChannelState(t *testing.T, state *channeldb.OpenChannel) ( + *channeldb.OpenChannel, error) { // Make a copy of the DB. dbFile := filepath.Join(state.Db.GetParentDB().Path(), "channel.db") - tempDbPath, err := ioutil.TempDir("", "past-state") - if err != nil { - return nil, nil, err - } - - cleanup := func() { - os.RemoveAll(tempDbPath) - } + tempDbPath := t.TempDir() tempDbFile := filepath.Join(tempDbPath, "channel.db") - err = copyFile(tempDbFile, dbFile) + err := copyFile(tempDbFile, dbFile) if err != nil { - cleanup() - return nil, nil, err + return nil, err } newDb, err := channeldb.Open(tempDbPath) if err != nil { - cleanup() - return nil, nil, err + return nil, err } chans, err := newDb.ChannelStateDB().FetchAllChannels() if err != nil { - cleanup() - return nil, nil, err + return nil, err } // We only support DBs with a single channel, for now. if len(chans) != 1 { - cleanup() - return nil, nil, fmt.Errorf("found %d chans in the db", + return nil, fmt.Errorf("found %d chans in the db", len(chans)) } - return chans[0], cleanup, nil + return chans[0], nil } From dbf3cf62fea8cee324341ace14dbf30e7b3e73ee Mon Sep 17 00:00:00 2001 From: Eng Zer Jun Date: Mon, 15 Aug 2022 21:06:04 +0800 Subject: [PATCH 08/23] discovery: use `T.TempDir` to create temporary test directory Signed-off-by: Eng Zer Jun --- discovery/gossiper_test.go | 116 +++++++++++--------------------- discovery/message_store_test.go | 26 +++---- 2 files changed, 47 insertions(+), 95 deletions(-) diff --git a/discovery/gossiper_test.go b/discovery/gossiper_test.go index 8ddde84e3..a426620fe 100644 --- a/discovery/gossiper_test.go +++ b/discovery/gossiper_test.go @@ -4,10 +4,8 @@ import ( "bytes" "encoding/hex" "fmt" - "io/ioutil" prand "math/rand" "net" - "os" "reflect" "strings" "sync" @@ -72,29 +70,19 @@ var ( rebroadcastInterval = time.Hour * 1000000 ) -// makeTestDB creates a new instance of the ChannelDB for testing purposes. A -// callback which cleans up the created temporary directories is also returned -// and intended to be executed after the test completes. -func makeTestDB() (*channeldb.DB, func(), error) { - // First, create a temporary directory to be used for the duration of - // this test. - tempDirName, err := ioutil.TempDir("", "channeldb") +// makeTestDB creates a new instance of the ChannelDB for testing purposes. +func makeTestDB(t *testing.T) (*channeldb.DB, error) { + // Create channeldb for the first time. + cdb, err := channeldb.Open(t.TempDir()) if err != nil { - return nil, nil, err + return nil, err } - // Next, create channeldb for the first time. - cdb, err := channeldb.Open(tempDirName) - if err != nil { - return nil, nil, err - } - - cleanUp := func() { + t.Cleanup(func() { cdb.Close() - os.RemoveAll(tempDirName) - } + }) - return cdb, cleanUp, nil + return cdb, nil } type mockGraphSource struct { @@ -716,7 +704,7 @@ type testCtx struct { broadcastedMessage chan msgWithSenders } -func createTestCtx(startHeight uint32) (*testCtx, func(), error) { +func createTestCtx(t *testing.T, startHeight uint32) (*testCtx, error) { // Next we'll initialize an instance of the channel router with mock // versions of the chain and channel notifier. As we don't need to test // any p2p functionality, the peer send and switch send, @@ -724,15 +712,14 @@ func createTestCtx(startHeight uint32) (*testCtx, func(), error) { notifier := newMockNotifier() router := newMockRouter(startHeight) - db, cleanUpDb, err := makeTestDB() + db, err := makeTestDB(t) if err != nil { - return nil, nil, err + return nil, err } waitingProofStore, err := channeldb.NewWaitingProofStore(db) if err != nil { - cleanUpDb() - return nil, nil, err + return nil, err } broadcastedMessage := make(chan msgWithSenders, 10) @@ -808,25 +795,23 @@ func createTestCtx(startHeight uint32) (*testCtx, func(), error) { }, selfKeyDesc) if err := gossiper.Start(); err != nil { - cleanUpDb() - return nil, nil, fmt.Errorf("unable to start router: %v", err) + return nil, fmt.Errorf("unable to start router: %v", err) } // Mark the graph as synced in order to allow the announcements to be // broadcast. gossiper.syncMgr.markGraphSynced() - cleanUp := func() { + t.Cleanup(func() { gossiper.Stop() - cleanUpDb() - } + }) return &testCtx{ router: router, notifier: notifier, gossiper: gossiper, broadcastedMessage: broadcastedMessage, - }, cleanUp, nil + }, nil } // TestProcessAnnouncement checks that mature announcements are propagated to @@ -835,9 +820,8 @@ func TestProcessAnnouncement(t *testing.T) { t.Parallel() timestamp := testTimestamp - ctx, cleanup, err := createTestCtx(0) + ctx, err := createTestCtx(t, 0) require.NoError(t, err, "can't create context") - defer cleanup() assertSenderExistence := func(sender *btcec.PublicKey, msg msgWithSenders) { if _, ok := msg.senders[route.NewVertex(sender)]; !ok { @@ -929,9 +913,8 @@ func TestPrematureAnnouncement(t *testing.T) { timestamp := testTimestamp - ctx, cleanup, err := createTestCtx(0) + ctx, err := createTestCtx(t, 0) require.NoError(t, err, "can't create context") - defer cleanup() _, err = createNodeAnnouncement(remoteKeyPriv1, timestamp) require.NoError(t, err, "can't create node announcement") @@ -961,9 +944,8 @@ func TestPrematureAnnouncement(t *testing.T) { func TestSignatureAnnouncementLocalFirst(t *testing.T) { t.Parallel() - ctx, cleanup, err := createTestCtx(uint32(proofMatureDelta)) + ctx, err := createTestCtx(t, proofMatureDelta) require.NoError(t, err, "can't create context") - defer cleanup() // Set up a channel that we can use to inspect the messages sent // directly from the gossiper. @@ -1134,9 +1116,8 @@ func TestSignatureAnnouncementLocalFirst(t *testing.T) { func TestOrphanSignatureAnnouncement(t *testing.T) { t.Parallel() - ctx, cleanup, err := createTestCtx(uint32(proofMatureDelta)) + ctx, err := createTestCtx(t, proofMatureDelta) require.NoError(t, err, "can't create context") - defer cleanup() // Set up a channel that we can use to inspect the messages sent // directly from the gossiper. @@ -1318,9 +1299,8 @@ func TestOrphanSignatureAnnouncement(t *testing.T) { func TestSignatureAnnouncementRetryAtStartup(t *testing.T) { t.Parallel() - ctx, cleanup, err := createTestCtx(uint32(proofMatureDelta)) + ctx, err := createTestCtx(t, proofMatureDelta) require.NoError(t, err, "can't create context") - defer cleanup() batch, err := createLocalAnnouncements(0) require.NoError(t, err, "can't generate announcements") @@ -1550,9 +1530,8 @@ out: func TestSignatureAnnouncementFullProofWhenRemoteProof(t *testing.T) { t.Parallel() - ctx, cleanup, err := createTestCtx(uint32(proofMatureDelta)) + ctx, err := createTestCtx(t, proofMatureDelta) require.NoError(t, err, "can't create context") - defer cleanup() batch, err := createLocalAnnouncements(0) require.NoError(t, err, "can't generate announcements") @@ -1983,9 +1962,8 @@ func TestForwardPrivateNodeAnnouncement(t *testing.T) { timestamp = 123456 ) - ctx, cleanup, err := createTestCtx(startingHeight) + ctx, err := createTestCtx(t, startingHeight) require.NoError(t, err, "can't create context") - defer cleanup() // We'll start off by processing a channel announcement without a proof // (i.e., an unadvertised channel), followed by a node announcement for @@ -2083,9 +2061,8 @@ func TestRejectZombieEdge(t *testing.T) { // We'll start by creating our test context with a batch of // announcements. - ctx, cleanup, err := createTestCtx(0) + ctx, err := createTestCtx(t, 0) require.NoError(t, err, "unable to create test context") - defer cleanup() batch, err := createRemoteAnnouncements(0) require.NoError(t, err, "unable to create announcements") @@ -2185,9 +2162,8 @@ func TestProcessZombieEdgeNowLive(t *testing.T) { // We'll start by creating our test context with a batch of // announcements. - ctx, cleanup, err := createTestCtx(0) + ctx, err := createTestCtx(t, 0) require.NoError(t, err, "unable to create test context") - defer cleanup() batch, err := createRemoteAnnouncements(0) require.NoError(t, err, "unable to create announcements") @@ -2343,9 +2319,8 @@ func TestProcessZombieEdgeNowLive(t *testing.T) { func TestReceiveRemoteChannelUpdateFirst(t *testing.T) { t.Parallel() - ctx, cleanup, err := createTestCtx(uint32(proofMatureDelta)) + ctx, err := createTestCtx(t, proofMatureDelta) require.NoError(t, err, "can't create context") - defer cleanup() batch, err := createLocalAnnouncements(0) require.NoError(t, err, "can't generate announcements") @@ -2541,9 +2516,8 @@ func TestReceiveRemoteChannelUpdateFirst(t *testing.T) { func TestExtraDataChannelAnnouncementValidation(t *testing.T) { t.Parallel() - ctx, cleanup, err := createTestCtx(0) + ctx, err := createTestCtx(t, 0) require.NoError(t, err, "can't create context") - defer cleanup() remotePeer := &mockPeer{remoteKeyPriv1.PubKey(), nil, nil} @@ -2573,9 +2547,8 @@ func TestExtraDataChannelUpdateValidation(t *testing.T) { t.Parallel() timestamp := testTimestamp - ctx, cleanup, err := createTestCtx(0) + ctx, err := createTestCtx(t, 0) require.NoError(t, err, "can't create context") - defer cleanup() remotePeer := &mockPeer{remoteKeyPriv1.PubKey(), nil, nil} @@ -2625,9 +2598,8 @@ func TestExtraDataChannelUpdateValidation(t *testing.T) { func TestExtraDataNodeAnnouncementValidation(t *testing.T) { t.Parallel() - ctx, cleanup, err := createTestCtx(0) + ctx, err := createTestCtx(t, 0) require.NoError(t, err, "can't create context") - defer cleanup() remotePeer := &mockPeer{remoteKeyPriv1.PubKey(), nil, nil} timestamp := testTimestamp @@ -2694,9 +2666,8 @@ func assertProcessAnnouncement(t *testing.T, result chan error) { func TestRetransmit(t *testing.T) { t.Parallel() - ctx, cleanup, err := createTestCtx(proofMatureDelta) + ctx, err := createTestCtx(t, proofMatureDelta) require.NoError(t, err, "can't create context") - defer cleanup() batch, err := createLocalAnnouncements(0) require.NoError(t, err, "can't generate announcements") @@ -2801,9 +2772,8 @@ func TestRetransmit(t *testing.T) { func TestNodeAnnouncementNoChannels(t *testing.T) { t.Parallel() - ctx, cleanup, err := createTestCtx(0) + ctx, err := createTestCtx(t, 0) require.NoError(t, err, "can't create context") - defer cleanup() batch, err := createRemoteAnnouncements(0) require.NoError(t, err, "can't generate announcements") @@ -2887,9 +2857,8 @@ func TestNodeAnnouncementNoChannels(t *testing.T) { func TestOptionalFieldsChannelUpdateValidation(t *testing.T) { t.Parallel() - ctx, cleanup, err := createTestCtx(0) + ctx, err := createTestCtx(t, 0) require.NoError(t, err, "can't create context") - defer cleanup() chanUpdateHeight := uint32(0) timestamp := uint32(123456) @@ -2966,9 +2935,8 @@ func TestSendChannelUpdateReliably(t *testing.T) { // We'll start by creating our test context and a batch of // announcements. - ctx, cleanup, err := createTestCtx(uint32(proofMatureDelta)) + ctx, err := createTestCtx(t, proofMatureDelta) require.NoError(t, err, "unable to create test context") - defer cleanup() batch, err := createLocalAnnouncements(0) require.NoError(t, err, "can't generate announcements") @@ -3319,9 +3287,8 @@ func TestPropagateChanPolicyUpdate(t *testing.T) { // First, we'll make out test context and add 3 random channels to the // graph. startingHeight := uint32(10) - ctx, cleanup, err := createTestCtx(startingHeight) + ctx, err := createTestCtx(t, startingHeight) require.NoError(t, err, "unable to create test context") - defer cleanup() const numChannels = 3 channelsToAnnounce := make([]*annBatch, 0, numChannels) @@ -3497,9 +3464,8 @@ func TestProcessChannelAnnouncementOptionalMsgFields(t *testing.T) { // We'll start by creating our test context and a set of test channel // announcements. - ctx, cleanup, err := createTestCtx(0) + ctx, err := createTestCtx(t, 0) require.NoError(t, err, "unable to create test context") - defer cleanup() chanAnn1 := createAnnouncementWithoutProof( 100, selfKeyDesc.PubKey, remoteKeyPub1, @@ -3661,9 +3627,8 @@ func (m *SyncManager) markGraphSyncing() { func TestBroadcastAnnsAfterGraphSynced(t *testing.T) { t.Parallel() - ctx, cleanup, err := createTestCtx(10) + ctx, err := createTestCtx(t, 10) require.NoError(t, err, "can't create context") - defer cleanup() // We'll mark the graph as not synced. This should prevent us from // broadcasting any messages we've received as part of our initial @@ -3735,9 +3700,8 @@ func TestRateLimitChannelUpdates(t *testing.T) { // Create our test harness. const blockHeight = 100 - ctx, cleanup, err := createTestCtx(blockHeight) + ctx, err := createTestCtx(t, blockHeight) require.NoError(t, err, "can't create context") - defer cleanup() ctx.gossiper.cfg.RebroadcastInterval = time.Hour ctx.gossiper.cfg.MaxChannelUpdateBurst = 5 ctx.gossiper.cfg.ChannelUpdateInterval = 5 * time.Second @@ -3882,9 +3846,8 @@ func TestRateLimitChannelUpdates(t *testing.T) { func TestIgnoreOwnAnnouncement(t *testing.T) { t.Parallel() - ctx, cleanup, err := createTestCtx(proofMatureDelta) + ctx, err := createTestCtx(t, proofMatureDelta) require.NoError(t, err, "can't create context") - defer cleanup() batch, err := createLocalAnnouncements(0) require.NoError(t, err, "can't generate announcements") @@ -4027,9 +3990,8 @@ func TestIgnoreOwnAnnouncement(t *testing.T) { func TestRejectCacheChannelAnn(t *testing.T) { t.Parallel() - ctx, cleanup, err := createTestCtx(proofMatureDelta) + ctx, err := createTestCtx(t, proofMatureDelta) require.NoError(t, err, "can't create context") - defer cleanup() // First, we create a channel announcement to send over to our test // peer. diff --git a/discovery/message_store_test.go b/discovery/message_store_test.go index 4bb13c18c..e812c3f1a 100644 --- a/discovery/message_store_test.go +++ b/discovery/message_store_test.go @@ -2,9 +2,7 @@ package discovery import ( "bytes" - "io/ioutil" "math/rand" - "os" "reflect" "testing" @@ -16,29 +14,24 @@ import ( "github.com/stretchr/testify/require" ) -func createTestMessageStore(t *testing.T) (*MessageStore, func()) { +func createTestMessageStore(t *testing.T) *MessageStore { t.Helper() - tempDir, err := ioutil.TempDir("", "channeldb") - require.NoError(t, err, "unable to create temp dir") - db, err := channeldb.Open(tempDir) + db, err := channeldb.Open(t.TempDir()) if err != nil { - os.RemoveAll(tempDir) t.Fatalf("unable to open db: %v", err) } - cleanUp := func() { + t.Cleanup(func() { db.Close() - os.RemoveAll(tempDir) - } + }) store, err := NewMessageStore(db) if err != nil { - cleanUp() t.Fatalf("unable to initialize message store: %v", err) } - return store, cleanUp + return store } func randPubKey(t *testing.T) *btcec.PublicKey { @@ -79,8 +72,7 @@ func TestMessageStoreMessages(t *testing.T) { t.Parallel() // We'll start by creating our test message store. - msgStore, cleanUp := createTestMessageStore(t) - defer cleanUp() + msgStore := createTestMessageStore(t) // We'll then create some test messages for two test peers, and none for // an additional test peer. @@ -212,8 +204,7 @@ func TestMessageStoreUnsupportedMessage(t *testing.T) { t.Parallel() // We'll start by creating our test message store. - msgStore, cleanUp := createTestMessageStore(t) - defer cleanUp() + msgStore := createTestMessageStore(t) // Create a message that is known to not be supported by the store. peer := randCompressedPubKey(t) @@ -261,8 +252,7 @@ func TestMessageStoreUnsupportedMessage(t *testing.T) { func TestMessageStoreDeleteMessage(t *testing.T) { t.Parallel() - msgStore, cleanUp := createTestMessageStore(t) - defer cleanUp() + msgStore := createTestMessageStore(t) // assertMsg is a helper closure we'll use to ensure a message // does/doesn't exist within the store. From 4f1e4ff87a3733404132f4205e035c3e57be22f1 Mon Sep 17 00:00:00 2001 From: Eng Zer Jun Date: Mon, 15 Aug 2022 21:06:13 +0800 Subject: [PATCH 09/23] funding: use `T.TempDir` to create temporary test directory Signed-off-by: Eng Zer Jun --- funding/manager_test.go | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/funding/manager_test.go b/funding/manager_test.go index d10a0c766..48ce3a3e6 100644 --- a/funding/manager_test.go +++ b/funding/manager_test.go @@ -8,9 +8,7 @@ import ( "encoding/hex" "errors" "fmt" - "io/ioutil" "net" - "os" "path/filepath" "runtime" "strings" @@ -644,19 +642,13 @@ type cfgOption func(*Config) func setupFundingManagers(t *testing.T, options ...cfgOption) (*testNode, *testNode) { - aliceTestDir, err := ioutil.TempDir("", "alicelnwallet") - require.NoError(t, err, "unable to create temp directory") - alice, err := createTestFundingManager( - t, alicePrivKey, aliceAddr, aliceTestDir, options..., + t, alicePrivKey, aliceAddr, t.TempDir(), options..., ) require.NoError(t, err, "failed creating fundingManager") - bobTestDir, err := ioutil.TempDir("", "boblnwallet") - require.NoError(t, err, "unable to create temp directory") - bob, err := createTestFundingManager( - t, bobPrivKey, bobAddr, bobTestDir, options..., + t, bobPrivKey, bobAddr, t.TempDir(), options..., ) require.NoError(t, err, "failed creating fundingManager") @@ -699,8 +691,6 @@ func tearDownFundingManagers(t *testing.T, a, b *testNode) { if err := b.fundingMgr.Stop(); err != nil { t.Fatalf("failed stop funding manager: %v", err) } - os.RemoveAll(a.testDir) - os.RemoveAll(b.testDir) } // openChannel takes the funding process to the point where the funding From 9294b53fcc0350371f2a820c9dd07783c223194f Mon Sep 17 00:00:00 2001 From: Eng Zer Jun Date: Mon, 15 Aug 2022 21:06:21 +0800 Subject: [PATCH 10/23] htlcswitch: use `T.TempDir` to create temporary test directory Signed-off-by: Eng Zer Jun --- htlcswitch/circuit_test.go | 7 +--- htlcswitch/decayedlog_test.go | 60 ++++++++++++++--------------- htlcswitch/payment_result_test.go | 8 +--- htlcswitch/resolution_store_test.go | 14 ++----- htlcswitch/switch_test.go | 32 +++++---------- 5 files changed, 43 insertions(+), 78 deletions(-) diff --git a/htlcswitch/circuit_test.go b/htlcswitch/circuit_test.go index 256aa4aa7..8109d49b5 100644 --- a/htlcswitch/circuit_test.go +++ b/htlcswitch/circuit_test.go @@ -3,7 +3,6 @@ package htlcswitch_test import ( "bytes" "fmt" - "io/ioutil" "reflect" "testing" @@ -624,11 +623,7 @@ func equalIgnoreLFD(c, c2 *htlcswitch.PaymentCircuit) bool { // will be created. func makeCircuitDB(t *testing.T, path string) *channeldb.DB { if path == "" { - var err error - path, err = ioutil.TempDir("", "circuitdb") - if err != nil { - t.Fatalf("unable to create temp path: %v", err) - } + path = t.TempDir() } db, err := channeldb.Open(path) diff --git a/htlcswitch/decayedlog_test.go b/htlcswitch/decayedlog_test.go index 7dd90d3e6..44446abc1 100644 --- a/htlcswitch/decayedlog_test.go +++ b/htlcswitch/decayedlog_test.go @@ -3,8 +3,6 @@ package htlcswitch import ( "crypto/rand" "fmt" - "io/ioutil" - "os" "testing" "time" @@ -19,15 +17,6 @@ const ( cltv uint32 = 100000 ) -// tempDecayedLogPath creates a new temporary database path to back a single -// decayed log instance. -func tempDecayedLogPath(t *testing.T) string { - dir, err := ioutil.TempDir("", "decayedlog") - require.NoError(t, err, "unable to create temporary decayed log dir") - - return dir -} - // startup sets up the DecayedLog and possibly the garbage collector. func startup(dbPath string, notifier bool) (sphinx.ReplayLog, *mock.ChainNotifier, *sphinx.HashPrefix, func(), error) { @@ -82,24 +71,19 @@ func startup(dbPath string, notifier bool) (sphinx.ReplayLog, return log, chainNotifier, &hashedSecret, stop, nil } -// shutdown deletes the temporary directory that the test database uses -// and handles closing the database. -func shutdown(dir string, d sphinx.ReplayLog) { - d.Stop() - os.RemoveAll(dir) -} - // TestDecayedLogGarbageCollector tests the ability of the garbage collector // to delete expired cltv values every time a block is received. Expired cltv // values are cltv values that are < current block height. func TestDecayedLogGarbageCollector(t *testing.T) { t.Parallel() - dbPath := tempDecayedLogPath(t) + dbPath := t.TempDir() d, notifier, hashedSecret, _, err := startup(dbPath, true) require.NoError(t, err, "Unable to start up DecayedLog") - defer shutdown(dbPath, d) + t.Cleanup(func() { + require.NoError(t, d.Stop()) + }) // Store in the sharedHashBucket. err = d.Put(hashedSecret, cltv) @@ -150,11 +134,13 @@ func TestDecayedLogGarbageCollector(t *testing.T) { func TestDecayedLogPersistentGarbageCollector(t *testing.T) { t.Parallel() - dbPath := tempDecayedLogPath(t) + dbPath := t.TempDir() d, _, hashedSecret, stop, err := startup(dbPath, true) require.NoError(t, err, "Unable to start up DecayedLog") - defer shutdown(dbPath, d) + t.Cleanup(func() { + require.NoError(t, d.Stop()) + }) // Store in the sharedHashBucket if err = d.Put(hashedSecret, cltv); err != nil { @@ -172,7 +158,9 @@ func TestDecayedLogPersistentGarbageCollector(t *testing.T) { d2, notifier2, _, _, err := startup(dbPath, true) require.NoError(t, err, "Unable to restart DecayedLog") - defer shutdown(dbPath, d2) + t.Cleanup(func() { + require.NoError(t, d2.Stop()) + }) // Check that the hash prefix still exists in the new db instance. _, err = d2.Get(hashedSecret) @@ -202,11 +190,13 @@ func TestDecayedLogPersistentGarbageCollector(t *testing.T) { func TestDecayedLogInsertionAndDeletion(t *testing.T) { t.Parallel() - dbPath := tempDecayedLogPath(t) + dbPath := t.TempDir() d, _, hashedSecret, _, err := startup(dbPath, false) require.NoError(t, err, "Unable to start up DecayedLog") - defer shutdown(dbPath, d) + t.Cleanup(func() { + require.NoError(t, d.Stop()) + }) // Store in the sharedHashBucket. err = d.Put(hashedSecret, cltv) @@ -234,11 +224,13 @@ func TestDecayedLogInsertionAndDeletion(t *testing.T) { func TestDecayedLogStartAndStop(t *testing.T) { t.Parallel() - dbPath := tempDecayedLogPath(t) + dbPath := t.TempDir() d, _, hashedSecret, stop, err := startup(dbPath, false) require.NoError(t, err, "Unable to start up DecayedLog") - defer shutdown(dbPath, d) + t.Cleanup(func() { + require.NoError(t, d.Stop()) + }) // Store in the sharedHashBucket. err = d.Put(hashedSecret, cltv) @@ -249,7 +241,9 @@ func TestDecayedLogStartAndStop(t *testing.T) { d2, _, hashedSecret2, stop, err := startup(dbPath, false) require.NoError(t, err, "Unable to restart DecayedLog") - defer shutdown(dbPath, d2) + t.Cleanup(func() { + require.NoError(t, d2.Stop()) + }) // Retrieve the stored cltv value given the hashedSecret key. value, err := d2.Get(hashedSecret) @@ -270,7 +264,9 @@ func TestDecayedLogStartAndStop(t *testing.T) { d3, _, hashedSecret3, _, err := startup(dbPath, false) require.NoError(t, err, "Unable to restart DecayedLog") - defer shutdown(dbPath, d3) + t.Cleanup(func() { + require.NoError(t, d3.Stop()) + }) // Assert that hashedSecret is not in the sharedHashBucket _, err = d3.Get(hashedSecret3) @@ -288,11 +284,13 @@ func TestDecayedLogStartAndStop(t *testing.T) { func TestDecayedLogStorageAndRetrieval(t *testing.T) { t.Parallel() - dbPath := tempDecayedLogPath(t) + dbPath := t.TempDir() d, _, hashedSecret, _, err := startup(dbPath, false) require.NoError(t, err, "Unable to start up DecayedLog") - defer shutdown(dbPath, d) + t.Cleanup(func() { + require.NoError(t, d.Stop()) + }) // Store in the sharedHashBucket err = d.Put(hashedSecret, cltv) diff --git a/htlcswitch/payment_result_test.go b/htlcswitch/payment_result_test.go index 96d25bec4..33fc37a5c 100644 --- a/htlcswitch/payment_result_test.go +++ b/htlcswitch/payment_result_test.go @@ -2,7 +2,6 @@ package htlcswitch import ( "bytes" - "io/ioutil" "math/rand" "reflect" "testing" @@ -103,12 +102,7 @@ func TestNetworkResultStore(t *testing.T) { const numResults = 4 - tempDir, err := ioutil.TempDir("", "testdb") - if err != nil { - t.Fatal(err) - } - - db, err := channeldb.Open(tempDir) + db, err := channeldb.Open(t.TempDir()) if err != nil { t.Fatal(err) } diff --git a/htlcswitch/resolution_store_test.go b/htlcswitch/resolution_store_test.go index f58f39d85..122c699a1 100644 --- a/htlcswitch/resolution_store_test.go +++ b/htlcswitch/resolution_store_test.go @@ -1,8 +1,6 @@ package htlcswitch import ( - "io/ioutil" - "os" "path/filepath" "testing" @@ -40,20 +38,14 @@ func TestInsertAndDelete(t *testing.T) { // Create the backend database and use it to create the resolution // store. - dbDir, err := ioutil.TempDir("", "resolutionStore") - require.NoError(t, err) - - dbPath := filepath.Join(dbDir, "testdb") + dbPath := filepath.Join(t.TempDir(), "testdb") db, err := kvdb.Create( kvdb.BoltBackendName, dbPath, true, kvdb.DefaultDBTimeout, ) require.NoError(t, err) - - cleanUp := func() { + t.Cleanup(func() { db.Close() - os.RemoveAll(dbDir) - } - defer cleanUp() + }) resStore := newResolutionStore(db) diff --git a/htlcswitch/switch_test.go b/htlcswitch/switch_test.go index 335eb1bc2..0132ec521 100644 --- a/htlcswitch/switch_test.go +++ b/htlcswitch/switch_test.go @@ -6,9 +6,7 @@ import ( "crypto/sha256" "fmt" "io" - "io/ioutil" mrand "math/rand" - "os" "reflect" "testing" "time" @@ -997,8 +995,7 @@ func TestSwitchForwardFailAfterFullAdd(t *testing.T) { ) require.NoError(t, err, "unable to create bob server") - tempPath, err := ioutil.TempDir("", "circuitdb") - require.NoError(t, err, "unable to temporary path") + tempPath := t.TempDir() cdb, err := channeldb.Open(tempPath) require.NoError(t, err, "unable to open channeldb") @@ -1186,8 +1183,7 @@ func TestSwitchForwardSettleAfterFullAdd(t *testing.T) { ) require.NoError(t, err, "unable to create bob server") - tempPath, err := ioutil.TempDir("", "circuitdb") - require.NoError(t, err, "unable to temporary path") + tempPath := t.TempDir() cdb, err := channeldb.Open(tempPath) require.NoError(t, err, "unable to open channeldb") @@ -1378,8 +1374,7 @@ func TestSwitchForwardDropAfterFullAdd(t *testing.T) { ) require.NoError(t, err, "unable to create bob server") - tempPath, err := ioutil.TempDir("", "circuitdb") - require.NoError(t, err, "unable to temporary path") + tempPath := t.TempDir() cdb, err := channeldb.Open(tempPath) require.NoError(t, err, "unable to open channeldb") @@ -1533,8 +1528,7 @@ func TestSwitchForwardFailAfterHalfAdd(t *testing.T) { ) require.NoError(t, err, "unable to create bob server") - tempPath, err := ioutil.TempDir("", "circuitdb") - require.NoError(t, err, "unable to temporary path") + tempPath := t.TempDir() cdb, err := channeldb.Open(tempPath) require.NoError(t, err, "unable to open channeldb") @@ -1689,8 +1683,7 @@ func TestSwitchForwardCircuitPersistence(t *testing.T) { ) require.NoError(t, err, "unable to create bob server") - tempPath, err := ioutil.TempDir("", "circuitdb") - require.NoError(t, err, "unable to temporary path") + tempPath := t.TempDir() cdb, err := channeldb.Open(tempPath) require.NoError(t, err, "unable to open channeldb") @@ -3771,8 +3764,7 @@ func TestSwitchHoldForward(t *testing.T) { ) require.NoError(t, err, "unable to create bob server") - tempPath, err := ioutil.TempDir("", "circuitdb") - require.NoError(t, err, "unable to temporary path") + tempPath := t.TempDir() cdb, err := channeldb.Open(tempPath) require.NoError(t, err, "unable to open channeldb") @@ -4706,8 +4698,7 @@ func testSwitchForwardFailAlias(t *testing.T, zeroConf bool) { ) require.NoError(t, err) - tempPath, err := ioutil.TempDir("", "circuitdb") - require.NoError(t, err) + tempPath := t.TempDir() cdb, err := channeldb.Open(tempPath) require.NoError(t, err) @@ -4795,7 +4786,6 @@ func testSwitchForwardFailAlias(t *testing.T, zeroConf bool) { defer func() { _ = s2.Stop() - _ = os.RemoveAll(tempPath) }() var aliceLink2 *mockChannelLink @@ -4922,8 +4912,7 @@ func testSwitchAliasFailAdd(t *testing.T, zeroConf, private, useAlias bool) { ) require.NoError(t, err) - tempPath, err := ioutil.TempDir("", "circuitdb") - require.NoError(t, err) + tempPath := t.TempDir() cdb, err := channeldb.Open(tempPath) require.NoError(t, err) @@ -4939,7 +4928,6 @@ func testSwitchAliasFailAdd(t *testing.T, zeroConf, private, useAlias bool) { defer func() { _ = s.Stop() - _ = os.RemoveAll(tempPath) }() // Make Alice's channel zero-conf or option-scid-alias (feature bit). @@ -5263,8 +5251,7 @@ func testSwitchAliasInterceptFail(t *testing.T, zeroConf bool) { ) require.NoError(t, err) - tempPath, err := ioutil.TempDir("", "circuitdb") - require.NoError(t, err) + tempPath := t.TempDir() cdb, err := channeldb.Open(tempPath) require.NoError(t, err) @@ -5277,7 +5264,6 @@ func testSwitchAliasInterceptFail(t *testing.T, zeroConf bool) { defer func() { _ = s.Stop() - _ = os.RemoveAll(tempPath) }() // Make Alice's alias here. From 6c4ce69b2683d8ddb4b689b94eede6de71562164 Mon Sep 17 00:00:00 2001 From: Eng Zer Jun Date: Mon, 15 Aug 2022 21:06:46 +0800 Subject: [PATCH 11/23] invoices: use `T.TempDir` to create temporary test directory Signed-off-by: Eng Zer Jun --- invoices/invoiceregistry_test.go | 14 ++++---------- invoices/test_utils_test.go | 28 ++++++++-------------------- 2 files changed, 12 insertions(+), 30 deletions(-) diff --git a/invoices/invoiceregistry_test.go b/invoices/invoiceregistry_test.go index 0c2b0c2c5..7273a7c73 100644 --- a/invoices/invoiceregistry_test.go +++ b/invoices/invoiceregistry_test.go @@ -337,11 +337,10 @@ func TestCancelInvoice(t *testing.T) { func TestSettleHoldInvoice(t *testing.T) { defer timeout()() - cdb, cleanup, err := newTestChannelDB(clock.NewTestClock(time.Time{})) + cdb, err := newTestChannelDB(t, clock.NewTestClock(time.Time{})) if err != nil { t.Fatal(err) } - defer cleanup() // Instantiate and start the invoice ctx.registry. cfg := RegistryConfig{ @@ -510,11 +509,10 @@ func TestSettleHoldInvoice(t *testing.T) { func TestCancelHoldInvoice(t *testing.T) { defer timeout()() - cdb, cleanup, err := newTestChannelDB(clock.NewTestClock(time.Time{})) + cdb, err := newTestChannelDB(t, clock.NewTestClock(time.Time{})) if err != nil { t.Fatal(err) } - defer cleanup() // Instantiate and start the invoice ctx.registry. cfg := RegistryConfig{ @@ -935,9 +933,7 @@ func TestMppPayment(t *testing.T) { func TestInvoiceExpiryWithRegistry(t *testing.T) { t.Parallel() - cdb, cleanup, err := newTestChannelDB(clock.NewTestClock(time.Time{})) - defer cleanup() - + cdb, err := newTestChannelDB(t, clock.NewTestClock(time.Time{})) if err != nil { t.Fatal(err) } @@ -1043,9 +1039,7 @@ func TestOldInvoiceRemovalOnStart(t *testing.T) { t.Parallel() testClock := clock.NewTestClock(testTime) - cdb, cleanup, err := newTestChannelDB(testClock) - defer cleanup() - + cdb, err := newTestChannelDB(t, testClock) require.NoError(t, err) cfg := RegistryConfig{ diff --git a/invoices/test_utils_test.go b/invoices/test_utils_test.go index b1bdafeff..d158d2e9e 100644 --- a/invoices/test_utils_test.go +++ b/invoices/test_utils_test.go @@ -5,7 +5,6 @@ import ( "encoding/binary" "encoding/hex" "fmt" - "io/ioutil" "os" "runtime/pprof" "sync" @@ -162,29 +161,20 @@ var ( } ) -func newTestChannelDB(clock clock.Clock) (*channeldb.DB, func(), error) { - // First, create a temporary directory to be used for the duration of - // this test. - tempDirName, err := ioutil.TempDir("", "channeldb") - if err != nil { - return nil, nil, err - } - - // Next, create channeldb for the first time. +func newTestChannelDB(t *testing.T, clock clock.Clock) (*channeldb.DB, error) { + // Create channeldb for the first time. cdb, err := channeldb.Open( - tempDirName, channeldb.OptionClock(clock), + t.TempDir(), channeldb.OptionClock(clock), ) if err != nil { - os.RemoveAll(tempDirName) - return nil, nil, err + return nil, err } - cleanUp := func() { + t.Cleanup(func() { cdb.Close() - os.RemoveAll(tempDirName) - } + }) - return cdb, cleanUp, nil + return cdb, nil } type testContext struct { @@ -200,7 +190,7 @@ type testContext struct { func newTestContext(t *testing.T) *testContext { clock := clock.NewTestClock(testTime) - cdb, cleanup, err := newTestChannelDB(clock) + cdb, err := newTestChannelDB(t, clock) if err != nil { t.Fatal(err) } @@ -221,7 +211,6 @@ func newTestContext(t *testing.T) *testContext { err = registry.Start() if err != nil { - cleanup() t.Fatal(err) } @@ -235,7 +224,6 @@ func newTestContext(t *testing.T) *testContext { if err = registry.Stop(); err != nil { t.Fatalf("failed to stop invoice registry: %v", err) } - cleanup() }, } From d926ad1f8433d314df7ec481d182ab3f067c573f Mon Sep 17 00:00:00 2001 From: Eng Zer Jun Date: Mon, 15 Aug 2022 21:06:56 +0800 Subject: [PATCH 12/23] keychain: use `T.TempDir` to create temporary test directory Signed-off-by: Eng Zer Jun --- keychain/bench_test.go | 6 +-- keychain/interface_test.go | 89 +++++++++++++++----------------------- 2 files changed, 35 insertions(+), 60 deletions(-) diff --git a/keychain/bench_test.go b/keychain/bench_test.go index e6746fcbb..8b9e2f12a 100644 --- a/keychain/bench_test.go +++ b/keychain/bench_test.go @@ -8,15 +8,11 @@ import ( ) func BenchmarkDerivePrivKey(t *testing.B) { - cleanUp, wallet, err := createTestBtcWallet( - CoinTypeBitcoin, - ) + wallet, err := createTestBtcWallet(t, CoinTypeBitcoin) require.NoError(t, err, "unable to create wallet") keyRing := NewBtcWalletKeyRing(wallet, CoinTypeBitcoin) - defer cleanUp() - var ( privKey *btcec.PrivateKey ) diff --git a/keychain/interface_test.go b/keychain/interface_test.go index 106cf700d..b190f6a78 100644 --- a/keychain/interface_test.go +++ b/keychain/interface_test.go @@ -2,9 +2,7 @@ package keychain import ( "fmt" - "io/ioutil" "math/rand" - "os" "testing" "time" @@ -32,7 +30,7 @@ var ( testDBTimeout = time.Second * 10 ) -func createTestBtcWallet(coinType uint32) (func(), *wallet.Wallet, error) { +func createTestBtcWallet(t testing.TB, coinType uint32) (*wallet.Wallet, error) { // Instruct waddrmgr to use the cranked down scrypt parameters when // creating new wallet encryption keys. fastScrypt := waddrmgr.FastScryptOptions @@ -46,12 +44,8 @@ func createTestBtcWallet(coinType uint32) (func(), *wallet.Wallet, error) { waddrmgr.SetSecretKeyGen(keyGen) // Create a new test wallet that uses fast scrypt as KDF. - tempDir, err := ioutil.TempDir("", "keyring-lnwallet") - if err != nil { - return nil, nil, err - } loader := wallet.NewLoader( - &chaincfg.SimNetParams, tempDir, true, testDBTimeout, 0, + &chaincfg.SimNetParams, t.TempDir(), true, testDBTimeout, 0, ) pass := []byte("test") @@ -60,11 +54,11 @@ func createTestBtcWallet(coinType uint32) (func(), *wallet.Wallet, error) { pass, pass, testHDSeed[:], time.Time{}, ) if err != nil { - return nil, nil, err + return nil, err } if err := baseWallet.Unlock(pass, nil); err != nil { - return nil, nil, err + return nil, err } // Construct the key scope required to derive keys for the chose @@ -88,16 +82,15 @@ func createTestBtcWallet(coinType uint32) (func(), *wallet.Wallet, error) { return err }) if err != nil { - return nil, nil, err + return nil, err } } - cleanUp := func() { + t.Cleanup(func() { baseWallet.Lock() - os.RemoveAll(tempDir) - } + }) - return cleanUp, baseWallet, nil + return baseWallet, nil } func assertEqualKeyLocator(t *testing.T, a, b KeyLocator) { @@ -110,9 +103,9 @@ func assertEqualKeyLocator(t *testing.T, a, b KeyLocator) { // secretKeyRingConstructor is a function signature that's used as a generic // constructor for various implementations of the KeyRing interface. A string -// naming the returned interface, a function closure that cleans up any -// resources, and the clean up interface itself are to be returned. -type keyRingConstructor func() (string, func(), KeyRing, error) +// naming the returned interface, and the KeyRing interface itself are to be +// returned. +type keyRingConstructor func() (string, KeyRing, error) // TestKeyRingDerivation tests that each known KeyRing implementation properly // adheres to the expected behavior of the set of interfaces. @@ -120,35 +113,29 @@ func TestKeyRingDerivation(t *testing.T) { t.Parallel() keyRingImplementations := []keyRingConstructor{ - func() (string, func(), KeyRing, error) { - cleanUp, wallet, err := createTestBtcWallet( - CoinTypeBitcoin, - ) + func() (string, KeyRing, error) { + wallet, err := createTestBtcWallet(t, CoinTypeBitcoin) require.NoError(t, err) keyRing := NewBtcWalletKeyRing(wallet, CoinTypeBitcoin) - return "btcwallet", cleanUp, keyRing, nil + return "btcwallet", keyRing, nil }, - func() (string, func(), KeyRing, error) { - cleanUp, wallet, err := createTestBtcWallet( - CoinTypeLitecoin, - ) + func() (string, KeyRing, error) { + wallet, err := createTestBtcWallet(t, CoinTypeLitecoin) require.NoError(t, err) keyRing := NewBtcWalletKeyRing(wallet, CoinTypeLitecoin) - return "ltcwallet", cleanUp, keyRing, nil + return "ltcwallet", keyRing, nil }, - func() (string, func(), KeyRing, error) { - cleanUp, wallet, err := createTestBtcWallet( - CoinTypeTestnet, - ) + func() (string, KeyRing, error) { + wallet, err := createTestBtcWallet(t, CoinTypeTestnet) require.NoError(t, err) keyRing := NewBtcWalletKeyRing(wallet, CoinTypeTestnet) - return "testwallet", cleanUp, keyRing, nil + return "testwallet", keyRing, nil }, } @@ -158,12 +145,11 @@ func TestKeyRingDerivation(t *testing.T) { // an identical set of tests in order to ensure that the interface // adheres to our nominal specification. for _, keyRingConstructor := range keyRingImplementations { - keyRingName, cleanUp, keyRing, err := keyRingConstructor() + keyRingName, keyRing, err := keyRingConstructor() if err != nil { t.Fatalf("unable to create key ring %v: %v", keyRingName, err) } - defer cleanUp() success := t.Run(fmt.Sprintf("%v", keyRingName), func(t *testing.T) { // First, we'll ensure that we're able to derive keys @@ -244,9 +230,9 @@ func TestKeyRingDerivation(t *testing.T) { // secretKeyRingConstructor is a function signature that's used as a generic // constructor for various implementations of the SecretKeyRing interface. A -// string naming the returned interface, a function closure that cleans up any -// resources, and the clean up interface itself are to be returned. -type secretKeyRingConstructor func() (string, func(), SecretKeyRing, error) +// string naming the returned interface, and the SecretKeyRing interface itself +// are to be returned. +type secretKeyRingConstructor func() (string, SecretKeyRing, error) // TestSecretKeyRingDerivation tests that each known SecretKeyRing // implementation properly adheres to the expected behavior of the set of @@ -255,35 +241,29 @@ func TestSecretKeyRingDerivation(t *testing.T) { t.Parallel() secretKeyRingImplementations := []secretKeyRingConstructor{ - func() (string, func(), SecretKeyRing, error) { - cleanUp, wallet, err := createTestBtcWallet( - CoinTypeBitcoin, - ) + func() (string, SecretKeyRing, error) { + wallet, err := createTestBtcWallet(t, CoinTypeBitcoin) require.NoError(t, err) keyRing := NewBtcWalletKeyRing(wallet, CoinTypeBitcoin) - return "btcwallet", cleanUp, keyRing, nil + return "btcwallet", keyRing, nil }, - func() (string, func(), SecretKeyRing, error) { - cleanUp, wallet, err := createTestBtcWallet( - CoinTypeLitecoin, - ) + func() (string, SecretKeyRing, error) { + wallet, err := createTestBtcWallet(t, CoinTypeLitecoin) require.NoError(t, err) keyRing := NewBtcWalletKeyRing(wallet, CoinTypeLitecoin) - return "ltcwallet", cleanUp, keyRing, nil + return "ltcwallet", keyRing, nil }, - func() (string, func(), SecretKeyRing, error) { - cleanUp, wallet, err := createTestBtcWallet( - CoinTypeTestnet, - ) + func() (string, SecretKeyRing, error) { + wallet, err := createTestBtcWallet(t, CoinTypeTestnet) require.NoError(t, err) keyRing := NewBtcWalletKeyRing(wallet, CoinTypeTestnet) - return "testwallet", cleanUp, keyRing, nil + return "testwallet", keyRing, nil }, } @@ -291,12 +271,11 @@ func TestSecretKeyRingDerivation(t *testing.T) { // an identical set of tests in order to ensure that the interface // adheres to our nominal specification. for _, secretKeyRingConstructor := range secretKeyRingImplementations { - keyRingName, cleanUp, secretKeyRing, err := secretKeyRingConstructor() + keyRingName, secretKeyRing, err := secretKeyRingConstructor() if err != nil { t.Fatalf("unable to create secret key ring %v: %v", keyRingName, err) } - defer cleanUp() success := t.Run(fmt.Sprintf("%v", keyRingName), func(t *testing.T) { // For, each key family, we'll ensure that we're able From 0edc16aa01f28632b7f69f2aa4c3c0779a461643 Mon Sep 17 00:00:00 2001 From: Eng Zer Jun Date: Mon, 15 Aug 2022 21:07:05 +0800 Subject: [PATCH 13/23] kvdb: use `T.TempDir` to create temporary test directory Signed-off-by: Eng Zer Jun --- kvdb/bolt_fixture.go | 11 +---------- kvdb/bolt_test.go | 1 - kvdb/etcd/fixture.go | 19 +++++-------------- 3 files changed, 6 insertions(+), 25 deletions(-) diff --git a/kvdb/bolt_fixture.go b/kvdb/bolt_fixture.go index d78e1ece3..0ac54b55f 100644 --- a/kvdb/bolt_fixture.go +++ b/kvdb/bolt_fixture.go @@ -1,8 +1,6 @@ package kvdb import ( - "io/ioutil" - "os" "path/filepath" "testing" @@ -16,19 +14,12 @@ type boltFixture struct { } func NewBoltFixture(t *testing.T) *boltFixture { - tempDir, err := ioutil.TempDir("", "test") - require.NoError(t, err) - return &boltFixture{ t: t, - tempDir: tempDir, + tempDir: t.TempDir(), } } -func (b *boltFixture) Cleanup() { - os.RemoveAll(b.tempDir) -} - func (b *boltFixture) NewBackend() walletdb.DB { dbPath := filepath.Join(b.tempDir) diff --git a/kvdb/bolt_test.go b/kvdb/bolt_test.go index a6cec252b..69366c5b5 100644 --- a/kvdb/bolt_test.go +++ b/kvdb/bolt_test.go @@ -76,7 +76,6 @@ func TestBolt(t *testing.T) { t.Parallel() f := NewBoltFixture(t) - defer f.Cleanup() test.test(t, f.NewBackend()) }) diff --git a/kvdb/etcd/fixture.go b/kvdb/etcd/fixture.go index 4dde37cc9..8f0e0af7f 100644 --- a/kvdb/etcd/fixture.go +++ b/kvdb/etcd/fixture.go @@ -5,8 +5,6 @@ package etcd import ( "context" - "io/ioutil" - "os" "testing" "time" @@ -46,10 +44,7 @@ func NewTestEtcdInstance(t *testing.T, path string) (*Config, func()) { // NewTestEtcdTestFixture creates a new etcd-test fixture. This is helper // object to facilitate etcd tests and ensure pre and post conditions. func NewEtcdTestFixture(t *testing.T) *EtcdTestFixture { - tmpDir, err := ioutil.TempDir("", "etcd") - if err != nil { - t.Fatalf("unable to create temp dir: %v", err) - } + tmpDir := t.TempDir() config, etcdCleanup := NewTestEtcdInstance(t, tmpDir) @@ -59,7 +54,6 @@ func NewEtcdTestFixture(t *testing.T) *EtcdTestFixture { Password: config.Pass, }) if err != nil { - os.RemoveAll(tmpDir) t.Fatalf("unable to create etcd test fixture: %v", err) } @@ -69,13 +63,10 @@ func NewEtcdTestFixture(t *testing.T) *EtcdTestFixture { cli.Lease = namespace.NewLease(cli.Lease, defaultNamespace) return &EtcdTestFixture{ - t: t, - cli: cli, - config: config, - cleanup: func() { - etcdCleanup() - os.RemoveAll(tmpDir) - }, + t: t, + cli: cli, + config: config, + cleanup: etcdCleanup, } } From ca5c6955260ae79b87bd5f30e0097fb6adaa45a4 Mon Sep 17 00:00:00 2001 From: Eng Zer Jun Date: Mon, 15 Aug 2022 21:07:17 +0800 Subject: [PATCH 14/23] lntest/itest: use `T.TempDir` to create temporary test directory Signed-off-by: Eng Zer Jun --- lntest/itest/lnd_channel_backup_test.go | 6 +----- lntest/itest/lnd_etcd_failover_test.go | 7 +------ 2 files changed, 2 insertions(+), 11 deletions(-) diff --git a/lntest/itest/lnd_channel_backup_test.go b/lntest/itest/lnd_channel_backup_test.go index 2ebabaeb7..9895d9dbb 100644 --- a/lntest/itest/lnd_channel_backup_test.go +++ b/lntest/itest/lnd_channel_backup_test.go @@ -569,11 +569,7 @@ func testChannelBackupUpdates(net *lntest.NetworkHarness, t *harnessTest) { // First, we'll make a temp directory that we'll use to store our // backup file, so we can check in on it during the test easily. - backupDir, err := ioutil.TempDir("", "") - if err != nil { - t.Fatalf("unable to create backup dir: %v", err) - } - defer os.RemoveAll(backupDir) + backupDir := t.t.TempDir() // First, we'll create a new node, Carol. We'll also create a temporary // file that Carol will use to store her channel backups. diff --git a/lntest/itest/lnd_etcd_failover_test.go b/lntest/itest/lnd_etcd_failover_test.go index 3be233303..4de2d4490 100644 --- a/lntest/itest/lnd_etcd_failover_test.go +++ b/lntest/itest/lnd_etcd_failover_test.go @@ -5,7 +5,6 @@ package itest import ( "context" - "io/ioutil" "testing" "time" @@ -68,12 +67,8 @@ func testEtcdFailoverCase(net *lntest.NetworkHarness, ht *harnessTest, ctxb := context.Background() - tmpDir, err := ioutil.TempDir("", "etcd") - if err != nil { - ht.Fatalf("Failed to create temp dir: %v", err) - } etcdCfg, cleanup, err := kvdb.StartEtcdTestBackend( - tmpDir, uint16(lntest.NextAvailablePort()), + ht.t.TempDir(), uint16(lntest.NextAvailablePort()), uint16(lntest.NextAvailablePort()), "", ) if err != nil { From 9acd53a5deab208e25b6bbb60f335ed093872158 Mon Sep 17 00:00:00 2001 From: Eng Zer Jun Date: Mon, 15 Aug 2022 21:07:28 +0800 Subject: [PATCH 15/23] lnwallet: use `T.TempDir` to create temporary test directory Signed-off-by: Eng Zer Jun --- lnwallet/btcwallet/psbt_test.go | 3 +-- lnwallet/btcwallet/signer_test.go | 27 ++++++--------------------- lnwallet/test/test_interface.go | 28 +++++++--------------------- lnwallet/transactions_test.go | 25 +++++++------------------ 4 files changed, 21 insertions(+), 62 deletions(-) diff --git a/lnwallet/btcwallet/psbt_test.go b/lnwallet/btcwallet/psbt_test.go index 6bb657d8b..245ea682c 100644 --- a/lnwallet/btcwallet/psbt_test.go +++ b/lnwallet/btcwallet/psbt_test.go @@ -252,8 +252,7 @@ func serializeTxWitness(txWitness wire.TxWitness) ([]byte, error) { // TestSignPsbt tests the PSBT signing functionality. func TestSignPsbt(t *testing.T) { - w, _, cleanup := newTestWallet(t, netParams, seedBytes) - defer cleanup() + w, _ := newTestWallet(t, netParams, seedBytes) testCases := []struct { name string diff --git a/lnwallet/btcwallet/signer_test.go b/lnwallet/btcwallet/signer_test.go index 4b36accc7..25e62b4df 100644 --- a/lnwallet/btcwallet/signer_test.go +++ b/lnwallet/btcwallet/signer_test.go @@ -3,9 +3,7 @@ package btcwallet import ( "encoding/hex" "fmt" - "io/ioutil" "math" - "os" "testing" "time" @@ -164,8 +162,7 @@ var ( // BIP32 key path correctly. func TestBip32KeyDerivation(t *testing.T) { netParams := &chaincfg.RegressionNetParams - w, _, cleanup := newTestWallet(t, netParams, seedBytes) - defer cleanup() + w, _ := newTestWallet(t, netParams, seedBytes) // This is just a sanity check that the wallet was initialized // correctly. We make sure the first derived address is the expected @@ -203,8 +200,7 @@ func TestBip32KeyDerivation(t *testing.T) { // with a proof to make sure the resulting addresses match up. func TestScriptImport(t *testing.T) { netParams := &chaincfg.RegressionNetParams - w, miner, cleanup := newTestWallet(t, netParams, seedBytes) - defer cleanup() + w, miner := newTestWallet(t, netParams, seedBytes) firstDerivedAddr, err := w.NewAddress( lnwallet.TaprootPubkey, false, lnwallet.DefaultAccountName, @@ -286,21 +282,12 @@ func TestScriptImport(t *testing.T) { } func newTestWallet(t *testing.T, netParams *chaincfg.Params, - seedBytes []byte) (*BtcWallet, *rpctest.Harness, func()) { - - tempDir, err := ioutil.TempDir("", "lnwallet") - if err != nil { - _ = os.RemoveAll(tempDir) - t.Fatalf("creating temp dir failed: %v", err) - } + seedBytes []byte) (*BtcWallet, *rpctest.Harness) { chainBackend, miner, backendCleanup := getChainBackend(t, netParams) - cleanup := func() { - _ = os.RemoveAll(tempDir) - backendCleanup() - } + t.Cleanup(backendCleanup) - loaderOpt := LoaderWithLocalWalletDB(tempDir, false, time.Minute) + loaderOpt := LoaderWithLocalWalletDB(t.TempDir(), false, time.Minute) config := Config{ PrivatePass: []byte("some-pass"), HdSeed: seedBytes, @@ -314,17 +301,15 @@ func newTestWallet(t *testing.T, netParams *chaincfg.Params, blockCache := blockcache.NewBlockCache(10000) w, err := New(config, blockCache) if err != nil { - cleanup() t.Fatalf("creating wallet failed: %v", err) } err = w.Start() if err != nil { - cleanup() t.Fatalf("starting wallet failed: %v", err) } - return w, miner, cleanup + return w, miner } // getChainBackend returns a simple btcd based chain backend to back the wallet. diff --git a/lnwallet/test/test_interface.go b/lnwallet/test/test_interface.go index ebaf91e8d..88a9a3501 100644 --- a/lnwallet/test/test_interface.go +++ b/lnwallet/test/test_interface.go @@ -5,9 +5,7 @@ import ( "crypto/sha256" "encoding/hex" "fmt" - "io/ioutil" "net" - "os" "os/exec" "path/filepath" "reflect" @@ -3040,8 +3038,7 @@ func TestLightningWallet(t *testing.T, targetBackEnd string) { rpcConfig := miningNode.RPCConfig() - tempDir, err := ioutil.TempDir("", "channeldb") - require.NoError(t, err, "unable to create temp dir") + tempDir := t.TempDir() db, err := channeldb.Open(tempDir) require.NoError(t, err, "unable to create db") testCfg := chainntnfs.CacheConfig{ @@ -3093,15 +3090,12 @@ func runTests(t *testing.T, walletDriver *lnwallet.WalletDriver, aliceWalletController lnwallet.WalletController bobWalletController lnwallet.WalletController + + err error ) - tempTestDirAlice, err := ioutil.TempDir("", "lnwallet") - require.NoError(t, err, "unable to create temp directory") - defer os.RemoveAll(tempTestDirAlice) - - tempTestDirBob, err := ioutil.TempDir("", "lnwallet") - require.NoError(t, err, "unable to create temp directory") - defer os.RemoveAll(tempTestDirBob) + tempTestDirAlice := t.TempDir() + tempTestDirBob := t.TempDir() blockCache := blockcache.NewBlockCache(10000) @@ -3191,13 +3185,9 @@ func runTests(t *testing.T, walletDriver *lnwallet.WalletDriver, case "bitcoind": // Start a bitcoind instance. - tempBitcoindDir, err := ioutil.TempDir("", "bitcoind") - if err != nil { - t.Fatalf("unable to create temp directory: %v", err) - } + tempBitcoindDir := t.TempDir() zmqBlockHost := "ipc:///" + tempBitcoindDir + "/blocks.socket" zmqTxHost := "ipc:///" + tempBitcoindDir + "/tx.socket" - defer os.RemoveAll(tempBitcoindDir) rpcPort := getFreePort() bitcoind := exec.Command( "bitcoind", @@ -3269,11 +3259,7 @@ func runTests(t *testing.T, walletDriver *lnwallet.WalletDriver, case "bitcoind-rpc-polling": // Start a bitcoind instance. - tempBitcoindDir, err := ioutil.TempDir("", "bitcoind") - if err != nil { - t.Fatalf("unable to create temp directory: %v", err) - } - defer os.RemoveAll(tempBitcoindDir) + tempBitcoindDir := t.TempDir() rpcPort := getFreePort() bitcoind := exec.Command( "bitcoind", diff --git a/lnwallet/transactions_test.go b/lnwallet/transactions_test.go index 2bb14573c..b525d8a59 100644 --- a/lnwallet/transactions_test.go +++ b/lnwallet/transactions_test.go @@ -10,7 +10,6 @@ import ( "io" "io/ioutil" "net" - "os" "sort" "testing" "time" @@ -269,13 +268,12 @@ func testVectors(t *testing.T, chanType channeldb.ChannelType, test testCase) { // Set up a test channel on which the test commitment transaction is // going to be produced. - remoteChannel, localChannel, cleanUp := createTestChannelsForVectors( + remoteChannel, localChannel := createTestChannelsForVectors( tc, chanType, test.FeePerKw, remoteBalance.ToSatoshis(), localBalance.ToSatoshis(), ) - defer cleanUp() // Add htlcs (if any) to the update logs of both sides and save a hash // map that allows us to identify the htlcs in the scripts later on and @@ -747,7 +745,7 @@ func (p *mockProducer) Encode(w io.Writer) error { // test channel that is used to verify the test vectors. func createTestChannelsForVectors(tc *testContext, chanType channeldb.ChannelType, feeRate btcutil.Amount, remoteBalance, localBalance btcutil.Amount) ( - *LightningChannel, *LightningChannel, func()) { + *LightningChannel, *LightningChannel) { t := tc.t @@ -846,16 +844,10 @@ func createTestChannelsForVectors(tc *testContext, chanType channeldb.ChannelTyp ) // Create temporary databases. - remotePath, err := ioutil.TempDir("", "remotedb") + dbRemote, err := channeldb.Open(t.TempDir()) require.NoError(t, err) - dbRemote, err := channeldb.Open(remotePath) - require.NoError(t, err) - - localPath, err := ioutil.TempDir("", "localdb") - require.NoError(t, err) - - dbLocal, err := channeldb.Open(localPath) + dbLocal, err := channeldb.Open(t.TempDir()) require.NoError(t, err) // Create the initial commitment transactions for the channel. @@ -1005,16 +997,13 @@ func createTestChannelsForVectors(tc *testContext, chanType channeldb.ChannelTyp // Return a clean up function that stops goroutines and removes the test // databases. - cleanUpFunc := func() { + t.Cleanup(func() { dbLocal.Close() dbRemote.Close() - os.RemoveAll(localPath) - os.RemoveAll(remotePath) - require.NoError(t, remotePool.Stop()) require.NoError(t, localPool.Stop()) - } + }) - return channelRemote, channelLocal, cleanUpFunc + return channelRemote, channelLocal } From f3dec8fbb01e48c00bb1dc383896d0788cfb7d21 Mon Sep 17 00:00:00 2001 From: Eng Zer Jun Date: Mon, 15 Aug 2022 21:07:45 +0800 Subject: [PATCH 16/23] macarooons: use `T.TempDir` to create temporary test directory Signed-off-by: Eng Zer Jun --- macaroons/service_test.go | 22 +++++++-------------- macaroons/store_test.go | 41 +++++++++++++-------------------------- 2 files changed, 21 insertions(+), 42 deletions(-) diff --git a/macaroons/service_test.go b/macaroons/service_test.go index 18bcf00ea..d84aa6e25 100644 --- a/macaroons/service_test.go +++ b/macaroons/service_test.go @@ -3,8 +3,6 @@ package macaroons_test import ( "context" "encoding/hex" - "io/ioutil" - "os" "path" "testing" @@ -33,11 +31,9 @@ var ( // default password of 'hello'. Only the path to the temporary // DB file is returned, because the service will open the file // and read the store on its own. -func setupTestRootKeyStorage(t *testing.T) (string, kvdb.Backend) { - tempDir, err := ioutil.TempDir("", "macaroonstore-") - require.NoError(t, err, "Error creating temp dir") +func setupTestRootKeyStorage(t *testing.T) kvdb.Backend { db, err := kvdb.Create( - kvdb.BoltBackendName, path.Join(tempDir, "macaroons.db"), true, + kvdb.BoltBackendName, path.Join(t.TempDir(), "macaroons.db"), true, kvdb.DefaultDBTimeout, ) require.NoError(t, err, "Error opening store DB") @@ -49,15 +45,14 @@ func setupTestRootKeyStorage(t *testing.T) (string, kvdb.Backend) { defer store.Close() err = store.CreateUnlock(&defaultPw) require.NoError(t, err, "error creating unlock") - return tempDir, db + return db } // TestNewService tests the creation of the macaroon service. func TestNewService(t *testing.T) { // First, initialize a dummy DB file with a store that the service // can read from. Make sure the file is removed in the end. - tempDir, db := setupTestRootKeyStorage(t) - defer os.RemoveAll(tempDir) + db := setupTestRootKeyStorage(t) rootKeyStore, err := macaroons.NewRootKeyStorage(db) require.NoError(t, err) @@ -107,8 +102,7 @@ func TestNewService(t *testing.T) { // incoming context. func TestValidateMacaroon(t *testing.T) { // First, initialize the service and unlock it. - tempDir, db := setupTestRootKeyStorage(t) - defer os.RemoveAll(tempDir) + db := setupTestRootKeyStorage(t) rootKeyStore, err := macaroons.NewRootKeyStorage(db) require.NoError(t, err) service, err := macaroons.NewService( @@ -154,8 +148,7 @@ func TestValidateMacaroon(t *testing.T) { func TestListMacaroonIDs(t *testing.T) { // First, initialize a dummy DB file with a store that the service // can read from. Make sure the file is removed in the end. - tempDir, db := setupTestRootKeyStorage(t) - defer os.RemoveAll(tempDir) + db := setupTestRootKeyStorage(t) // Second, create the new service instance, unlock it and pass in a // checker that we expect it to add to the bakery. @@ -188,8 +181,7 @@ func TestDeleteMacaroonID(t *testing.T) { // First, initialize a dummy DB file with a store that the service // can read from. Make sure the file is removed in the end. - tempDir, db := setupTestRootKeyStorage(t) - defer os.RemoveAll(tempDir) + db := setupTestRootKeyStorage(t) // Second, create the new service instance, unlock it and pass in a // checker that we expect it to add to the bakery. diff --git a/macaroons/store_test.go b/macaroons/store_test.go index ff7a5d454..a9a49edda 100644 --- a/macaroons/store_test.go +++ b/macaroons/store_test.go @@ -3,8 +3,6 @@ package macaroons_test import ( "context" "crypto/rand" - "io/ioutil" - "os" "path" "testing" @@ -22,24 +20,17 @@ var ( // newTestStore creates a new bolt DB in a temporary directory and then // initializes a root key storage for that DB. -func newTestStore(t *testing.T) (string, func(), *macaroons.RootKeyStorage) { - tempDir, err := ioutil.TempDir("", "macaroonstore-") - require.NoError(t, err) +func newTestStore(t *testing.T) (string, *macaroons.RootKeyStorage) { + tempDir := t.TempDir() - cleanup, store := openTestStore(t, tempDir) - cleanup2 := func() { - cleanup() - _ = os.RemoveAll(tempDir) - } + store := openTestStore(t, tempDir) - return tempDir, cleanup2, store + return tempDir, store } // openTestStore opens an existing bolt DB and then initializes a root key // storage for that DB. -func openTestStore(t *testing.T, tempDir string) (func(), - *macaroons.RootKeyStorage) { - +func openTestStore(t *testing.T, tempDir string) *macaroons.RootKeyStorage { db, err := kvdb.Create( kvdb.BoltBackendName, path.Join(tempDir, "weks.db"), true, kvdb.DefaultDBTimeout, @@ -52,19 +43,18 @@ func openTestStore(t *testing.T, tempDir string) (func(), t.Fatalf("Error creating root key store: %v", err) } - cleanup := func() { + t.Cleanup(func() { _ = store.Close() _ = db.Close() - } + }) - return cleanup, store + return store } // TestStore tests the normal use cases of the store like creating, unlocking, // reading keys and closing it. func TestStore(t *testing.T) { - tempDir, cleanup, store := newTestStore(t) - defer cleanup() + tempDir, store := newTestStore(t) _, _, err := store.RootKey(context.TODO()) require.Equal(t, macaroons.ErrStoreLocked, err) @@ -114,7 +104,7 @@ func TestStore(t *testing.T) { // Between here and the re-opening of the store, it's possible to get // a double-close, but that's not such a big deal since the tests will // fail anyway in that case. - _, store = openTestStore(t, tempDir) + store = openTestStore(t, tempDir) err = store.CreateUnlock(&badpw) require.Equal(t, snacl.ErrInvalidPassword, err) @@ -144,8 +134,7 @@ func TestStore(t *testing.T) { // TestStoreGenerateNewRootKey tests that a root key can be replaced with a new // one in the store without changing the password. func TestStoreGenerateNewRootKey(t *testing.T) { - _, cleanup, store := newTestStore(t) - defer cleanup() + _, store := newTestStore(t) // The store must be unlocked to replace the root key. err := store.GenerateNewRootKey() @@ -172,8 +161,7 @@ func TestStoreGenerateNewRootKey(t *testing.T) { // TestStoreSetRootKey tests that a root key can be set to a specified value. func TestStoreSetRootKey(t *testing.T) { - _, cleanup, store := newTestStore(t) - defer cleanup() + _, store := newTestStore(t) // Create a new random key rootKey := make([]byte, 32) @@ -209,8 +197,7 @@ func TestStoreSetRootKey(t *testing.T) { // TestStoreChangePassword tests that the password for the store can be changed // without changing the root key. func TestStoreChangePassword(t *testing.T) { - tempDir, cleanup, store := newTestStore(t) - defer cleanup() + tempDir, store := newTestStore(t) // The store must be unlocked to replace the root key. err := store.ChangePassword(nil, nil) @@ -251,7 +238,7 @@ func TestStoreChangePassword(t *testing.T) { require.Error(t, err) // Let's open it again and try unlocking with the new password. - _, store = openTestStore(t, tempDir) + store = openTestStore(t, tempDir) err = store.CreateUnlock(&newPw) require.NoError(t, err) From 71185d46fb614830ff4dd0a98cee922be537c671 Mon Sep 17 00:00:00 2001 From: Eng Zer Jun Date: Mon, 15 Aug 2022 21:08:10 +0800 Subject: [PATCH 17/23] peer: use `T.TempDir` to create temporary test directory Signed-off-by: Eng Zer Jun --- peer/brontide_test.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/peer/brontide_test.go b/peer/brontide_test.go index 38ef2e5e7..a0a354c1c 100644 --- a/peer/brontide_test.go +++ b/peer/brontide_test.go @@ -2,7 +2,6 @@ package peer import ( "bytes" - "io/ioutil" "testing" "time" @@ -1003,10 +1002,7 @@ func TestPeerCustomMessage(t *testing.T) { t.Parallel() // Set up node Alice. - alicePath, err := ioutil.TempDir("", "alicedb") - require.NoError(t, err) - - dbAlice, err := channeldb.Open(alicePath) + dbAlice, err := channeldb.Open(t.TempDir()) require.NoError(t, err) aliceKey, err := btcec.NewPrivateKey() From b5a626be783e2a65e87073742a5a46bb5f5952c3 Mon Sep 17 00:00:00 2001 From: Eng Zer Jun Date: Mon, 15 Aug 2022 21:08:16 +0800 Subject: [PATCH 18/23] routing: use `T.TempDir` to create temporary test directory Signed-off-by: Eng Zer Jun --- routing/chainview/interface_test.go | 129 ++++++++------------- routing/control_tower_test.go | 16 +-- routing/integrated_routing_context_test.go | 18 ++- routing/missioncontrol_store_test.go | 12 +- routing/missioncontrol_test.go | 17 ++- routing/pathfind_test.go | 88 ++++---------- routing/payment_lifecycle_test.go | 3 +- routing/router_test.go | 31 ++--- 8 files changed, 117 insertions(+), 197 deletions(-) diff --git a/routing/chainview/interface_test.go b/routing/chainview/interface_test.go index 2096298cb..010f33807 100644 --- a/routing/chainview/interface_test.go +++ b/routing/chainview/interface_test.go @@ -3,9 +3,7 @@ package chainview import ( "bytes" "fmt" - "io/ioutil" "net" - "os" "os/exec" "path/filepath" "runtime" @@ -504,15 +502,10 @@ func testFilterBlockDisconnected(node *rpctest.Harness, require.NoError(t, err, "error getting best block") // Init a chain view that has this node as its block source. - cleanUpFunc, reorgView, err := chainViewInit( - reorgNode.RPCConfig(), reorgNode.P2PAddress(), bestHeight, + reorgView, err := chainViewInit( + t, reorgNode.RPCConfig(), reorgNode.P2PAddress(), bestHeight, ) require.NoError(t, err, "unable to create chain view") - defer func() { - if cleanUpFunc != nil { - cleanUpFunc() - } - }() if err = reorgView.Start(); err != nil { t.Fatalf("unable to start btcd chain view: %v", err) @@ -681,8 +674,8 @@ func testFilterBlockDisconnected(node *rpctest.Harness, time.Sleep(time.Millisecond * 500) } -type chainViewInitFunc func(rpcInfo rpcclient.ConnConfig, - p2pAddr string, bestHeight int32) (func(), FilteredChainView, error) +type chainViewInitFunc func(t *testing.T, rpcInfo rpcclient.ConnConfig, + p2pAddr string, bestHeight int32) (FilteredChainView, error) type testCase struct { name string @@ -715,20 +708,14 @@ var interfaceImpls = []struct { }{ { name: "bitcoind_zmq", - chainViewInit: func(_ rpcclient.ConnConfig, - p2pAddr string, bestHeight int32) (func(), - FilteredChainView, error) { + chainViewInit: func(t *testing.T, _ rpcclient.ConnConfig, + p2pAddr string, bestHeight int32) (FilteredChainView, error) { // Start a bitcoind instance. - tempBitcoindDir, err := ioutil.TempDir("", "bitcoind") - if err != nil { - return nil, nil, err - } + tempBitcoindDir := t.TempDir() zmqBlockHost := "ipc:///" + tempBitcoindDir + "/blocks.socket" zmqTxHost := "ipc:///" + tempBitcoindDir + "/tx.socket" - cleanUp1 := func() { - os.RemoveAll(tempBitcoindDir) - } + rpcPort := getFreePort() bitcoind := exec.Command( "bitcoind", @@ -744,25 +731,22 @@ var interfaceImpls = []struct { "-zmqpubrawblock="+zmqBlockHost, "-zmqpubrawtx="+zmqTxHost, ) - err = bitcoind.Start() + err := bitcoind.Start() if err != nil { - cleanUp1() - return nil, nil, err + return nil, err } // Sanity check to ensure that the process did in fact // start. if bitcoind.Process == nil { - cleanUp1() - return nil, nil, fmt.Errorf("bitcoind cmd " + + return nil, fmt.Errorf("bitcoind cmd " + "Process is not set after Start") } - cleanUp2 := func() { + t.Cleanup(func() { _ = bitcoind.Process.Kill() _ = bitcoind.Wait() - cleanUp1() - } + }) host := fmt.Sprintf("127.0.0.1:%d", rpcPort) cfg := &chain.BitcoindConfig{ @@ -806,14 +790,13 @@ var interfaceImpls = []struct { return nil }, 10*time.Second) if err != nil { - return cleanUp2, nil, fmt.Errorf("unable to "+ + return nil, fmt.Errorf("unable to "+ "establish connection to bitcoind: %v", err) } - cleanUp3 := func() { + t.Cleanup(func() { chainConn.Stop() - cleanUp2() - } + }) blockCache := blockcache.NewBlockCache(10000) @@ -821,23 +804,17 @@ var interfaceImpls = []struct { chainConn, blockCache, ) - return cleanUp3, chainView, nil + return chainView, nil }, }, { name: "bitcoind_polling", - chainViewInit: func(_ rpcclient.ConnConfig, - p2pAddr string, bestHeight int32) (func(), - FilteredChainView, error) { + chainViewInit: func(t *testing.T, _ rpcclient.ConnConfig, + p2pAddr string, bestHeight int32) (FilteredChainView, error) { // Start a bitcoind instance. - tempBitcoindDir, err := ioutil.TempDir("", "bitcoind") - if err != nil { - return nil, nil, err - } - cleanUp1 := func() { - os.RemoveAll(tempBitcoindDir) - } + tempBitcoindDir := t.TempDir() + rpcPort := getFreePort() bitcoind := exec.Command( "bitcoind", @@ -851,25 +828,22 @@ var interfaceImpls = []struct { fmt.Sprintf("-rpcport=%d", rpcPort), "-disablewallet", ) - err = bitcoind.Start() + err := bitcoind.Start() if err != nil { - cleanUp1() - return nil, nil, err + return nil, err } // Sanity check to ensure that the process did in fact // start. if bitcoind.Process == nil { - cleanUp1() - return nil, nil, fmt.Errorf("bitcoind cmd " + + return nil, fmt.Errorf("bitcoind cmd " + "Process is not set after Start") } - cleanUp2 := func() { + t.Cleanup(func() { _ = bitcoind.Process.Kill() _ = bitcoind.Wait() - cleanUp1() - } + }) host := fmt.Sprintf("127.0.0.1:%d", rpcPort) cfg := &chain.BitcoindConfig{ @@ -913,14 +887,13 @@ var interfaceImpls = []struct { return nil }, 10*time.Second) if err != nil { - return cleanUp2, nil, fmt.Errorf("unable to "+ + return nil, fmt.Errorf("unable to "+ "establish connection to bitcoind: %v", err) } - cleanUp3 := func() { + t.Cleanup(func() { chainConn.Stop() - cleanUp2() - } + }) blockCache := blockcache.NewBlockCache(10000) @@ -928,26 +901,22 @@ var interfaceImpls = []struct { chainConn, blockCache, ) - return cleanUp3, chainView, nil + return chainView, nil }, }, { name: "p2p_neutrino", - chainViewInit: func(_ rpcclient.ConnConfig, - p2pAddr string, bestHeight int32) (func(), - FilteredChainView, error) { + chainViewInit: func(t *testing.T, _ rpcclient.ConnConfig, + p2pAddr string, bestHeight int32) (FilteredChainView, error) { - spvDir, err := ioutil.TempDir("", "neutrino") - if err != nil { - return nil, nil, err - } + spvDir := t.TempDir() dbName := filepath.Join(spvDir, "neutrino.db") spvDatabase, err := walletdb.Create( "bdb", dbName, true, kvdb.DefaultDBTimeout, ) if err != nil { - return nil, nil, err + return nil, err } spvConfig := neutrino.Config{ @@ -959,7 +928,7 @@ var interfaceImpls = []struct { spvNode, err := neutrino.NewChainService(spvConfig) if err != nil { - return nil, nil, err + return nil, err } // Wait until the node has fully synced up to the local @@ -982,16 +951,15 @@ var interfaceImpls = []struct { return nil }, 10*time.Second) if err != nil { - return nil, nil, fmt.Errorf("unable to "+ + return nil, fmt.Errorf("unable to "+ "establish connection to bitcoind: %v", err) } - cleanUp := func() { + t.Cleanup(func() { spvDatabase.Close() spvNode.Stop() - os.RemoveAll(spvDir) - } + }) blockCache := blockcache.NewBlockCache(10000) @@ -999,27 +967,26 @@ var interfaceImpls = []struct { spvNode, blockCache, ) if err != nil { - return nil, nil, err + return nil, err } - return cleanUp, chainView, nil + return chainView, nil }, }, { name: "btcd_websockets", - chainViewInit: func(config rpcclient.ConnConfig, - p2pAddr string, bestHeight int32) (func(), - FilteredChainView, error) { + chainViewInit: func(_ *testing.T, config rpcclient.ConnConfig, + p2pAddr string, bestHeight int32) (FilteredChainView, error) { blockCache := blockcache.NewBlockCache(10000) chainView, err := NewBtcdFilteredChainView( config, blockCache, ) if err != nil { - return nil, nil, err + return nil, err } - return nil, chainView, err + return chainView, err }, }, } @@ -1048,8 +1015,8 @@ func TestFilteredChainView(t *testing.T) { t.Fatalf("error getting best block: %v", err) } - cleanUpFunc, chainView, err := chainViewImpl.chainViewInit( - rpcConfig, p2pAddr, bestHeight, + chainView, err := chainViewImpl.chainViewInit( + t, rpcConfig, p2pAddr, bestHeight, ) if err != nil { t.Fatalf("unable to make chain view: %v", err) @@ -1075,9 +1042,5 @@ func TestFilteredChainView(t *testing.T) { if err := chainView.Stop(); err != nil { t.Fatalf("unable to stop chain view: %v", err) } - - if cleanUpFunc != nil { - cleanUpFunc() - } } } diff --git a/routing/control_tower_test.go b/routing/control_tower_test.go index 267e60505..80a8fbc5c 100644 --- a/routing/control_tower_test.go +++ b/routing/control_tower_test.go @@ -5,7 +5,6 @@ import ( "crypto/sha256" "fmt" "io" - "io/ioutil" "reflect" "testing" "time" @@ -48,7 +47,7 @@ var ( func TestControlTowerSubscribeUnknown(t *testing.T) { t.Parallel() - db, err := initDB(false) + db, err := initDB(t, false) require.NoError(t, err, "unable to init db") pControl := NewControlTower(channeldb.NewPaymentControl(db)) @@ -65,7 +64,7 @@ func TestControlTowerSubscribeUnknown(t *testing.T) { func TestControlTowerSubscribeSuccess(t *testing.T) { t.Parallel() - db, err := initDB(false) + db, err := initDB(t, false) require.NoError(t, err, "unable to init db") pControl := NewControlTower(channeldb.NewPaymentControl(db)) @@ -182,7 +181,7 @@ func TestPaymentControlSubscribeFail(t *testing.T) { func testPaymentControlSubscribeFail(t *testing.T, registerAttempt, keepFailedPaymentAttempts bool) { - db, err := initDB(keepFailedPaymentAttempts) + db, err := initDB(t, keepFailedPaymentAttempts) require.NoError(t, err, "unable to init db") pControl := NewControlTower(channeldb.NewPaymentControl(db)) @@ -294,14 +293,9 @@ func testPaymentControlSubscribeFail(t *testing.T, registerAttempt, } } -func initDB(keepFailedPaymentAttempts bool) (*channeldb.DB, error) { - tempPath, err := ioutil.TempDir("", "routingdb") - if err != nil { - return nil, err - } - +func initDB(t *testing.T, keepFailedPaymentAttempts bool) (*channeldb.DB, error) { db, err := channeldb.Open( - tempPath, channeldb.OptionKeepFailedPaymentAttempts( + t.TempDir(), channeldb.OptionKeepFailedPaymentAttempts( keepFailedPaymentAttempts, ), ) diff --git a/routing/integrated_routing_context_test.go b/routing/integrated_routing_context_test.go index bbca1975a..d3baa3b41 100644 --- a/routing/integrated_routing_context_test.go +++ b/routing/integrated_routing_context_test.go @@ -2,7 +2,6 @@ package routing import ( "fmt" - "io/ioutil" "math" "os" "testing" @@ -122,13 +121,20 @@ func (c *integratedRoutingContext) testPayment(maxParts uint32, ) // Create temporary database for mission control. - file, err := ioutil.TempFile("", "*.db") + file, err := os.CreateTemp("", "*.db") if err != nil { c.t.Fatal(err) } dbPath := file.Name() - defer os.Remove(dbPath) + c.t.Cleanup(func() { + if err := file.Close(); err != nil { + c.t.Fatal(err) + } + if err := os.Remove(dbPath); err != nil { + c.t.Fatal(err) + } + }) db, err := kvdb.Open( kvdb.BoltBackendName, dbPath, true, kvdb.DefaultDBTimeout, @@ -136,7 +142,11 @@ func (c *integratedRoutingContext) testPayment(maxParts uint32, if err != nil { c.t.Fatal(err) } - defer db.Close() + c.t.Cleanup(func() { + if err := db.Close(); err != nil { + c.t.Fatal(err) + } + }) // Instantiate a new mission control with the current configuration // values. diff --git a/routing/missioncontrol_store_test.go b/routing/missioncontrol_store_test.go index f1e3a45f0..81d1da4b3 100644 --- a/routing/missioncontrol_store_test.go +++ b/routing/missioncontrol_store_test.go @@ -1,7 +1,6 @@ package routing import ( - "io/ioutil" "os" "reflect" "testing" @@ -24,12 +23,16 @@ func TestMissionControlStore(t *testing.T) { // Set time zone explicitly to keep test deterministic. time.Local = time.UTC - file, err := ioutil.TempFile("", "*.db") + file, err := os.CreateTemp("", "*.db") if err != nil { t.Fatal(err) } dbPath := file.Name() + t.Cleanup(func() { + require.NoError(t, file.Close()) + require.NoError(t, os.Remove(dbPath)) + }) db, err := kvdb.Create( kvdb.BoltBackendName, dbPath, true, kvdb.DefaultDBTimeout, @@ -37,8 +40,9 @@ func TestMissionControlStore(t *testing.T) { if err != nil { t.Fatal(err) } - defer db.Close() - defer os.Remove(dbPath) + t.Cleanup(func() { + require.NoError(t, db.Close()) + }) store, err := newMissionControlStore(db, testMaxRecords, time.Second) if err != nil { diff --git a/routing/missioncontrol_test.go b/routing/missioncontrol_test.go index 2e79ffb0f..dac2cfebc 100644 --- a/routing/missioncontrol_test.go +++ b/routing/missioncontrol_test.go @@ -1,7 +1,6 @@ package routing import ( - "io/ioutil" "os" "testing" "time" @@ -57,10 +56,14 @@ func createMcTestContext(t *testing.T) *mcTestContext { now: mcTestTime, } - file, err := ioutil.TempFile("", "*.db") + file, err := os.CreateTemp("", "*.db") if err != nil { t.Fatal(err) } + t.Cleanup(func() { + require.NoError(t, file.Close()) + require.NoError(t, os.Remove(file.Name())) + }) ctx.dbPath = file.Name() @@ -70,6 +73,9 @@ func createMcTestContext(t *testing.T) *mcTestContext { if err != nil { t.Fatal(err) } + t.Cleanup(func() { + require.NoError(t, ctx.db.Close()) + }) ctx.restartMc() @@ -102,12 +108,6 @@ func (ctx *mcTestContext) restartMc() { ctx.mc = mc } -// cleanup closes the database and removes the temp file. -func (ctx *mcTestContext) cleanup() { - ctx.db.Close() - os.Remove(ctx.dbPath) -} - // Assert that mission control returns a probability for an edge. func (ctx *mcTestContext) expectP(amt lnwire.MilliSatoshi, expected float64) { ctx.t.Helper() @@ -143,7 +143,6 @@ func (ctx *mcTestContext) reportSuccess() { // TestMissionControl tests mission control probability estimation. func TestMissionControl(t *testing.T) { ctx := createMcTestContext(t) - defer ctx.cleanup() ctx.now = testTime diff --git a/routing/pathfind_test.go b/routing/pathfind_test.go index 5e1bb2f62..7f576196a 100644 --- a/routing/pathfind_test.go +++ b/routing/pathfind_test.go @@ -11,7 +11,6 @@ import ( "io/ioutil" "math" "net" - "os" "reflect" "strings" "testing" @@ -150,28 +149,17 @@ type testChan struct { } // makeTestGraph creates a new instance of a channeldb.ChannelGraph for testing -// purposes. A callback which cleans up the created temporary directories is -// also returned and intended to be executed after the test completes. -func makeTestGraph(useCache bool) (*channeldb.ChannelGraph, kvdb.Backend, - func(), error) { +// purposes. +func makeTestGraph(t *testing.T, useCache bool) (*channeldb.ChannelGraph, + kvdb.Backend, error) { - // First, create a temporary directory to be used for the duration of - // this test. - tempDirName, err := ioutil.TempDir("", "channeldb") + // Create channelgraph for the first time. + backend, backendCleanup, err := kvdb.GetTestBackend(t.TempDir(), "cgr") if err != nil { - return nil, nil, nil, err + return nil, nil, err } - // Next, create channelgraph for the first time. - backend, backendCleanup, err := kvdb.GetTestBackend(tempDirName, "cgr") - if err != nil { - return nil, nil, nil, err - } - - cleanUp := func() { - backendCleanup() - _ = os.RemoveAll(tempDirName) - } + t.Cleanup(backendCleanup) opts := channeldb.DefaultOptions() graph, err := channeldb.NewChannelGraph( @@ -180,16 +168,17 @@ func makeTestGraph(useCache bool) (*channeldb.ChannelGraph, kvdb.Backend, useCache, false, ) if err != nil { - cleanUp() - return nil, nil, nil, err + return nil, nil, err } - return graph, backend, cleanUp, nil + return graph, backend, nil } // parseTestGraph returns a fully populated ChannelGraph given a path to a JSON // file which encodes a test graph. -func parseTestGraph(useCache bool, path string) (*testGraphInstance, error) { +func parseTestGraph(t *testing.T, useCache bool, path string) ( + *testGraphInstance, error) { + graphJSON, err := ioutil.ReadFile(path) if err != nil { return nil, err @@ -214,7 +203,7 @@ func parseTestGraph(useCache bool, path string) (*testGraphInstance, error) { testAddrs = append(testAddrs, testAddr) // Next, create a temporary graph database for usage within the test. - graph, graphBackend, cleanUp, err := makeTestGraph(useCache) + graph, graphBackend, err := makeTestGraph(t, useCache) if err != nil { return nil, err } @@ -429,7 +418,6 @@ func parseTestGraph(useCache bool, path string) (*testGraphInstance, error) { return &testGraphInstance{ graph: graph, graphBackend: graphBackend, - cleanUp: cleanUp, aliasMap: aliasMap, privKeyMap: privKeyMap, channelIDs: channelIDs, @@ -497,7 +485,6 @@ type testChannel struct { type testGraphInstance struct { graph *channeldb.ChannelGraph graphBackend kvdb.Backend - cleanUp func() // aliasMap is a map from a node's alias to its public key. This type is // provided in order to allow easily look up from the human memorable alias @@ -533,8 +520,8 @@ func (g *testGraphInstance) getLink(chanID lnwire.ShortChannelID) ( // a deterministical way and added to the channel graph. A list of nodes is // not required and derived from the channel data. The goal is to keep // instantiating a test channel graph as light weight as possible. -func createTestGraphFromChannels(useCache bool, testChannels []*testChannel, - source string) (*testGraphInstance, error) { +func createTestGraphFromChannels(t *testing.T, useCache bool, + testChannels []*testChannel, source string) (*testGraphInstance, error) { // We'll use this fake address for the IP address of all the nodes in // our tests. This value isn't needed for path finding so it doesn't @@ -547,7 +534,7 @@ func createTestGraphFromChannels(useCache bool, testChannels []*testChannel, testAddrs = append(testAddrs, testAddr) // Next, create a temporary graph database for usage within the test. - graph, graphBackend, cleanUp, err := makeTestGraph(useCache) + graph, graphBackend, err := makeTestGraph(t, useCache) if err != nil { return nil, err } @@ -765,7 +752,6 @@ func createTestGraphFromChannels(useCache bool, testChannels []*testChannel, return &testGraphInstance{ graph: graph, graphBackend: graphBackend, - cleanUp: cleanUp, aliasMap: aliasMap, privKeyMap: privKeyMap, links: links, @@ -883,7 +869,6 @@ func runFindPathWithMetadata(t *testing.T, useCache bool) { } ctx := newPathFindingTestContext(t, useCache, testChannels, "alice") - defer ctx.cleanup() paymentAmt := lnwire.NewMSatFromSatoshis(100) target := ctx.keyFromAlias("bob") @@ -952,7 +937,6 @@ func runFindLowestFeePath(t *testing.T, useCache bool) { } ctx := newPathFindingTestContext(t, useCache, testChannels, "roasbeef") - defer ctx.cleanup() const ( startingHeight = 100 @@ -1053,9 +1037,8 @@ var basicGraphPathFindingTests = []basicGraphPathFindingTestCase{ }} func runBasicGraphPathFinding(t *testing.T, useCache bool) { - testGraphInstance, err := parseTestGraph(useCache, basicGraphFilePath) + testGraphInstance, err := parseTestGraph(t, useCache, basicGraphFilePath) require.NoError(t, err, "unable to create graph") - defer testGraphInstance.cleanUp() // With the test graph loaded, we'll test some basic path finding using // the pre-generated graph. Consult the testdata/basic_graph.json file @@ -1218,9 +1201,8 @@ func testBasicGraphPathFindingCase(t *testing.T, graphInstance *testGraphInstanc // the path can support custom TLV records for the receiver under the // appropriate circumstances. func runPathFindingWithAdditionalEdges(t *testing.T, useCache bool) { - graph, err := parseTestGraph(useCache, basicGraphFilePath) + graph, err := parseTestGraph(t, useCache, basicGraphFilePath) require.NoError(t, err, "unable to create graph") - defer graph.cleanUp() sourceNode, err := graph.graph.SourceNode() require.NoError(t, err, "unable to fetch source node") @@ -1641,7 +1623,6 @@ func runNewRoutePathTooLong(t *testing.T, useCache bool) { } ctx := newPathFindingTestContext(t, useCache, testChannels, "start") - defer ctx.cleanup() // Assert that we can find 20 hop routes. node20 := ctx.keyFromAlias("node-20") @@ -1669,9 +1650,8 @@ func runNewRoutePathTooLong(t *testing.T, useCache bool) { } func runPathNotAvailable(t *testing.T, useCache bool) { - graph, err := parseTestGraph(useCache, basicGraphFilePath) + graph, err := parseTestGraph(t, useCache, basicGraphFilePath) require.NoError(t, err, "unable to create graph") - defer graph.cleanUp() sourceNode, err := graph.graph.SourceNode() require.NoError(t, err, "unable to fetch source node") @@ -1728,7 +1708,6 @@ func runDestTLVGraphFallback(t *testing.T, useCache bool) { } ctx := newPathFindingTestContext(t, useCache, testChannels, "roasbeef") - defer ctx.cleanup() sourceNode, err := ctx.graph.SourceNode() require.NoError(t, err, "unable to fetch source node") @@ -1826,7 +1805,6 @@ func runMissingFeatureDep(t *testing.T, useCache bool) { } ctx := newPathFindingTestContext(t, useCache, testChannels, "roasbeef") - defer ctx.cleanup() // Conner's node in the graph has a broken feature vector, since it // signals payment addresses without signaling tlv onions. Pathfinding @@ -1899,7 +1877,6 @@ func runUnknownRequiredFeatures(t *testing.T, useCache bool) { } ctx := newPathFindingTestContext(t, useCache, testChannels, "roasbeef") - defer ctx.cleanup() conner := ctx.keyFromAlias("conner") joost := ctx.keyFromAlias("joost") @@ -1941,7 +1918,6 @@ func runDestPaymentAddr(t *testing.T, useCache bool) { } ctx := newPathFindingTestContext(t, useCache, testChannels, "roasbeef") - defer ctx.cleanup() luoji := ctx.keyFromAlias("luoji") @@ -1967,9 +1943,8 @@ func runDestPaymentAddr(t *testing.T, useCache bool) { } func runPathInsufficientCapacity(t *testing.T, useCache bool) { - graph, err := parseTestGraph(useCache, basicGraphFilePath) + graph, err := parseTestGraph(t, useCache, basicGraphFilePath) require.NoError(t, err, "unable to create graph") - defer graph.cleanUp() sourceNode, err := graph.graph.SourceNode() require.NoError(t, err, "unable to fetch source node") @@ -1998,9 +1973,8 @@ func runPathInsufficientCapacity(t *testing.T, useCache bool) { // runRouteFailMinHTLC tests that if we attempt to route an HTLC which is // smaller than the advertised minHTLC of an edge, then path finding fails. func runRouteFailMinHTLC(t *testing.T, useCache bool) { - graph, err := parseTestGraph(useCache, basicGraphFilePath) + graph, err := parseTestGraph(t, useCache, basicGraphFilePath) require.NoError(t, err, "unable to create graph") - defer graph.cleanUp() sourceNode, err := graph.graph.SourceNode() require.NoError(t, err, "unable to fetch source node") @@ -2050,7 +2024,6 @@ func runRouteFailMaxHTLC(t *testing.T, useCache bool) { } ctx := newPathFindingTestContext(t, useCache, testChannels, "roasbeef") - defer ctx.cleanup() // First, attempt to send a payment greater than the max HTLC we are // about to set, which should succeed. @@ -2084,9 +2057,8 @@ func runRouteFailMaxHTLC(t *testing.T, useCache bool) { // ignore the disable flags, with the assumption that the correct bandwidth is // found among the bandwidth hints. func runRouteFailDisabledEdge(t *testing.T, useCache bool) { - graph, err := parseTestGraph(useCache, basicGraphFilePath) + graph, err := parseTestGraph(t, useCache, basicGraphFilePath) require.NoError(t, err, "unable to create graph") - defer graph.cleanUp() sourceNode, err := graph.graph.SourceNode() require.NoError(t, err, "unable to fetch source node") @@ -2150,9 +2122,8 @@ func runRouteFailDisabledEdge(t *testing.T, useCache bool) { // bandwidth hints is used by the path finding algorithm to consider whether to // use a local channel. func runPathSourceEdgesBandwidth(t *testing.T, useCache bool) { - graph, err := parseTestGraph(useCache, basicGraphFilePath) + graph, err := parseTestGraph(t, useCache, basicGraphFilePath) require.NoError(t, err, "unable to create graph") - defer graph.cleanUp() sourceNode, err := graph.graph.SourceNode() require.NoError(t, err, "unable to fetch source node") @@ -2462,7 +2433,6 @@ func runRestrictOutgoingChannel(t *testing.T, useCache bool) { } ctx := newPathFindingTestContext(t, useCache, testChannels, "roasbeef") - defer ctx.cleanup() const ( startingHeight = 100 @@ -2523,7 +2493,6 @@ func runRestrictLastHop(t *testing.T, useCache bool) { } ctx := newPathFindingTestContext(t, useCache, testChannels, "source") - defer ctx.cleanup() paymentAmt := lnwire.NewMSatFromSatoshis(100) target := ctx.keyFromAlias("target") @@ -2592,7 +2561,6 @@ func testCltvLimit(t *testing.T, useCache bool, limit uint32, } ctx := newPathFindingTestContext(t, useCache, testChannels, "roasbeef") - defer ctx.cleanup() paymentAmt := lnwire.NewMSatFromSatoshis(100) target := ctx.keyFromAlias("target") @@ -2770,7 +2738,6 @@ func testProbabilityRouting(t *testing.T, useCache bool, } ctx := newPathFindingTestContext(t, useCache, testChannels, "roasbeef") - defer ctx.cleanup() alias := ctx.testGraphInstance.aliasMap @@ -2853,7 +2820,6 @@ func runEqualCostRouteSelection(t *testing.T, useCache bool) { } ctx := newPathFindingTestContext(t, useCache, testChannels, "source") - defer ctx.cleanup() alias := ctx.testGraphInstance.aliasMap @@ -2922,7 +2888,6 @@ func runNoCycle(t *testing.T, useCache bool) { } ctx := newPathFindingTestContext(t, useCache, testChannels, "source") - defer ctx.cleanup() const ( startingHeight = 100 @@ -2975,7 +2940,6 @@ func runRouteToSelf(t *testing.T, useCache bool) { } ctx := newPathFindingTestContext(t, useCache, testChannels, "source") - defer ctx.cleanup() paymentAmt := lnwire.NewMSatFromSatoshis(100) target := ctx.source @@ -3013,7 +2977,7 @@ func newPathFindingTestContext(t *testing.T, useCache bool, testChannels []*testChannel, source string) *pathFindingTestContext { testGraphInstance, err := createTestGraphFromChannels( - useCache, testChannels, source, + t, useCache, testChannels, source, ) require.NoError(t, err, "unable to create graph") @@ -3046,10 +3010,6 @@ func (c *pathFindingTestContext) aliasFromKey(pubKey route.Vertex) string { return "" } -func (c *pathFindingTestContext) cleanup() { - c.testGraphInstance.cleanUp() -} - func (c *pathFindingTestContext) findPath(target route.Vertex, amt lnwire.MilliSatoshi) ([]*channeldb.CachedEdgePolicy, error) { diff --git a/routing/payment_lifecycle_test.go b/routing/payment_lifecycle_test.go index b34c63eb6..f398723bc 100644 --- a/routing/payment_lifecycle_test.go +++ b/routing/payment_lifecycle_test.go @@ -180,9 +180,8 @@ func TestRouterPaymentStateMachine(t *testing.T) { }, 2), } - testGraph, err := createTestGraphFromChannels(true, testChannels, "a") + testGraph, err := createTestGraphFromChannels(t, true, testChannels, "a") require.NoError(t, err, "unable to create graph") - defer testGraph.cleanUp() paymentAmt := lnwire.NewMSatFromSatoshis(1000) diff --git a/routing/router_test.go b/routing/router_test.go index 0ea0a79fe..42683fc9c 100644 --- a/routing/router_test.go +++ b/routing/router_test.go @@ -196,7 +196,7 @@ func createTestCtxFromGraphInstanceAssumeValid(t *testing.T, func createTestCtxSingleNode(t *testing.T, startingHeight uint32) (*testCtx, func()) { - graph, graphBackend, cleanup, err := makeTestGraph(true) + graph, graphBackend, err := makeTestGraph(t, true) require.NoError(t, err, "failed to make test graph") sourceNode, err := createTestNode() @@ -209,7 +209,6 @@ func createTestCtxSingleNode(t *testing.T, graphInstance := &testGraphInstance{ graph: graph, graphBackend: graphBackend, - cleanUp: cleanup, } return createTestCtxFromGraphInstance( @@ -222,7 +221,7 @@ func createTestCtxFromFile(t *testing.T, // We'll attempt to locate and parse out the file // that encodes the graph that our tests should be run against. - graphInstance, err := parseTestGraph(true, testGraph) + graphInstance, err := parseTestGraph(t, true, testGraph) require.NoError(t, err, "unable to create test graph") return createTestCtxFromGraphInstance( @@ -392,9 +391,8 @@ func TestChannelUpdateValidation(t *testing.T) { }, 2), } - testGraph, err := createTestGraphFromChannels(true, testChannels, "a") + testGraph, err := createTestGraphFromChannels(t, true, testChannels, "a") require.NoError(t, err, "unable to create graph") - defer testGraph.cleanUp() const startingBlockHeight = 101 ctx, cleanUp := createTestCtxFromGraphInstance( @@ -1247,10 +1245,9 @@ func TestIgnoreChannelEdgePolicyForUnknownChannel(t *testing.T) { // Setup an initially empty network. testChannels := []*testChannel{} testGraph, err := createTestGraphFromChannels( - true, testChannels, "roasbeef", + t, true, testChannels, "roasbeef", ) require.NoError(t, err, "unable to create graph") - defer testGraph.cleanUp() ctx, cleanUp := createTestCtxFromGraphInstance( t, startingBlockHeight, testGraph, false, @@ -2220,12 +2217,11 @@ func TestPruneChannelGraphStaleEdges(t *testing.T) { // We'll create our test graph and router backed with these test // channels we've created. testGraph, err := createTestGraphFromChannels( - true, testChannels, "a", + t, true, testChannels, "a", ) if err != nil { t.Fatalf("unable to create test graph: %v", err) } - defer testGraph.cleanUp() const startingHeight = 100 ctx, cleanUp := createTestCtxFromGraphInstance( @@ -2352,10 +2348,9 @@ func testPruneChannelGraphDoubleDisabled(t *testing.T, assumeValid bool) { // We'll create our test graph and router backed with these test // channels we've created. testGraph, err := createTestGraphFromChannels( - true, testChannels, "self", + t, true, testChannels, "self", ) require.NoError(t, err, "unable to create test graph") - defer testGraph.cleanUp() const startingHeight = 100 ctx, cleanUp := createTestCtxFromGraphInstanceAssumeValid( @@ -2711,8 +2706,7 @@ func TestUnknownErrorSource(t *testing.T) { }, 4), } - testGraph, err := createTestGraphFromChannels(true, testChannels, "a") - defer testGraph.cleanUp() + testGraph, err := createTestGraphFromChannels(t, true, testChannels, "a") require.NoError(t, err, "unable to create graph") const startingBlockHeight = 101 @@ -2843,9 +2837,8 @@ func TestSendToRouteStructuredError(t *testing.T) { }, 2), } - testGraph, err := createTestGraphFromChannels(true, testChannels, "a") + testGraph, err := createTestGraphFromChannels(t, true, testChannels, "a") require.NoError(t, err, "unable to create graph") - defer testGraph.cleanUp() const startingBlockHeight = 101 ctx, cleanUp := createTestCtxFromGraphInstance( @@ -2959,9 +2952,8 @@ func TestSendToRouteMaxHops(t *testing.T) { }, 1), } - testGraph, err := createTestGraphFromChannels(true, testChannels, "a") + testGraph, err := createTestGraphFromChannels(t, true, testChannels, "a") require.NoError(t, err, "unable to create graph") - defer testGraph.cleanUp() const startingBlockHeight = 101 @@ -3066,9 +3058,8 @@ func TestBuildRoute(t *testing.T) { }, 4), } - testGraph, err := createTestGraphFromChannels(true, testChannels, "a") + testGraph, err := createTestGraphFromChannels(t, true, testChannels, "a") require.NoError(t, err, "unable to create graph") - defer testGraph.cleanUp() const startingBlockHeight = 101 @@ -3304,7 +3295,7 @@ func createDummyTestGraph(t *testing.T) *testGraphInstance { }, 2), } - testGraph, err := createTestGraphFromChannels(true, testChannels, "a") + testGraph, err := createTestGraphFromChannels(t, true, testChannels, "a") require.NoError(t, err, "failed to create graph") return testGraph } From ca1cc68b7904630ace9a47a4b256421a8f03f851 Mon Sep 17 00:00:00 2001 From: Eng Zer Jun Date: Mon, 15 Aug 2022 21:08:22 +0800 Subject: [PATCH 19/23] tor: use `T.TempDir` to create temporary test directory Signed-off-by: Eng Zer Jun --- tor/cmd_onion_test.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/tor/cmd_onion_test.go b/tor/cmd_onion_test.go index f6b2c4b17..bef548a75 100644 --- a/tor/cmd_onion_test.go +++ b/tor/cmd_onion_test.go @@ -3,7 +3,6 @@ package tor import ( "bytes" "errors" - "io/ioutil" "path/filepath" "testing" @@ -16,11 +15,8 @@ import ( func TestOnionFile(t *testing.T) { t.Parallel() - tempDir, err := ioutil.TempDir("", "onion_store") - require.NoError(t, err, "unable to create temp dir") - privateKey := []byte("hide_me_plz") - privateKeyPath := filepath.Join(tempDir, "secret") + privateKeyPath := filepath.Join(t.TempDir(), "secret") // Create a new file-based onion store. A private key should not exist // yet. From 05c0dd7c910cf75b049fa45674e628ccb829e5b2 Mon Sep 17 00:00:00 2001 From: Eng Zer Jun Date: Mon, 15 Aug 2022 21:08:35 +0800 Subject: [PATCH 20/23] walletunlocker: use `T.TempDir` to create temporary test directory Signed-off-by: Eng Zer Jun --- walletunlocker/service_test.go | 71 +++++++++------------------------- 1 file changed, 18 insertions(+), 53 deletions(-) diff --git a/walletunlocker/service_test.go b/walletunlocker/service_test.go index 5c723d759..20c6ac8ce 100644 --- a/walletunlocker/service_test.go +++ b/walletunlocker/service_test.go @@ -4,7 +4,6 @@ import ( "bytes" "context" "fmt" - "io/ioutil" "os" "path" "testing" @@ -148,11 +147,7 @@ func TestGenSeed(t *testing.T) { // First, we'll create a new test directory and unlocker service for // that directory. - testDir, err := ioutil.TempDir("", "testcreate") - require.NoError(t, err) - defer func() { - _ = os.RemoveAll(testDir) - }() + testDir := t.TempDir() service := walletunlocker.New( testNetParams, nil, false, testLoaderOpts(testDir), @@ -186,11 +181,7 @@ func TestGenSeedGenerateEntropy(t *testing.T) { // First, we'll create a new test directory and unlocker service for // that directory. - testDir, err := ioutil.TempDir("", "testcreate") - require.NoError(t, err) - defer func() { - _ = os.RemoveAll(testDir) - }() + testDir := t.TempDir() service := walletunlocker.New( testNetParams, nil, false, testLoaderOpts(testDir), ) @@ -222,11 +213,7 @@ func TestGenSeedInvalidEntropy(t *testing.T) { // First, we'll create a new test directory and unlocker service for // that directory. - testDir, err := ioutil.TempDir("", "testcreate") - require.NoError(t, err) - defer func() { - _ = os.RemoveAll(testDir) - }() + testDir := t.TempDir() service := walletunlocker.New( testNetParams, nil, false, testLoaderOpts(testDir), ) @@ -242,7 +229,7 @@ func TestGenSeedInvalidEntropy(t *testing.T) { // We should get an error now since the entropy source was invalid. ctx := context.Background() - _, err = service.GenSeed(ctx, genSeedReq) + _, err := service.GenSeed(ctx, genSeedReq) require.Error(t, err) require.Contains(t, err.Error(), "incorrect entropy length") } @@ -253,11 +240,7 @@ func TestInitWallet(t *testing.T) { t.Parallel() // testDir is empty, meaning wallet was not created from before. - testDir, err := ioutil.TempDir("", "testcreate") - require.NoError(t, err) - defer func() { - _ = os.RemoveAll(testDir) - }() + testDir := t.TempDir() // Create new UnlockerService. service := walletunlocker.New( @@ -326,7 +309,7 @@ func TestInitWallet(t *testing.T) { // Now calling InitWallet should fail, since a wallet already exists in // the directory. - _, err = service.InitWallet(ctx, req) + _, err := service.InitWallet(ctx, req) require.Error(t, err) // Similarly, if we try to do GenSeed again, we should get an error as @@ -341,11 +324,7 @@ func TestCreateWalletInvalidEntropy(t *testing.T) { t.Parallel() // testDir is empty, meaning wallet was not created from before. - testDir, err := ioutil.TempDir("", "testcreate") - require.NoError(t, err) - defer func() { - _ = os.RemoveAll(testDir) - }() + testDir := t.TempDir() // Create new UnlockerService. service := walletunlocker.New( @@ -361,7 +340,7 @@ func TestCreateWalletInvalidEntropy(t *testing.T) { } ctx := context.Background() - _, err = service.InitWallet(ctx, req) + _, err := service.InitWallet(ctx, req) require.Error(t, err) } @@ -372,11 +351,7 @@ func TestUnlockWallet(t *testing.T) { t.Parallel() // testDir is empty, meaning wallet was not created from before. - testDir, err := ioutil.TempDir("", "testunlock") - require.NoError(t, err) - defer func() { - _ = os.RemoveAll(testDir) - }() + testDir := t.TempDir() // Create new UnlockerService that'll also drop the wallet's history on // unlock. @@ -392,7 +367,7 @@ func TestUnlockWallet(t *testing.T) { } // Should fail to unlock non-existing wallet. - _, err = service.UnlockWallet(ctx, req) + _, err := service.UnlockWallet(ctx, req) require.Error(t, err) // Create a wallet we can try to unlock. @@ -442,11 +417,7 @@ func TestChangeWalletPasswordNewRootkey(t *testing.T) { t.Parallel() // testDir is empty, meaning wallet was not created from before. - testDir, err := ioutil.TempDir("", "testchangepassword") - require.NoError(t, err) - defer func() { - _ = os.RemoveAll(testDir) - }() + testDir := t.TempDir() // Changing the password of the wallet will also try to change the // password of the macaroon DB. We create a default DB here but close it @@ -462,7 +433,7 @@ func TestChangeWalletPasswordNewRootkey(t *testing.T) { // requested. var tempFiles []string for i := 0; i < 3; i++ { - file, err := ioutil.TempFile(testDir, "") + file, err := os.CreateTemp(testDir, "") if err != nil { t.Fatalf("unable to create temp file: %v", err) } @@ -539,7 +510,9 @@ func TestChangeWalletPasswordNewRootkey(t *testing.T) { // The files should no longer exist. for _, tempFile := range tempFiles { - if _, err := os.Open(tempFile); err == nil { + f, err := os.Open(tempFile) + if err == nil { + _ = f.Close() t.Fatal("file exists but it shouldn't") } } @@ -553,11 +526,7 @@ func TestChangeWalletPasswordStateless(t *testing.T) { t.Parallel() // testDir is empty, meaning wallet was not created from before. - testDir, err := ioutil.TempDir("", "testchangepasswordstateless") - require.NoError(t, err) - defer func() { - _ = os.RemoveAll(testDir) - }() + testDir := t.TempDir() // Changing the password of the wallet will also try to change the // password of the macaroon DB. We create a default DB here but close it @@ -570,7 +539,7 @@ func TestChangeWalletPasswordStateless(t *testing.T) { // Create a temp file that will act as the macaroon DB file that will // be deleted by changing the password. - tmpFile, err := ioutil.TempFile(testDir, "") + tmpFile, err := os.CreateTemp(testDir, "") require.NoError(t, err) tempMacFile := tmpFile.Name() err = tmpFile.Close() @@ -636,6 +605,7 @@ func TestChangeWalletPasswordStateless(t *testing.T) { // Send a fake macaroon that should be returned in the response // in the async code above. service.MacResponseChan <- testMac + require.NoError(t, unlockMsg.UnloadWallet()) case <-time.After(defaultTestTimeout): t.Fatalf("password not received") @@ -683,9 +653,4 @@ func doChangePassword(service *walletunlocker.UnlockerService, testDir string, errChan <- fmt.Errorf("could not close store: %v", err) return } - err = os.RemoveAll(testDir) - if err != nil { - errChan <- err - return - } } From 6d00cdea453a2dc4d2d8456fd37b1c71a6bbfb48 Mon Sep 17 00:00:00 2001 From: Eng Zer Jun Date: Mon, 15 Aug 2022 21:08:45 +0800 Subject: [PATCH 21/23] watchtower/wtdb: use `T.TempDir` to create temporary test directory Signed-off-by: Eng Zer Jun --- watchtower/wtdb/client_db_test.go | 60 ++++++++++--------------------- watchtower/wtdb/tower_db_test.go | 58 ++++++++++-------------------- 2 files changed, 36 insertions(+), 82 deletions(-) diff --git a/watchtower/wtdb/client_db_test.go b/watchtower/wtdb/client_db_test.go index 266e65e7f..f694b746f 100644 --- a/watchtower/wtdb/client_db_test.go +++ b/watchtower/wtdb/client_db_test.go @@ -4,9 +4,7 @@ import ( "bytes" crand "crypto/rand" "io" - "io/ioutil" "net" - "os" "reflect" "testing" @@ -20,24 +18,23 @@ import ( "github.com/lightningnetwork/lnd/watchtower/wtpolicy" ) -// clientDBInit is a closure used to initialize a wtclient.DB instance its -// cleanup function. -type clientDBInit func(t *testing.T) (wtclient.DB, func()) +// clientDBInit is a closure used to initialize a wtclient.DB instance. +type clientDBInit func(t *testing.T) wtclient.DB type clientDBHarness struct { t *testing.T db wtclient.DB } -func newClientDBHarness(t *testing.T, init clientDBInit) (*clientDBHarness, func()) { - db, cleanup := init(t) +func newClientDBHarness(t *testing.T, init clientDBInit) *clientDBHarness { + db := init(t) h := &clientDBHarness{ t: t, db: db, } - return h, cleanup + return h } func (h *clientDBHarness) insertSession(session *wtdb.ClientSession, expErr error) { @@ -778,55 +775,40 @@ func TestClientDB(t *testing.T) { }{ { name: "fresh clientdb", - init: func(t *testing.T) (wtclient.DB, func()) { - path, err := ioutil.TempDir("", "clientdb") - if err != nil { - t.Fatalf("unable to make temp dir: %v", - err) - } - + init: func(t *testing.T) wtclient.DB { bdb, err := wtdb.NewBoltBackendCreator( - true, path, "wtclient.db", + true, t.TempDir(), "wtclient.db", )(dbCfg) if err != nil { - os.RemoveAll(path) t.Fatalf("unable to open db: %v", err) } db, err := wtdb.OpenClientDB(bdb) if err != nil { - os.RemoveAll(path) t.Fatalf("unable to open db: %v", err) } - cleanup := func() { + t.Cleanup(func() { db.Close() - os.RemoveAll(path) - } + }) - return db, cleanup + return db }, }, { name: "reopened clientdb", - init: func(t *testing.T) (wtclient.DB, func()) { - path, err := ioutil.TempDir("", "clientdb") - if err != nil { - t.Fatalf("unable to make temp dir: %v", - err) - } + init: func(t *testing.T) wtclient.DB { + path := t.TempDir() bdb, err := wtdb.NewBoltBackendCreator( true, path, "wtclient.db", )(dbCfg) if err != nil { - os.RemoveAll(path) t.Fatalf("unable to open db: %v", err) } db, err := wtdb.OpenClientDB(bdb) if err != nil { - os.RemoveAll(path) t.Fatalf("unable to open db: %v", err) } db.Close() @@ -835,28 +817,25 @@ func TestClientDB(t *testing.T) { true, path, "wtclient.db", )(dbCfg) if err != nil { - os.RemoveAll(path) t.Fatalf("unable to open db: %v", err) } db, err = wtdb.OpenClientDB(bdb) if err != nil { - os.RemoveAll(path) t.Fatalf("unable to reopen db: %v", err) } - cleanup := func() { + t.Cleanup(func() { db.Close() - os.RemoveAll(path) - } + }) - return db, cleanup + return db }, }, { name: "mock", - init: func(t *testing.T) (wtclient.DB, func()) { - return wtmock.NewClientDB(), func() {} + init: func(t *testing.T) wtclient.DB { + return wtmock.NewClientDB() }, }, } @@ -902,10 +881,7 @@ func TestClientDB(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { - h, cleanup := newClientDBHarness( - t, db.init, - ) - defer cleanup() + h := newClientDBHarness(t, db.init) test.run(h) }) diff --git a/watchtower/wtdb/tower_db_test.go b/watchtower/wtdb/tower_db_test.go index 62e19e018..177dbd233 100644 --- a/watchtower/wtdb/tower_db_test.go +++ b/watchtower/wtdb/tower_db_test.go @@ -3,8 +3,6 @@ package wtdb_test import ( "bytes" "encoding/binary" - "io/ioutil" - "os" "reflect" "testing" @@ -22,9 +20,8 @@ var ( testBlob = make([]byte, blob.Size(blob.TypeAltruistCommit)) ) -// dbInit is a closure used to initialize a watchtower.DB instance and its -// cleanup function. -type dbInit func(*testing.T) (watchtower.DB, func()) +// dbInit is a closure used to initialize a watchtower.DB instance. +type dbInit func(*testing.T) watchtower.DB // towerDBHarness holds the resources required to execute the tower db tests. type towerDBHarness struct { @@ -34,15 +31,15 @@ type towerDBHarness struct { // newTowerDBHarness initializes a fresh test harness for testing watchtower.DB // implementations. -func newTowerDBHarness(t *testing.T, init dbInit) (*towerDBHarness, func()) { - db, cleanup := init(t) +func newTowerDBHarness(t *testing.T, init dbInit) *towerDBHarness { + db := init(t) h := &towerDBHarness{ t: t, db: db, } - return h, cleanup + return h } // insertSession attempts to isnert the passed session and asserts that the @@ -637,55 +634,42 @@ func TestTowerDB(t *testing.T) { }{ { name: "fresh boltdb", - init: func(t *testing.T) (watchtower.DB, func()) { - path, err := ioutil.TempDir("", "towerdb") - if err != nil { - t.Fatalf("unable to make temp dir: %v", - err) - } + init: func(t *testing.T) watchtower.DB { + path := t.TempDir() bdb, err := wtdb.NewBoltBackendCreator( true, path, "watchtower.db", )(dbCfg) if err != nil { - os.RemoveAll(path) t.Fatalf("unable to open db: %v", err) } db, err := wtdb.OpenTowerDB(bdb) if err != nil { - os.RemoveAll(path) t.Fatalf("unable to open db: %v", err) } - cleanup := func() { + t.Cleanup(func() { db.Close() - os.RemoveAll(path) - } + }) - return db, cleanup + return db }, }, { name: "reopened boltdb", - init: func(t *testing.T) (watchtower.DB, func()) { - path, err := ioutil.TempDir("", "towerdb") - if err != nil { - t.Fatalf("unable to make temp dir: %v", - err) - } + init: func(t *testing.T) watchtower.DB { + path := t.TempDir() bdb, err := wtdb.NewBoltBackendCreator( true, path, "watchtower.db", )(dbCfg) if err != nil { - os.RemoveAll(path) t.Fatalf("unable to open db: %v", err) } db, err := wtdb.OpenTowerDB(bdb) if err != nil { - os.RemoveAll(path) t.Fatalf("unable to open db: %v", err) } db.Close() @@ -697,28 +681,25 @@ func TestTowerDB(t *testing.T) { true, path, "watchtower.db", )(dbCfg) if err != nil { - os.RemoveAll(path) t.Fatalf("unable to open db: %v", err) } db, err = wtdb.OpenTowerDB(bdb) if err != nil { - os.RemoveAll(path) t.Fatalf("unable to open db: %v", err) } - cleanup := func() { + t.Cleanup(func() { db.Close() - os.RemoveAll(path) - } + }) - return db, cleanup + return db }, }, { name: "mock", - init: func(t *testing.T) (watchtower.DB, func()) { - return wtmock.NewTowerDB(), func() {} + init: func(t *testing.T) watchtower.DB { + return wtmock.NewTowerDB() }, }, } @@ -792,10 +773,7 @@ func TestTowerDB(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { - h, cleanup := newTowerDBHarness( - t, db.init, - ) - defer cleanup() + h := newTowerDBHarness(t, db.init) test.run(h) }) From 1ceee96fc4e1a89314255ded91666009581f73dc Mon Sep 17 00:00:00 2001 From: Eng Zer Jun Date: Mon, 15 Aug 2022 21:09:18 +0800 Subject: [PATCH 22/23] server: use `T.TempDir` to create temporary test directory Signed-off-by: Eng Zer Jun --- server_test.go | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/server_test.go b/server_test.go index eb36026b4..41954d1c1 100644 --- a/server_test.go +++ b/server_test.go @@ -15,7 +15,6 @@ import ( "io/ioutil" "math/big" "net" - "os" "testing" "time" @@ -27,11 +26,7 @@ import ( // new TLS certificate pair is regenerated when the old pair expires. This is // necessary because the pair expires after a little over a year. func TestTLSAutoRegeneration(t *testing.T) { - tempDirPath, err := ioutil.TempDir("", ".testLnd") - if err != nil { - t.Fatalf("couldn't create temporary cert directory") - } - defer os.RemoveAll(tempDirPath) + tempDirPath := t.TempDir() certPath := tempDirPath + "/tls.cert" keyPath := tempDirPath + "/tls.key" From 5ee4c5657a2129dc38ab0b00b8f79f0dc23da413 Mon Sep 17 00:00:00 2001 From: Eng Zer Jun Date: Mon, 15 Aug 2022 21:13:03 +0800 Subject: [PATCH 23/23] docs: add release note entry for #6710 Signed-off-by: Eng Zer Jun --- docs/release-notes/release-notes-0.16.0.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/release-notes/release-notes-0.16.0.md b/docs/release-notes/release-notes-0.16.0.md index 7228cacd1..c643f9b4e 100644 --- a/docs/release-notes/release-notes-0.16.0.md +++ b/docs/release-notes/release-notes-0.16.0.md @@ -40,6 +40,9 @@ transaction](https://github.com/lightningnetwork/lnd/pull/6730). ## Code Health +* [test: use `T.TempDir` to create temporary test + directory](https://github.com/lightningnetwork/lnd/pull/6710) + ### Tooling and documentation * [The `golangci-lint` tool was updated to