From 39e54798f56996a91b1984f9eaff7538b4dfa5aa Mon Sep 17 00:00:00 2001
From: Adrian Gallagher <adrian.gallagher@thrasher.io>
Date: Mon, 10 Mar 2025 15:11:59 +1100
Subject: [PATCH] Improve testing, assertify usage and use
 common.ErrParsingWSField

---
 backtester/eventhandlers/strategies/strategies_test.go |  7 +++----
 config/versions/versions.go                            |  2 +-
 engine/engine_test.go                                  |  6 +++---
 exchanges/bitfinex/bitfinex_websocket.go               | 10 ++++------
 4 files changed, 11 insertions(+), 14 deletions(-)

diff --git a/backtester/eventhandlers/strategies/strategies_test.go b/backtester/eventhandlers/strategies/strategies_test.go
index 42a591b12d1..c7d708a5abc 100644
--- a/backtester/eventhandlers/strategies/strategies_test.go
+++ b/backtester/eventhandlers/strategies/strategies_test.go
@@ -83,12 +83,11 @@ func TestCreateNewStrategy(t *testing.T) {
 	t.Parallel()
 
 	// invalid Handler
-	resp, err := createNewStrategy(dollarcostaverage.Name, false, nil)
+	_, err := createNewStrategy(dollarcostaverage.Name, false, nil)
 	assert.ErrorIs(t, err, common.ErrNilPointer)
-	assert.Nil(t, resp)
 
 	// mismatched name
-	resp, err = createNewStrategy(dollarcostaverage.Name, false, &customStrategy{})
+	resp, err := createNewStrategy(dollarcostaverage.Name, false, &customStrategy{})
 	assert.NoError(t, err, "createNewStrategy should not error")
 	assert.Nil(t, resp)
 
@@ -101,7 +100,7 @@ func TestCreateNewStrategy(t *testing.T) {
 	h = new(dollarcostaverage.Strategy)
 	resp, err = createNewStrategy(dollarcostaverage.Name, true, h)
 	assert.NoError(t, err, "createNewStrategy should not error")
-	assert.NotNil(t, resp)
+	assert.NotSame(t, h, resp, "createNewStrategy must return a new pointer")
 
 	// simultaneous processing desired but not supported
 	h = &customStrategy{allowSimultaneousProcessing: false}
diff --git a/config/versions/versions.go b/config/versions/versions.go
index f669de0591d..1be1c97bfdc 100644
--- a/config/versions/versions.go
+++ b/config/versions/versions.go
@@ -217,7 +217,7 @@ func (m *manager) latest() (uint16, error) {
 	if len(m.versions) == 0 {
 		return 0, errNoVersions
 	}
-	return uint16(len(m.versions)) - 1, nil
+	return uint16(len(m.versions)) - 1, nil //nolint:gosec // Ignore this as we hardcode version numbers
 }
 
 // checkVersions ensures that registered versions are consistent
diff --git a/engine/engine_test.go b/engine/engine_test.go
index 83fbaa66620..d508155d863 100644
--- a/engine/engine_test.go
+++ b/engine/engine_test.go
@@ -386,9 +386,9 @@ func TestGetDefaultConfigurations(t *testing.T) {
 			assert.True(t, defaultCfg.Enabled, "Enabled should have the correct value")
 
 			if exch.SupportsWebsocket() {
-				assert.Positive(t, defaultCfg.WebsocketResponseCheckTimeout, 0)
-				assert.Positive(t, defaultCfg.WebsocketResponseMaxLimit, 0)
-				assert.Positive(t, defaultCfg.WebsocketTrafficTimeout, 0)
+				assert.Positive(t, defaultCfg.WebsocketResponseCheckTimeout, "WebsocketResponseCheckTimeout should be positive")
+				assert.Positive(t, defaultCfg.WebsocketResponseMaxLimit, "WebsocketResponseMaxLimit should be positive")
+				assert.Positive(t, defaultCfg.WebsocketTrafficTimeout, "WebsocketTrafficTimeout should be positive")
 			}
 
 			require.NoError(t, exch.Setup(defaultCfg), "Setup must not error")
diff --git a/exchanges/bitfinex/bitfinex_websocket.go b/exchanges/bitfinex/bitfinex_websocket.go
index 8f5f8e37ce6..020db54d126 100644
--- a/exchanges/bitfinex/bitfinex_websocket.go
+++ b/exchanges/bitfinex/bitfinex_websocket.go
@@ -34,8 +34,6 @@ import (
 	"github.com/thrasher-corp/gocryptotrader/log"
 )
 
-var errParsingWSField = errors.New("error parsing WS field")
-
 const (
 	authenticatedBitfinexWebsocketEndpoint = "wss://api.bitfinex.com/ws/2"
 	publicBitfinexWebsocketEndpoint        = "wss://api-pub.bitfinex.com/ws/2"
@@ -949,22 +947,22 @@ func (b *Bitfinex) handleWSAllTrades(s *subscription.Subscription, respRaw []byt
 	}
 	v, valueType, _, err := jsonparser.Get(respRaw, "[1]")
 	if err != nil {
-		return fmt.Errorf("%w `tradesUpdate[1]`: %w", errParsingWSField, err)
+		return fmt.Errorf("%w `tradesUpdate[1]`: %w", common.ErrParsingWSField, err)
 	}
 	var wsTrades []*wsTrade
 	switch valueType {
 	case jsonparser.String:
 		t, err := b.handleWSPublicTradeUpdate(respRaw)
 		if err != nil {
-			return fmt.Errorf("%w `tradesUpdate[2]`: %w", errParsingWSField, err)
+			return fmt.Errorf("%w `tradesUpdate[2]`: %w", common.ErrParsingWSField, err)
 		}
 		wsTrades = []*wsTrade{t}
 	case jsonparser.Array:
 		if wsTrades, err = b.handleWSPublicTradesSnapshot(v); err != nil {
-			return fmt.Errorf("%w `tradesSnapshot`: %w", errParsingWSField, err)
+			return fmt.Errorf("%w `tradesSnapshot`: %w", common.ErrParsingWSField, err)
 		}
 	default:
-		return fmt.Errorf("%w `tradesUpdate[1]`: %w `%s`", errParsingWSField, jsonparser.UnknownValueTypeError, valueType)
+		return fmt.Errorf("%w `tradesUpdate[1]`: %w `%s`", common.ErrParsingWSField, jsonparser.UnknownValueTypeError, valueType)
 	}
 	trades := make([]trade.Data, len(wsTrades))
 	for _, w := range wsTrades {