From 46aafb6f1e5be4404955e4802c4729287db94d34 Mon Sep 17 00:00:00 2001 From: Adrian Gallagher Date: Tue, 4 Mar 2025 15:25:38 +1100 Subject: [PATCH] Address glorious nits --- .golangci.yml | 2 +- engine/engine_test.go | 42 ++++-------------- engine/rpcserver_test.go | 33 ++++---------- exchanges/order/order_test.go | 62 +++++++++++--------------- exchanges/poloniex/poloniex.go | 66 ++++------------------------ exchanges/poloniex/poloniex_types.go | 11 ++--- 6 files changed, 58 insertions(+), 158 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 2cafb477e61..9e9ae2d6b9a 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -109,7 +109,7 @@ linters: - unconvert - unparam - usestdlibvars -# - usetesting +# - usetesting # Disabled temporarily due to the number of t.Context changes required # - varnamelen - wastedassign - whitespace diff --git a/engine/engine_test.go b/engine/engine_test.go index 68864dff219..f1d704e4c3e 100644 --- a/engine/engine_test.go +++ b/engine/engine_test.go @@ -369,9 +369,7 @@ func TestGetDefaultConfigurations(t *testing.T) { t.Run(name, func(t *testing.T) { t.Parallel() exch, err := em.NewExchangeByName(name) - if err != nil { - t.Fatal(err) - } + require.NoError(t, err, "NewExchangeByName must not error") if isCITest() && slices.Contains(blockedCIExchanges, name) { t.Skipf("skipping %s due to CI test restrictions", name) @@ -382,40 +380,18 @@ func TestGetDefaultConfigurations(t *testing.T) { } defaultCfg, err := exchange.GetDefaultConfig(context.Background(), exch) - if err != nil { - t.Fatal(err) - } - - if defaultCfg == nil { - t.Fatal("expected config") - } - - if defaultCfg.Name == "" { - t.Error("name unset SetDefaults() not called") - } - - if !defaultCfg.Enabled { - t.Error("expected enabled", defaultCfg.Name) - } + require.NoError(t, err, "GetDefaultConfig must not error") + require.NotNil(t, defaultCfg) + assert.NotEmpty(t, defaultCfg.Name) + assert.True(t, defaultCfg.Enabled) if exch.SupportsWebsocket() { - if defaultCfg.WebsocketResponseCheckTimeout <= 0 { - t.Error("expected websocketResponseCheckTimeout to be greater than 0", defaultCfg.Name) - } - - if defaultCfg.WebsocketResponseMaxLimit <= 0 { - t.Error("expected WebsocketResponseMaxLimit to be greater than 0", defaultCfg.Name) - } - - if defaultCfg.WebsocketTrafficTimeout <= 0 { - t.Error("expected WebsocketTrafficTimeout to be greater than 0", defaultCfg.Name) - } + assert.Positive(t, defaultCfg.WebsocketResponseCheckTimeout, 0) + assert.Positive(t, defaultCfg.WebsocketResponseMaxLimit, 0) + assert.Positive(t, defaultCfg.WebsocketTrafficTimeout, 0) } - // Makes sure the config is valid and can be used to setup the exchange - if err := exch.Setup(defaultCfg); err != nil { - t.Fatal(err) - } + require.NoError(t, exch.Setup(defaultCfg), "Setup must not error") }) } } diff --git a/engine/rpcserver_test.go b/engine/rpcserver_test.go index c80a9cdc72a..b99f650750a 100644 --- a/engine/rpcserver_test.go +++ b/engine/rpcserver_test.go @@ -1945,35 +1945,18 @@ func TestGetDataHistoryJobSummary(t *testing.T) { EndDate: time.Now().UTC(), Interval: kline.OneMin, } - - err := m.UpsertJob(dhj, false) - if !errors.Is(err, nil) { - t.Errorf("received %v, expected %v", err, nil) - } - - _, err = s.GetDataHistoryJobSummary(context.Background(), nil) - if !errors.Is(err, errNilRequestData) { - t.Errorf("received %v, expected %v", err, errNilRequestData) - } + assert.NoError(t, m.UpsertJob(dhj, false), "UpsertJob should not error") + _, err := s.GetDataHistoryJobSummary(context.Background(), nil) + assert.ErrorIs(t, err, errNilRequestData) _, err = s.GetDataHistoryJobSummary(context.Background(), &gctrpc.GetDataHistoryJobDetailsRequest{}) - if !errors.Is(err, errNicknameUnset) { - t.Errorf("received %v, expected %v", err, errNicknameUnset) - } + assert.ErrorIs(t, err, errNicknameUnset) resp, err := s.GetDataHistoryJobSummary(context.Background(), &gctrpc.GetDataHistoryJobDetailsRequest{Nickname: "TestGetDataHistoryJobSummary"}) - if !errors.Is(err, nil) { - t.Errorf("received %v, expected %v", err, nil) - } - if resp == nil { //nolint:staticcheck,nolintlint // SA5011 Ignore the nil warnings - t.Fatal("expected job") - } - if resp.Nickname == "" { - t.Fatalf("received %v, expected %v", "", dhj.Nickname) - } - if resp.ResultSummaries == nil { //nolint:staticcheck,nolintlint // SA5011 Ignore the nil warnings - t.Fatalf("received %v, expected %v", nil, "result summaries slice") - } + assert.NoError(t, err, "GetDataHistoryJobSummary should not error") + require.NotNil(t, resp) + assert.NotEmpty(t, resp.Nickname) + assert.NotEmpty(t, resp.ResultSummaries) } func TestGetManagedOrders(t *testing.T) { diff --git a/exchanges/order/order_test.go b/exchanges/order/order_test.go index 3799726259e..7253d55be7c 100644 --- a/exchanges/order/order_test.go +++ b/exchanges/order/order_test.go @@ -1874,9 +1874,8 @@ func TestDetail_CopyPointerOrderSlice(t *testing.T) { func TestDeriveModify(t *testing.T) { t.Parallel() var o *Detail - if _, err := o.DeriveModify(); !errors.Is(err, errOrderDetailIsNil) { - t.Fatalf("received: '%v' but expected: '%v'", err, errOrderDetailIsNil) - } + _, err := o.DeriveModify() + require.ErrorIs(t, err, errOrderDetailIsNil) pair := currency.NewPair(currency.BTC, currency.AUD) @@ -1891,31 +1890,26 @@ func TestDeriveModify(t *testing.T) { } mod, err := o.DeriveModify() - if !errors.Is(err, nil) { - t.Fatalf("received: '%v' but expected: '%v'", err, nil) - } - - if mod == nil { - t.Fatal("should not be nil") - } + require.NoError(t, err) + require.NotNil(t, mod) - if mod.Exchange != "wow" || - mod.OrderID != "wow2" || - mod.ClientOrderID != "wow3" || - mod.Type != Market || - mod.Side != Long || - mod.AssetType != asset.Futures || - !mod.Pair.Equal(pair) { - t.Fatal("unexpected values") + exp := &Modify{ + Exchange: "wow", + OrderID: "wow2", + ClientOrderID: "wow3", + Type: Market, + Side: Long, + AssetType: asset.Futures, + Pair: pair, } + assert.Equal(t, exp, mod) } func TestDeriveModifyResponse(t *testing.T) { t.Parallel() var mod *Modify - if _, err := mod.DeriveModifyResponse(); !errors.Is(err, errOrderDetailIsNil) { - t.Fatalf("received: '%v' but expected: '%v'", err, errOrderDetailIsNil) - } + _, err := mod.DeriveModifyResponse() + require.ErrorIs(t, err, errOrderDetailIsNil) pair := currency.NewPair(currency.BTC, currency.AUD) @@ -1930,23 +1924,19 @@ func TestDeriveModifyResponse(t *testing.T) { } modresp, err := mod.DeriveModifyResponse() - if !errors.Is(err, nil) { - t.Fatalf("received: '%v' but expected: '%v'", err, nil) - } - - if modresp == nil { - t.Fatal("should not be nil") - } + require.NoError(t, err, "DeriveModifyResponse must not error") + require.NotNil(t, modresp) - if modresp.Exchange != "wow" || - modresp.OrderID != "wow2" || - modresp.ClientOrderID != "wow3" || - modresp.Type != Market || - modresp.Side != Long || - modresp.AssetType != asset.Futures || - !modresp.Pair.Equal(pair) { - t.Fatal("unexpected values") + exp := &ModifyResponse{ + Exchange: "wow", + OrderID: "wow2", + ClientOrderID: "wow3", + Type: Market, + Side: Long, + AssetType: asset.Futures, + Pair: pair, } + assert.Equal(t, exp, modresp) } func TestDeriveCancel(t *testing.T) { diff --git a/exchanges/poloniex/poloniex.go b/exchanges/poloniex/poloniex.go index 25d64513c78..9ec6ed13e4d 100644 --- a/exchanges/poloniex/poloniex.go +++ b/exchanges/poloniex/poloniex.go @@ -112,40 +112,15 @@ func (p *Poloniex) GetOrderbook(ctx context.Context, currencyPair string, depth Asks: make([]OrderbookItem, len(resp.Asks)), } for x := range resp.Asks { - askPrice, ok := resp.Asks[x][0].(string) - if !ok { - return oba, common.GetTypeAssertError("string", resp.Asks[x][0], "price") - } - price, err := strconv.ParseFloat(askPrice, 64) - if err != nil { - return oba, err - } - amt, ok := resp.Asks[x][1].(float64) - if !ok { - return oba, common.GetTypeAssertError("float64", resp.Asks[x][1], "amount") - } ob.Asks[x] = OrderbookItem{ - Price: price, - Amount: amt, + Price: resp.Asks[x][0].Float64(), + Amount: resp.Asks[x][1].Float64(), } } - for x := range resp.Bids { - bidPrice, ok := resp.Bids[x][0].(string) - if !ok { - return oba, common.GetTypeAssertError("string", resp.Bids[x][0], "price") - } - price, err := strconv.ParseFloat(bidPrice, 64) - if err != nil { - return oba, err - } - amt, ok := resp.Bids[x][1].(float64) - if !ok { - return oba, common.GetTypeAssertError("float64", resp.Bids[x][1], "amount") - } ob.Bids[x] = OrderbookItem{ - Price: price, - Amount: amt, + Price: resp.Bids[x][0].Float64(), + Amount: resp.Bids[x][1].Float64(), } } oba.Data[currencyPair] = ob @@ -163,40 +138,15 @@ func (p *Poloniex) GetOrderbook(ctx context.Context, currencyPair string, depth Asks: make([]OrderbookItem, len(orderbook.Asks)), } for x := range orderbook.Asks { - priceData, ok := orderbook.Asks[x][0].(string) - if !ok { - return oba, common.GetTypeAssertError("string", orderbook.Asks[x][0], "price") - } - price, err := strconv.ParseFloat(priceData, 64) - if err != nil { - return oba, err - } - - amt, ok := orderbook.Asks[x][1].(float64) - if !ok { - return oba, common.GetTypeAssertError("float64", orderbook.Asks[x][1], "amount") - } ob.Asks[x] = OrderbookItem{ - Price: price, - Amount: amt, + Price: orderbook.Asks[x][0].Float64(), + Amount: orderbook.Asks[x][1].Float64(), } } for x := range orderbook.Bids { - priceData, ok := orderbook.Bids[x][0].(string) - if !ok { - return oba, common.GetTypeAssertError("string", orderbook.Bids[x][0], "price") - } - price, err := strconv.ParseFloat(priceData, 64) - if err != nil { - return oba, err - } - amt, ok := orderbook.Bids[x][1].(float64) - if !ok { - return oba, common.GetTypeAssertError("float64", orderbook.Bids[x][1], "amount") - } ob.Bids[x] = OrderbookItem{ - Price: price, - Amount: amt, + Price: orderbook.Bids[x][0].Float64(), + Amount: orderbook.Bids[x][1].Float64(), } } oba.Data[currency] = ob diff --git a/exchanges/poloniex/poloniex_types.go b/exchanges/poloniex/poloniex_types.go index ede3fa8bdb9..0fd4341f381 100644 --- a/exchanges/poloniex/poloniex_types.go +++ b/exchanges/poloniex/poloniex_types.go @@ -5,6 +5,7 @@ import ( "github.com/thrasher-corp/gocryptotrader/currency" "github.com/thrasher-corp/gocryptotrader/encoding/json" + "github.com/thrasher-corp/gocryptotrader/types" ) // Ticker holds ticker data @@ -32,11 +33,11 @@ type CompleteBalances map[string]CompleteBalance // OrderbookResponse is a sub-type for orderbooks type OrderbookResponse struct { - Asks [][]interface{} `json:"asks"` - Bids [][]interface{} `json:"bids"` - IsFrozen string `json:"isFrozen"` - Error string `json:"error"` - Seq int64 `json:"seq"` + Asks [][2]types.Number `json:"asks"` + Bids [][2]types.Number `json:"bids"` + IsFrozen string `json:"isFrozen"` + Error string `json:"error"` + Seq int64 `json:"seq"` } // OrderbookItem holds data on an individual item