Merge pull request #5 from starius/minor-fixesx

cover errors.Is fix with tests

Author: hieblmi

loopd/daemon.go

@@ -47,6 +47,13 @@ var (
 	errOnlyStartOnce = fmt.Errorf("daemon can only be started once")
 )
 
+// shouldReportManagerErr determines whether a manager error should be forwarded
+// to the internal error channel. Context cancellations are treated as
+// non-fatal.
+func shouldReportManagerErr(err error) bool {
+	return err != nil && !errors.Is(err, context.Canceled)
+}
+
 // ListenerCfg holds closures used to retrieve listeners for the gRPC services.
 type ListenerCfg struct {
 	// grpcListener returns a TLS listener to use for the gRPC server, based
@@ -892,7 +899,7 @@ func (d *Daemon) initialize(withMacaroonService bool) error {
 			defer infof("Static address manager stopped")
 
 			err := staticAddressManager.Run(d.mainCtx, initChan)
-			if err != nil && !errors.Is(err, context.Canceled) {
+			if shouldReportManagerErr(err) {
 				d.internalErrChan <- err
 			}
 		}()
@@ -924,7 +931,7 @@ func (d *Daemon) initialize(withMacaroonService bool) error {
 			defer infof("Static address deposit manager stopped")
 
 			err := depositManager.Run(d.mainCtx, initChan)
-			if err != nil && !errors.Is(err, context.Canceled) {
+			if shouldReportManagerErr(err) {
 				d.internalErrChan <- err
 			}
 		}()
@@ -956,7 +963,7 @@ func (d *Daemon) initialize(withMacaroonService bool) error {
 			defer infof("Static address withdrawal manager stopped")
 
 			err := withdrawalManager.Run(d.mainCtx, initChan)
-			if err != nil && !errors.Is(err, context.Canceled) {
+			if shouldReportManagerErr(err) {
 				d.internalErrChan <- err
 			}
 		}()
@@ -992,7 +999,7 @@ func (d *Daemon) initialize(withMacaroonService bool) error {
 			infof("Starting static address loop-in manager...")
 			defer infof("Static address loop-in manager stopped")
 			err := staticLoopInManager.Run(d.mainCtx, initChan)
-			if err != nil && !errors.Is(err, context.Canceled) {
+			if shouldReportManagerErr(err) {
 				d.internalErrChan <- err
 			}
 		}()

loopd/daemon_test.go

@@ -0,0 +1,51 @@
+package loopd
+
+import (
+	"context"
+	"errors"
+	"fmt"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+// TestShouldReportManagerErr verifies that context cancellations are treated as
+// non-fatal while other errors are reported.
+func TestShouldReportManagerErr(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name     string
+		err      error
+		expected bool
+	}{
+		{
+			name:     "nil error",
+			err:      nil,
+			expected: false,
+		},
+		{
+			name:     "context canceled",
+			err:      context.Canceled,
+			expected: false,
+		},
+		{
+			name:     "wrapped context canceled",
+			err:      fmt.Errorf("wrap: %w", context.Canceled),
+			expected: false,
+		},
+		{
+			name:     "other error",
+			err:      errors.New("boom"),
+			expected: true,
+		},
+	}
+
+	for _, tt := range tests {
+		tt := tt
+		t.Run(tt.name, func(t *testing.T) {
+			got := shouldReportManagerErr(tt.err)
+			require.Equal(t, tt.expected, got)
+		})
+	}
+}

loopdb/store.go

@@ -18,6 +18,9 @@ import (
 )
 
 var (
+	// bboltOpen allows overriding the open function in tests.
+	bboltOpen = bbolt.Open
+
 	// dbFileName is the default file name of the client-side loop sub-swap
 	// database.
 	dbFileName = "loop.db"
@@ -191,7 +194,7 @@ func NewBoltSwapStore(dbPath string, chainParams *chaincfg.Params) (
 	// Now that we know that path exists, we'll open up bolt, which
 	// implements our default swap store.
 	path := filepath.Join(dbPath, dbFileName)
-	bdb, err := bbolt.Open(path, 0600, &bbolt.Options{
+	bdb, err := bboltOpen(path, 0600, &bbolt.Options{
 		Timeout: DefaultLoopDBTimeout,
 	})
 	if errors.Is(err, bbolt.ErrTimeout) {

loopdb/store_test.go

@@ -3,6 +3,7 @@ package loopdb
 import (
 	"context"
 	"crypto/sha256"
+	"fmt"
 	"io/ioutil"
 	"os"
 	"path/filepath"
@@ -50,6 +51,33 @@ var (
 	testTime = time.Date(2018, time.January, 9, 14, 00, 00, 0, time.UTC)
 )
 
+// TestNewBoltSwapStoreTimeout ensures a wrapped bbolt timeout is detected
+// correctly when opening the store.
+func TestNewBoltSwapStoreTimeout(t *testing.T) {
+	tempDir := t.TempDir()
+
+	// Override the bbolt open function to return a wrapped timeout.
+	origOpen := bboltOpen
+	t.Cleanup(func() {
+		bboltOpen = origOpen
+	})
+
+	wrappedErr := fmt.Errorf("wrapped: %w", bbolt.ErrTimeout)
+	bboltOpen = func(path string, mode os.FileMode,
+		options *bbolt.Options) (*bbolt.DB, error) {
+
+		require.NotNil(t, options)
+		require.Equal(t, filepath.Join(tempDir, dbFileName), path)
+
+		return nil, wrappedErr
+	}
+
+	store, err := NewBoltSwapStore(tempDir, &chaincfg.MainNetParams)
+	require.Nil(t, store)
+	require.ErrorIs(t, err, bbolt.ErrTimeout)
+	require.ErrorContains(t, err, "couldn't obtain exclusive lock")
+}
+
 // TestLoopOutStore tests all the basic functionality of the current bbolt
 // swap store.
 func TestLoopOutStore(t *testing.T) {