From c6f39af3df86f43b924c9ddbe1dd47396eada6c1 Mon Sep 17 00:00:00 2001 From: Gareth Kirwan Date: Sat, 23 Nov 2024 16:44:57 +0700 Subject: [PATCH] Assets: Fix False positives with IsValid supportedFlag&a is true for every even number, making the test pretty close to meaningless. [This fix](https://github.com/gbjk/gocryptotrader/commit/5b33bc5324e94a03de69c739ba8507e22199742c) was a viable fix maintaining bit shifted iota, but there's no benefit to it at all. Simplifying. --- exchanges/asset/asset.go | 43 +++++++++-------- exchanges/asset/asset_test.go | 88 +++++++++++++++-------------------- exchanges/order/order_test.go | 2 +- 3 files changed, 59 insertions(+), 74 deletions(-) diff --git a/exchanges/asset/asset.go b/exchanges/asset/asset.go index aa03ed4e16c..8e20e59e6dd 100644 --- a/exchanges/asset/asset.go +++ b/exchanges/asset/asset.go @@ -22,37 +22,36 @@ type Items []Item // Supported Assets const ( - Empty Item = 0 - Spot Item = 1 << (iota - 1) + Empty Item = iota + Spot Margin CrossMargin MarginFunding Index Binary + // Futures asset consts must come below this comment for method `IsFutures` + Futures PerpetualContract PerpetualSwap - Futures DeliveryFutures UpsideProfitContract DownsideProfitContract CoinMarginedFutures USDTMarginedFutures USDCMarginedFutures + FutureCombo + LinearContract + // Options asset consts must come below this comment for method `IsOptions` Options OptionCombo - FutureCombo - LinearContract // Derivatives with a linear Base (e.g. USDT or USDC) - All // Must come immediately after all valid assets + // All asset const must come immediately after all valid assets for method `IsValid` + All ) const ( - optionsFlag = OptionCombo | Options - futuresFlag = PerpetualContract | PerpetualSwap | Futures | DeliveryFutures | UpsideProfitContract | DownsideProfitContract | CoinMarginedFutures | USDTMarginedFutures | USDCMarginedFutures | LinearContract | FutureCombo - supportedFlag = Spot | Margin | CrossMargin | MarginFunding | Index | Binary | PerpetualContract | PerpetualSwap | Futures | DeliveryFutures | UpsideProfitContract | DownsideProfitContract | CoinMarginedFutures | USDTMarginedFutures | USDCMarginedFutures | Options | LinearContract | OptionCombo | FutureCombo - spot = "spot" margin = "margin" - crossMargin = "cross_margin" // for Gateio exchange + crossMargin = "cross_margin" marginFunding = "marginfunding" index = "index" binary = "binary" @@ -160,7 +159,17 @@ func (a Items) JoinToString(separator string) string { // IsValid returns whether or not the supplied asset type is valid or not func (a Item) IsValid() bool { - return a != Empty && supportedFlag&a == a + return a > Empty && a < All +} + +// IsFutures checks if the asset type is a futures contract based asset +func (a Item) IsFutures() bool { + return a >= Futures && a < Options +} + +// IsOptions checks if the asset type is options contract based asset +func (a Item) IsOptions() bool { + return a >= Options && a < All } // UnmarshalJSON conforms type to the umarshaler interface @@ -242,13 +251,3 @@ func New(input string) (Item, error) { func UseDefault() Item { return Spot } - -// IsFutures checks if the asset type is a futures contract based asset -func (a Item) IsFutures() bool { - return a != Empty && futuresFlag&a == a -} - -// IsOptions checks if the asset type is options contract based asset -func (a Item) IsOptions() bool { - return a != Empty && optionsFlag&a == a -} diff --git a/exchanges/asset/asset_test.go b/exchanges/asset/asset_test.go index f82129fa4d1..cef070415d3 100644 --- a/exchanges/asset/asset_test.go +++ b/exchanges/asset/asset_test.go @@ -3,17 +3,22 @@ package asset import ( "encoding/json" "errors" + "slices" "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestString(t *testing.T) { t.Parallel() - for a := Item(1); a <= All; a <<= 1 { - assert.NotEmptyf(t, a.String(), "%s.String should return non-empty", a) + for a := range All { + if a == 0 { + assert.Empty(t, a.String(), "Empty.String should return empty") + } else { + assert.NotEmptyf(t, a.String(), "%s.String should return empty", a) + } } - assert.Empty(t, Empty.String(), "Empty.String should return empty") } func TestStrings(t *testing.T) { @@ -53,12 +58,37 @@ func TestJoinToString(t *testing.T) { func TestIsValid(t *testing.T) { t.Parallel() - if Item(0).IsValid() { - t.Fatal("TestIsValid returned an unexpected result") + for a := range All { + if a.String() == "" { + require.Falsef(t, a.IsValid(), "IsValid must return false with non-asset value %d", a) + } else { + require.Truef(t, a.IsValid(), "IsValid must return true for %s", a) + } } + require.False(t, All.IsValid(), "IsValid must return false for All") +} - if !Spot.IsValid() { - t.Fatal("TestIsValid returned an unexpected result") +func TestIsFutures(t *testing.T) { + t.Parallel() + valid := []Item{PerpetualContract, PerpetualSwap, Futures, DeliveryFutures, UpsideProfitContract, DownsideProfitContract, CoinMarginedFutures, USDTMarginedFutures, USDCMarginedFutures, FutureCombo, LinearContract} + for a := range All { + if slices.Contains(valid, a) { + require.Truef(t, a.IsFutures(), "IsFutures must return true for %s", a) + } else { + require.Falsef(t, a.IsFutures(), "IsFutures must return false for non-asset value %d (%s)", a, a) + } + } +} + +func TestIsOptions(t *testing.T) { + t.Parallel() + valid := []Item{Options, OptionCombo} + for a := range All { + if slices.Contains(valid, a) { + require.Truef(t, a.IsOptions(), "IsOptions must return true for %s", a) + } else { + require.Falsef(t, a.IsOptions(), "IsOptions must return false for non-asset value %d (%s)", a, a) + } } } @@ -117,50 +147,6 @@ func TestSupported(t *testing.T) { } } -func TestIsFutures(t *testing.T) { - t.Parallel() - for _, a := range []Item{Spot, Margin, MarginFunding, Index, Binary} { - assert.Falsef(t, a.IsFutures(), "%s should return correctly for IsFutures", a) - } - for _, a := range []Item{PerpetualContract, PerpetualSwap, Futures, UpsideProfitContract, DownsideProfitContract, CoinMarginedFutures, USDTMarginedFutures, USDCMarginedFutures, FutureCombo} { - assert.Truef(t, a.IsFutures(), "%s should return correctly for IsFutures", a) - } -} - -func TestIsOptions(t *testing.T) { - t.Parallel() - type scenario struct { - item Item - isOptions bool - } - scenarios := []scenario{ - { - item: Options, - isOptions: true, - }, { - item: OptionCombo, - isOptions: true, - }, - { - item: Futures, - isOptions: false, - }, - { - item: Empty, - isOptions: false, - }, - } - for _, s := range scenarios { - testScenario := s - t.Run(testScenario.item.String(), func(t *testing.T) { - t.Parallel() - if testScenario.item.IsOptions() != testScenario.isOptions { - t.Errorf("expected %v isOptions to be %v", testScenario.item, testScenario.isOptions) - } - }) - } -} - func TestUnmarshalMarshal(t *testing.T) { t.Parallel() data, err := json.Marshal(Item(0)) diff --git a/exchanges/order/order_test.go b/exchanges/order/order_test.go index 04c0258d44a..2f2b8190595 100644 --- a/exchanges/order/order_test.go +++ b/exchanges/order/order_test.go @@ -55,7 +55,7 @@ func TestSubmit_Validate(t *testing.T) { Submit: &Submit{ Exchange: "test", Pair: testPair, - AssetType: 255, + AssetType: asset.All, }, }, // valid pair but invalid asset {