Subscription: Test coverage

currency/pairs.go

@@ -52,6 +52,19 @@ func (p Pairs) Strings() []string {
 	return list
 }
 
+// String is a convenience method returning a comma-separated string of uppercase currencies using / as delimiter
+func (p Pairs) String() string {
+	f := PairFormat{
+		Delimiter: "/",
+		Uppercase: true,
+	}
+	l := make([]string, len(p))
+	for i, pair := range p {
+		l[i] = f.Format(pair)
+	}
+	return strings.Join(l, ",")
+}
+
 // Join returns a comma separated list of currency pairs
 func (p Pairs) Join() string {
 	return strings.Join(p.Strings(), ",")

exchanges/stream/websocket_test.go

@@ -10,7 +10,6 @@ import (
 	"net"
 	"net/http"
 	"os"
-	"sort"
 	"strconv"
 	"strings"
 	"sync"
@@ -527,22 +526,18 @@ func TestResubscribe(t *testing.T) {
 	assert.NoError(t, ws.ResubscribeToChannel(channel[0]), "Resubscribe should not error now the channel is subscribed")
 }
 
-func TestAddSubscription(t *testing.T) {
-	t.Fatal("Not implemented, along with others")
-}
-
-// TestRemoveSubscriptions tests removing a subscription
-func TestRemoveSubscriptions(t *testing.T) {
+// TestSubscriptions tests adding, getting and removing subscriptions
+func TestSubscriptions(t *testing.T) {
 	t.Parallel()
-	ws := NewWebsocket()
+	w := NewWebsocket()
 
-	c := &subscription.Subscription{Key: 42, Channel: "Unite!"}
-	require.NoError(t, ws.AddSubscription(c), "Adding first subscription should not error")
-	assert.NotNil(t, ws.GetSubscription(42), "Added subscription should be findable")
+	c := &subscription.Subscription{Key: 42, Channel: subscription.TickerChannel}
+	require.NoError(t, w.AddSubscription(c), "Adding first subscription should not error")
+	assert.Same(t, c, w.GetSubscription(42), "Get Subscription should retrieve the same subscription")
 
-	err := ws.RemoveSubscriptions(subscription.List{c})
+	err := w.RemoveSubscriptions(subscription.List{c})
 	require.NoError(t, err, "RemoveSubscriptions must not error")
-	assert.Nil(t, ws.GetSubscription(42), "Remove should have removed the sub")
+	assert.Nil(t, w.GetSubscription(42), "Remove should have removed the sub")
 }
 
 // TestConnectionMonitorNoConnection logic test
@@ -567,7 +562,7 @@ func TestGetSubscription(t *testing.T) {
 	w := NewWebsocket()
 	assert.Nil(t, w.GetSubscription(nil), "GetSubscription with a nil key should return nil")
 	s := &subscription.Subscription{Key: 42, Channel: "hello3"}
-	w.AddSubscription(s)
+	require.NoError(t, w.AddSubscription(s), "AddSubscription must not error")
 	assert.Same(t, s, w.GetSubscription(42), "GetSubscription should delegate to the store")
 }
 
@@ -577,10 +572,11 @@ func TestGetSubscriptions(t *testing.T) {
 	assert.Nil(t, (*Websocket).GetSubscriptions(nil), "GetSubscription on a nil Websocket should return nil")
 	assert.Nil(t, (&Websocket{}).GetSubscriptions(), "GetSubscription on a Websocket with no sub store should return nil")
 	w := NewWebsocket()
-	w.AddSubscriptions(subscription.List{
+	err := w.AddSubscriptions(subscription.List{
 		{Key: 42, Channel: "hello3"},
 		{Key: 45, Channel: "hello4"},
 	})
+	require.NoError(t, err, "AddSubscriptions must not error")
 	assert.Equal(t, "hello3", w.GetSubscriptions()[0].Channel, "GetSubscriptions should return the correct channel details")
 }
 
@@ -913,48 +909,20 @@ func TestCheckWebsocketURL(t *testing.T) {
 	assert.NoError(t, err, "checkWebsocketURL should not error")
 }
 
+// TestGetChannelDifference exercises GetChannelDifference
+// See subscription.TestStoreDiff for further testing
 func TestGetChannelDifference(t *testing.T) {
 	t.Parallel()
-	web := Websocket{}
-
-	newChans := subscription.List{
-		{Channel: "Test1"},
-		{Channel: "Test2"},
-		{Channel: "Test3"},
-	}
-	subs, unsubs := web.GetChannelDifference(newChans)
-	assert.Implements(t, (*subscription.MatchableKey)(nil), subs[0].Key, "Sub key must be matchable")
-	assert.Equal(t, 3, len(subs), "Should get the correct number of subs")
-	assert.Empty(t, unsubs, "Should get no unsubs")
-
-	for _, s := range subs {
-		s.SetState(subscription.SubscribedState)
-	}
-
-	web.AddSubscriptions(subs)
 
-	flushedSubs := subscription.List{
-		{Channel: "Test2"},
-	}
-
-	subs, unsubs = web.GetChannelDifference(flushedSubs)
-	assert.Empty(t, subs, "Should get no subs")
-	assert.Equal(t, 2, len(unsubs), "Should get the correct number of unsubs")
-
-	flushedSubs = subscription.List{
-		{Channel: "Test2"},
-		{Channel: "Test4"},
-	}
-
-	subs, unsubs = web.GetChannelDifference(flushedSubs)
-	if assert.Equal(t, 1, len(subs), "Should get the correct number of subs") {
-		assert.Equal(t, "Test4", subs[0].Channel, "Should subscribe to the right channel")
-	}
-	if assert.Equal(t, 2, len(unsubs), "Should get the correct number of unsubs") {
-		sort.Slice(unsubs, func(i, j int) bool { return unsubs[i].Channel <= unsubs[j].Channel })
-		assert.Equal(t, "Test1", unsubs[0].Channel, "Should unsubscribe from the right channels")
-		assert.Equal(t, "Test3", unsubs[1].Channel, "Should unsubscribe from the right channels")
-	}
+	w := &Websocket{}
+	assert.NotPanics(t, func() { w.GetChannelDifference(subscription.List{}) }, "Should not panic when called without a store")
+	subs, unsubs := w.GetChannelDifference(subscription.List{{Channel: subscription.CandlesChannel}})
+	require.Equal(t, 1, len(subs), "Should get the correct number of subs")
+	require.Empty(t, unsubs, "Should get no unsubs")
+	require.NoError(t, w.AddSubscriptions(subs), "AddSubscriptions must not error")
+	subs, unsubs = w.GetChannelDifference(subscription.List{{Channel: subscription.TickerChannel}})
+	require.Equal(t, 1, len(subs), "Should get the correct number of subs")
+	assert.Equal(t, 1, len(unsubs), "Should get the correct number of unsubs")
 }
 
 // GenSubs defines a theoretical exchange with pair management
@@ -1050,27 +1018,26 @@ func TestFlushChannels(t *testing.T) {
 	w.GenerateSubs = newgen.generateSubs
 	subs, err := w.GenerateSubs()
 	require.NoError(t, err, "GenerateSubs must not error")
-	for _, s := range subs {
-		s.SetState(subscription.SubscribedState)
-	}
-	w.AddSubscriptions(subs)
+	require.NoError(t, w.AddSubscriptions(subs), "AddSubscriptions must not error")
 	err = w.FlushChannels()
 	assert.NoError(t, err, "FlushChannels should not error")
 	w.features.FullPayloadSubscribe = false
 	w.features.Subscribe = true
 
 	w.GenerateSubs = newgen.generateSubs
 	w.subscriptions = subscription.NewStore()
-	w.subscriptions.Add(&subscription.Subscription{
+	err = w.subscriptions.Add(&subscription.Subscription{
 		Key:     41,
 		Channel: "match channel",
 		Pairs:   currency.Pairs{currency.NewPair(currency.BTC, currency.AUD)},
 	})
-	w.subscriptions.Add(&subscription.Subscription{
+	require.NoError(t, err, "AddSubscription must not error")
+	err = w.subscriptions.Add(&subscription.Subscription{
 		Key:     42,
 		Channel: "unsub channel",
 		Pairs:   currency.Pairs{currency.NewPair(currency.THETA, currency.USDT)},
 	})
+	require.NoError(t, err, "AddSubscription must not error")
 
 	err = w.FlushChannels()
 	assert.NoError(t, err, "FlushChannels should not error")

exchanges/subscription/list_test.go

@@ -0,0 +1,25 @@
+package subscription
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/thrasher-corp/gocryptotrader/currency"
+	"github.com/thrasher-corp/gocryptotrader/exchanges/asset"
+)
+
+func TestListStrings(t *testing.T) {
+	l := List{
+		&Subscription{
+			Channel: TickerChannel,
+			Asset:   asset.Spot,
+			Pairs:   currency.Pairs{ethusdcPair, btcusdtPair},
+		},
+		&Subscription{
+			Channel: OrderbookChannel,
+			Pairs:   currency.Pairs{ethusdcPair},
+		},
+	}
+	exp := []string{"orderbook  ETH/USDC", "ticker spot ETH/USDC,BTC/USDT"}
+	assert.ElementsMatch(t, exp, l.Strings(), "String must return correct sorted list")
+}

exchanges/subscription/store.go

@@ -46,6 +46,9 @@ func (s *Store) Add(sub *Subscription) error {
 // Add adds a subscription to the store
 // This method provides no locking protection
 func (s *Store) add(sub *Subscription) error {
+	if s.m == nil {
+		s.m = map[any]*Subscription{}
+	}
 	key := sub.EnsureKeyed()
 	if found := s.get(key); found != nil {
 		return ErrDuplicate
@@ -55,9 +58,10 @@ func (s *Store) add(sub *Subscription) error {
 }
 
 // Get returns a pointer to a subscription or nil if not found
-// If key implements MatchableKey then key.Match will be used
+// If the key passed in is a Subscription then its Key will be used; which may be a pointer to itself.
+// If key implements MatchableKey then key.Match will be used; Note that *Subscription implements MatchableKey
 func (s *Store) Get(key any) *Subscription {
-	if s == nil {
+	if s == nil || s.m == nil {
 		return nil
 	}
 	s.mu.RLock()
@@ -69,8 +73,10 @@ func (s *Store) Get(key any) *Subscription {
 // If the key passed in is a Subscription then its Key will be used; which may be a pointer to itself.
 // If key implements MatchableKey then key.Match will be used; Note that *Subscription implements MatchableKey
 // This method provides no locking protection
-// returned subscriptions are implicitly guaranteed to have a Key
 func (s *Store) get(key any) *Subscription {
+	if s.m == nil {
+		return nil
+	}
 	switch v := key.(type) {
 	case Subscription:
 		key = v.EnsureKeyed()
@@ -87,14 +93,16 @@ func (s *Store) get(key any) *Subscription {
 }
 
 // Remove removes a subscription from the store
-func (s *Store) Remove(sub *Subscription) error {
-	if s == nil || sub == nil {
+// If the key passed in is a Subscription then its Key will be used; which may be a pointer to itself.
+// If key implements MatchableKey then key.Match will be used; Note that *Subscription implements MatchableKey
+func (s *Store) Remove(key any) error {
+	if s == nil || key == nil {
 		return common.ErrNilPointer
 	}
 	s.mu.Lock()
 	defer s.mu.Unlock()
 
-	if found := s.get(sub); found != nil {
+	if found := s.get(key); found != nil {
 		delete(s.m, found.Key)
 		return nil
 	}
@@ -104,7 +112,7 @@ func (s *Store) Remove(sub *Subscription) error {
 
 // List returns a slice of Subscriptions pointers
 func (s *Store) List() List {
-	if s == nil {
+	if s == nil || s.m == nil {
 		return List{}
 	}
 	s.mu.RLock()
@@ -123,6 +131,9 @@ func (s *Store) Clear() {
 	}
 	s.mu.Lock()
 	defer s.mu.Unlock()
+	if s.m == nil {
+		s.m = map[any]*Subscription{}
+	}
 	clear(s.m)
 }
 
@@ -167,7 +178,7 @@ func (s *Store) Diff(compare List) (added, removed List) {
 
 // Len returns the number of subscriptions
 func (s *Store) Len() int {
-	if s == nil {
+	if s == nil || s.m == nil {
 		return 0
 	}
 	s.mu.RLock()