Merge pull request #6832 from kklash/switch-test-resource-cleanup

htlcswitch: clean up test resources and temporary files
2024-11-19 09:53:54 +01:00 · 2022-10-11 15:55:57 +02:00 · 2022-10-11 15:55:57 +02:00 · b387e2c718
commit b387e2c718
parent ff35647db3 63428de6e5
6 changed files with 84 additions and 50 deletions
--- a/docs/release-notes/release-notes-0.16.0.md
+++ b/docs/release-notes/release-notes-0.16.0.md
@ -16,9 +16,9 @@ transaction](https://github.com/lightningnetwork/lnd/pull/6730).
  allow specifying a root key for macaroons during wallet init rather than
  having lnd randomly generate one for you.

-* [A new `SignedInputs`](https://github.com/lightningnetwork/lnd/pull/6771) 
-  field is added to `SignPsbtResponse` that returns the indices of inputs 
-  that were signed by our wallet. Prior to this change `SignPsbt` didn't 
+* [A new `SignedInputs`](https://github.com/lightningnetwork/lnd/pull/6771)
+  field is added to `SignPsbtResponse` that returns the indices of inputs
+  that were signed by our wallet. Prior to this change `SignPsbt` didn't
  indicate whether the Psbt held any inputs for our wallet to sign.

 * [Add list addresses RPC](https://github.com/lightningnetwork/lnd/pull/6596).
@ -75,8 +75,8 @@ crash](https://github.com/lightningnetwork/lnd/pull/7019).

 ## `lncli`
 * [Add an `insecure` flag to skip tls auth as well as a `metadata` string slice
-  flag](https://github.com/lightningnetwork/lnd/pull/6818) that allows the 
-  caller to specify key-value string pairs that should be appended to the 
+  flag](https://github.com/lightningnetwork/lnd/pull/6818) that allows the
+  caller to specify key-value string pairs that should be appended to the
  outgoing context.

 * [Fix](https://github.com/lightningnetwork/lnd/pull/6858) command line argument
@ -96,7 +96,7 @@ crash](https://github.com/lightningnetwork/lnd/pull/7019).

 ## Code Health

-* [test: use `T.TempDir` to create temporary test 
+* [test: use `T.TempDir` to create temporary test
  directory](https://github.com/lightningnetwork/lnd/pull/6710)

 * [The `tlv` package now allows decoding records larger than 65535 bytes. The
@ -108,6 +108,8 @@ crash](https://github.com/lightningnetwork/lnd/pull/7019).
 * [The `golangci-lint` tool was updated to
  `v1.46.2`](https://github.com/lightningnetwork/lnd/pull/6731)

+* [Tests in `htlcswitch` will now clean up the temporary resources they create](https://github.com/lightningnetwork/lnd/pull/6832).
+
 * Updated the github actions to use `make fmt-check` in its [build
  process](https://github.com/lightningnetwork/lnd/pull/6853).

--- a/htlcswitch/circuit_test.go
+++ b/htlcswitch/circuit_test.go
@ -628,6 +628,7 @@ func makeCircuitDB(t *testing.T, path string) *channeldb.DB {

 	db, err := channeldb.Open(path)
 	require.NoError(t, err, "unable to open channel db")
+	t.Cleanup(func() { db.Close() })

 	return db
 }
--- a/htlcswitch/link_test.go
+++ b/htlcswitch/link_test.go
@ -1945,7 +1945,8 @@ func newSingleLinkTestHarness(chanAmt, chanReserve btcutil.Amount) (

 	cleanUp := func() {
 		close(alicePeer.quit)
-		defer fCleanUp()
+		invoiceRegistry.cleanup()
+		fCleanUp()
 	}

 	return aliceLink, bobLc.channel, bticker.Force, start, cleanUp,
--- a/htlcswitch/mock.go
+++ b/htlcswitch/mock.go
@ -9,6 +9,7 @@ import (
 	"io/ioutil"
 	"net"
 	"os"
+	"path/filepath"
 	"sync"
 	"sync/atomic"
 	"testing"
@ -161,30 +162,7 @@ type mockServer struct {

 var _ lnpeer.Peer = (*mockServer)(nil)

-func initDB() (*channeldb.DB, error) {
-	tempPath, err := ioutil.TempDir("", "switchdb")
-	if err != nil {
-		return nil, err
-	}
-
-	db, err := channeldb.Open(tempPath)
-	if err != nil {
-		return nil, err
-	}
-
-	return db, err
-}
-
 func initSwitchWithDB(startingHeight uint32, db *channeldb.DB) (*Switch, error) {
-	var err error
-
-	if db == nil {
-		db, err = initDB()
-		if err != nil {
-			return nil, err
-		}
-	}
-
 	signAliasUpdate := func(u *lnwire.ChannelUpdate) (*ecdsa.Signature,
 		error) {

@ -226,6 +204,24 @@ func initSwitchWithDB(startingHeight uint32, db *channeldb.DB) (*Switch, error)
 	return New(cfg, startingHeight)
 }

+func initSwitchWithTempDB(t testing.TB, startingHeight uint32) (*Switch,
+	error) {
+
+	tempPath := filepath.Join(t.TempDir(), "switchdb")
+	db, err := channeldb.Open(tempPath)
+	if err != nil {
+		return nil, err
+	}
+	t.Cleanup(func() { db.Close() })
+
+	s, err := initSwitchWithDB(startingHeight, db)
+	if err != nil {
+		return nil, err
+	}
+
+	return s, nil
+}
+
 func newMockServer(t testing.TB, name string, startingHeight uint32,
 	db *channeldb.DB, defaultDelta uint32) (*mockServer, error) {

@ -235,13 +231,25 @@ func newMockServer(t testing.TB, name string, startingHeight uint32,

 	pCache := newMockPreimageCache()

-	htlcSwitch, err := initSwitchWithDB(startingHeight, db)
+	var (
+		htlcSwitch *Switch
+		err        error
+	)
+	if db == nil {
+		htlcSwitch, err = initSwitchWithTempDB(t, startingHeight)
+	} else {
+		htlcSwitch, err = initSwitchWithDB(startingHeight, db)
+	}
 	if err != nil {
 		return nil, err
 	}

+	t.Cleanup(func() { _ = htlcSwitch.Stop() })
+
 	registry := newMockRegistry(defaultDelta)

+	t.Cleanup(func() { registry.cleanup() })
+
 	return &mockServer{
 		t:                t,
 		id:               id,
--- a/htlcswitch/payment_result_test.go
+++ b/htlcswitch/payment_result_test.go
@ -106,6 +106,7 @@ func TestNetworkResultStore(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
+	t.Cleanup(func() { db.Close() })

 	store := newNetworkResultStore(db)

--- a/htlcswitch/switch_test.go
+++ b/htlcswitch/switch_test.go
@ -47,7 +47,7 @@ func TestSwitchAddDuplicateLink(t *testing.T) {
 	)
 	require.NoError(t, err, "unable to create alice server")

-	s, err := initSwitchWithDB(testStartingHeight, nil)
+	s, err := initSwitchWithTempDB(t, testStartingHeight)
 	require.NoError(t, err, "unable to init switch")
 	if err := s.Start(); err != nil {
 		t.Fatalf("unable to start switch: %v", err)
@ -90,7 +90,7 @@ func TestSwitchHasActiveLink(t *testing.T) {
 	)
 	require.NoError(t, err, "unable to create alice server")

-	s, err := initSwitchWithDB(testStartingHeight, nil)
+	s, err := initSwitchWithTempDB(t, testStartingHeight)
 	require.NoError(t, err, "unable to init switch")
 	if err := s.Start(); err != nil {
 		t.Fatalf("unable to start switch: %v", err)
@ -140,7 +140,7 @@ func TestSwitchSendPending(t *testing.T) {
 	)
 	require.NoError(t, err, "unable to create bob server")

-	s, err := initSwitchWithDB(testStartingHeight, nil)
+	s, err := initSwitchWithTempDB(t, testStartingHeight)
 	require.NoError(t, err, "unable to init switch")
 	if err := s.Start(); err != nil {
 		t.Fatalf("unable to start switch: %v", err)
@ -464,7 +464,7 @@ func testSwitchForwardMapping(t *testing.T, alicePrivate, aliceZeroConf,
 	)
 	require.NoError(t, err)

-	s, err := initSwitchWithDB(testStartingHeight, nil)
+	s, err := initSwitchWithTempDB(t, testStartingHeight)
 	require.NoError(t, err)
 	err = s.Start()
 	require.NoError(t, err)
@ -676,7 +676,7 @@ func testSwitchSendHtlcMapping(t *testing.T, zeroConf, useAlias bool, alias,
 	)
 	require.NoError(t, err)

-	s, err := initSwitchWithDB(testStartingHeight, nil)
+	s, err := initSwitchWithTempDB(t, testStartingHeight)
 	require.NoError(t, err)
 	err = s.Start()
 	require.NoError(t, err)
@ -738,7 +738,7 @@ func TestSwitchUpdateScid(t *testing.T) {
 	)
 	require.NoError(t, err, "unable to create alice server")

-	s, err := initSwitchWithDB(testStartingHeight, nil)
+	s, err := initSwitchWithTempDB(t, testStartingHeight)
 	require.NoError(t, err)
 	err = s.Start()
 	require.NoError(t, err)
@ -882,7 +882,7 @@ func TestSwitchForward(t *testing.T) {
 		t.Fatalf("unable to create bob server: %v", err)
 	}

-	s, err := initSwitchWithDB(testStartingHeight, nil)
+	s, err := initSwitchWithTempDB(t, testStartingHeight)
 	if err != nil {
 		t.Fatalf("unable to init switch: %v", err)
 	}
@ -999,6 +999,7 @@ func TestSwitchForwardFailAfterFullAdd(t *testing.T) {

 	cdb, err := channeldb.Open(tempPath)
 	require.NoError(t, err, "unable to open channeldb")
+	t.Cleanup(func() { cdb.Close() })

 	s, err := initSwitchWithDB(testStartingHeight, cdb)
 	require.NoError(t, err, "unable to init switch")
@ -1092,6 +1093,7 @@ func TestSwitchForwardFailAfterFullAdd(t *testing.T) {

 	cdb2, err := channeldb.Open(tempPath)
 	require.NoError(t, err, "unable to reopen channeldb")
+	t.Cleanup(func() { cdb2.Close() })

 	s2, err := initSwitchWithDB(testStartingHeight, cdb2)
 	require.NoError(t, err, "unable reinit switch")
@ -1187,6 +1189,7 @@ func TestSwitchForwardSettleAfterFullAdd(t *testing.T) {

 	cdb, err := channeldb.Open(tempPath)
 	require.NoError(t, err, "unable to open channeldb")
+	t.Cleanup(func() { cdb.Close() })

 	s, err := initSwitchWithDB(testStartingHeight, cdb)
 	require.NoError(t, err, "unable to init switch")
@ -1280,6 +1283,7 @@ func TestSwitchForwardSettleAfterFullAdd(t *testing.T) {

 	cdb2, err := channeldb.Open(tempPath)
 	require.NoError(t, err, "unable to reopen channeldb")
+	t.Cleanup(func() { cdb2.Close() })

 	s2, err := initSwitchWithDB(testStartingHeight, cdb2)
 	require.NoError(t, err, "unable reinit switch")
@ -1378,6 +1382,7 @@ func TestSwitchForwardDropAfterFullAdd(t *testing.T) {

 	cdb, err := channeldb.Open(tempPath)
 	require.NoError(t, err, "unable to open channeldb")
+	t.Cleanup(func() { cdb.Close() })

 	s, err := initSwitchWithDB(testStartingHeight, cdb)
 	require.NoError(t, err, "unable to init switch")
@ -1463,6 +1468,7 @@ func TestSwitchForwardDropAfterFullAdd(t *testing.T) {

 	cdb2, err := channeldb.Open(tempPath)
 	require.NoError(t, err, "unable to reopen channeldb")
+	t.Cleanup(func() { cdb2.Close() })

 	s2, err := initSwitchWithDB(testStartingHeight, cdb2)
 	require.NoError(t, err, "unable reinit switch")
@ -1532,6 +1538,7 @@ func TestSwitchForwardFailAfterHalfAdd(t *testing.T) {

 	cdb, err := channeldb.Open(tempPath)
 	require.NoError(t, err, "unable to open channeldb")
+	t.Cleanup(func() { cdb.Close() })

 	s, err := initSwitchWithDB(testStartingHeight, cdb)
 	require.NoError(t, err, "unable to init switch")
@ -1612,6 +1619,7 @@ func TestSwitchForwardFailAfterHalfAdd(t *testing.T) {

 	cdb2, err := channeldb.Open(tempPath)
 	require.NoError(t, err, "unable to reopen channeldb")
+	t.Cleanup(func() { cdb2.Close() })

 	s2, err := initSwitchWithDB(testStartingHeight, cdb2)
 	require.NoError(t, err, "unable reinit switch")
@ -1687,6 +1695,7 @@ func TestSwitchForwardCircuitPersistence(t *testing.T) {

 	cdb, err := channeldb.Open(tempPath)
 	require.NoError(t, err, "unable to open channeldb")
+	t.Cleanup(func() { cdb.Close() })

 	s, err := initSwitchWithDB(testStartingHeight, cdb)
 	require.NoError(t, err, "unable to init switch")
@ -1766,6 +1775,7 @@ func TestSwitchForwardCircuitPersistence(t *testing.T) {

 	cdb2, err := channeldb.Open(tempPath)
 	require.NoError(t, err, "unable to reopen channeldb")
+	t.Cleanup(func() { cdb2.Close() })

 	s2, err := initSwitchWithDB(testStartingHeight, cdb2)
 	require.NoError(t, err, "unable reinit switch")
@ -1857,6 +1867,7 @@ func TestSwitchForwardCircuitPersistence(t *testing.T) {

 	cdb3, err := channeldb.Open(tempPath)
 	require.NoError(t, err, "unable to reopen channeldb")
+	t.Cleanup(func() { cdb3.Close() })

 	s3, err := initSwitchWithDB(testStartingHeight, cdb3)
 	require.NoError(t, err, "unable reinit switch")
@ -1937,7 +1948,7 @@ func TestCircularForwards(t *testing.T) {
 					err)
 			}

-			s, err := initSwitchWithDB(testStartingHeight, nil)
+			s, err := initSwitchWithTempDB(t, testStartingHeight)
 			if err != nil {
 				t.Fatalf("unable to init switch: %v", err)
 			}
@ -2111,7 +2122,7 @@ func TestCheckCircularForward(t *testing.T) {
 		t.Run(test.name, func(t *testing.T) {
 			t.Parallel()

-			s, err := initSwitchWithDB(testStartingHeight, nil)
+			s, err := initSwitchWithTempDB(t, testStartingHeight)
 			require.NoError(t, err)
 			err = s.Start()
 			require.NoError(t, err)
@ -2216,7 +2227,7 @@ func testSkipIneligibleLinksMultiHopForward(t *testing.T,
 	)
 	require.NoError(t, err, "unable to create bob server")

-	s, err := initSwitchWithDB(testStartingHeight, nil)
+	s, err := initSwitchWithTempDB(t, testStartingHeight)
 	require.NoError(t, err, "unable to init switch")
 	if err := s.Start(); err != nil {
 		t.Fatalf("unable to start switch: %v", err)
@ -2339,7 +2350,7 @@ func testSkipLinkLocalForward(t *testing.T, eligible bool,
 	)
 	require.NoError(t, err, "unable to create alice server")

-	s, err := initSwitchWithDB(testStartingHeight, nil)
+	s, err := initSwitchWithTempDB(t, testStartingHeight)
 	require.NoError(t, err, "unable to init switch")
 	if err := s.Start(); err != nil {
 		t.Fatalf("unable to start switch: %v", err)
@ -2394,7 +2405,7 @@ func TestSwitchCancel(t *testing.T) {
 	)
 	require.NoError(t, err, "unable to create bob server")

-	s, err := initSwitchWithDB(testStartingHeight, nil)
+	s, err := initSwitchWithTempDB(t, testStartingHeight)
 	require.NoError(t, err, "unable to init switch")
 	if err := s.Start(); err != nil {
 		t.Fatalf("unable to start switch: %v", err)
@ -2505,7 +2516,7 @@ func TestSwitchAddSamePayment(t *testing.T) {
 	)
 	require.NoError(t, err, "unable to create bob server")

-	s, err := initSwitchWithDB(testStartingHeight, nil)
+	s, err := initSwitchWithTempDB(t, testStartingHeight)
 	require.NoError(t, err, "unable to init switch")
 	if err := s.Start(); err != nil {
 		t.Fatalf("unable to start switch: %v", err)
@ -2658,7 +2669,7 @@ func TestSwitchSendPayment(t *testing.T) {
 	)
 	require.NoError(t, err, "unable to create alice server")

-	s, err := initSwitchWithDB(testStartingHeight, nil)
+	s, err := initSwitchWithTempDB(t, testStartingHeight)
 	require.NoError(t, err, "unable to init switch")
 	if err := s.Start(); err != nil {
 		t.Fatalf("unable to start switch: %v", err)
@ -3075,7 +3086,7 @@ func TestSwitchGetPaymentResult(t *testing.T) {
 	var preimg lntypes.Preimage
 	preimg[0] = 3

-	s, err := initSwitchWithDB(testStartingHeight, nil)
+	s, err := initSwitchWithTempDB(t, testStartingHeight)
 	require.NoError(t, err, "unable to init switch")
 	if err := s.Start(); err != nil {
 		t.Fatalf("unable to start switch: %v", err)
@ -3178,7 +3189,7 @@ func TestInvalidFailure(t *testing.T) {
 	)
 	require.NoError(t, err, "unable to create alice server")

-	s, err := initSwitchWithDB(testStartingHeight, nil)
+	s, err := initSwitchWithTempDB(t, testStartingHeight)
 	require.NoError(t, err, "unable to init switch")
 	if err := s.Start(); err != nil {
 		t.Fatalf("unable to start switch: %v", err)
@ -3768,6 +3779,7 @@ func TestSwitchHoldForward(t *testing.T) {

 	cdb, err := channeldb.Open(tempPath)
 	require.NoError(t, err, "unable to open channeldb")
+	t.Cleanup(func() { cdb.Close() })

 	s, err := initSwitchWithDB(testStartingHeight, cdb)
 	require.NoError(t, err, "unable to init switch")
@ -4401,7 +4413,7 @@ func TestSwitchMailboxDust(t *testing.T) {
 	)
 	require.NoError(t, err)

-	s, err := initSwitchWithDB(testStartingHeight, nil)
+	s, err := initSwitchWithTempDB(t, testStartingHeight)
 	require.NoError(t, err)
 	err = s.Start()
 	require.NoError(t, err)
@ -4525,9 +4537,14 @@ func TestSwitchResolution(t *testing.T) {
 	)
 	require.NoError(t, err)

-	s, err := initSwitchWithDB(testStartingHeight, nil)
+	s, err := initSwitchWithTempDB(t, testStartingHeight)
 	require.NoError(t, err)

+	// Even though we intend to Stop s later in the test, it is safe to
+	// defer this Stop since its execution it is protected by an atomic
+	// guard, guaranteeing it executes at most once.
+	t.Cleanup(func() { var _ = s.Stop() })
+
 	err = s.Start()
 	require.NoError(t, err)

@ -4702,6 +4719,7 @@ func testSwitchForwardFailAlias(t *testing.T, zeroConf bool) {

 	cdb, err := channeldb.Open(tempPath)
 	require.NoError(t, err)
+	t.Cleanup(func() { cdb.Close() })

 	s, err := initSwitchWithDB(testStartingHeight, cdb)
 	require.NoError(t, err)
@ -4777,6 +4795,7 @@ func testSwitchForwardFailAlias(t *testing.T, zeroConf bool) {

 	cdb2, err := channeldb.Open(tempPath)
 	require.NoError(t, err)
+	t.Cleanup(func() { cdb2.Close() })

 	s2, err := initSwitchWithDB(testStartingHeight, cdb2)
 	require.NoError(t, err)
@ -4916,6 +4935,7 @@ func testSwitchAliasFailAdd(t *testing.T, zeroConf, private, useAlias bool) {

 	cdb, err := channeldb.Open(tempPath)
 	require.NoError(t, err)
+	defer cdb.Close()

 	s, err := initSwitchWithDB(testStartingHeight, cdb)
 	require.NoError(t, err)
@ -5099,7 +5119,7 @@ func testSwitchHandlePacketForward(t *testing.T, zeroConf, private,
 	require.NoError(t, err)
 	defer cleanUp()

-	s, err := initSwitchWithDB(testStartingHeight, nil)
+	s, err := initSwitchWithTempDB(t, testStartingHeight)
 	if err != nil {
 		t.Fatalf("unable to init switch: %v", err)
 	}
@ -5255,6 +5275,7 @@ func testSwitchAliasInterceptFail(t *testing.T, zeroConf bool) {

 	cdb, err := channeldb.Open(tempPath)
 	require.NoError(t, err)
+	t.Cleanup(func() { cdb.Close() })

 	s, err := initSwitchWithDB(testStartingHeight, cdb)
 	require.NoError(t, err)