Skip to content

Commit

Permalink
Engine: Integrate IsRunning into order manager interface; enhance web…
Browse files Browse the repository at this point in the history
…socket manager accordingly (thrasher-corp#1337)

* Engine: Fix false test passes for nil OrderManager

TestWebsocketRoutineManagerSetup tests that passing a nil value returns errNilOrderManager;
However that's not actually what would happen when order manager is configured off.

The arguments to setupWebsocketRoutineManager are interface types.
When a nil pointer of interface type is passed, it does NOT equal nil.
nil is a primitive type.
A nil pointer of interface type has the value (nil;type).
See [the Go lang spec](https://golang.org/ref/spec#Comparison_operators):
```
Interface values are comparable. Two interface values are equal if they have identical dynamic types and equal dynamic values or if both have value nil.
```

So that means that whilst this test was passing, because it was sending
in a real nil value and comparing it to a real nil, that's not what
would happen at runtime. At runtime the bot.OrderManager would be a nil
pointer to a concrete type *OrderManager, and so not comparible to nil.

This commit just fixes that oversight, and explains the often
misunderstood mechanics of comparing interface types to nil.

In practical terms this means that the tests assert that the WSRM would
not run without a OM, but in fact it actually would. And panic later.

This commit SHOULD introduce a FAILing test. Sorry if you're bisecting.

* WSM: Fix error on OrderManager not enabled

It's okay for OrderManager to not be enabled; It's configurable.
Remove the WSM setup protection and move it to runtime using IsRunning.

Left the WSM deps so they can be fixed as apporpriate to each.
  • Loading branch information
gbjk authored Sep 11, 2023
1 parent 2b504c8 commit 3218982
Show file tree
Hide file tree
Showing 4 changed files with 8 additions and 10 deletions.
1 change: 1 addition & 0 deletions engine/subsystem_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ type iCommsManager interface {

// iOrderManager defines a limited scoped order manager
type iOrderManager interface {
IsRunning() bool
Exists(*order.Detail) bool
Add(*order.Detail) error
Cancel(context.Context, *order.Cancel) error
Expand Down
9 changes: 6 additions & 3 deletions engine/websocketroutine_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,6 @@ func setupWebsocketRoutineManager(exchangeManager iExchangeManager, orderManager
if exchangeManager == nil {
return nil, errNilExchangeManager
}
if orderManager == nil {
return nil, errNilOrderManager
}
if syncer == nil {
return nil, errNilCurrencyPairSyncer
}
Expand Down Expand Up @@ -287,6 +284,9 @@ func (m *WebsocketRoutineManager) websocketDataHandler(exchName string, data int
}
m.syncer.PrintOrderbookSummary(base, "websocket", nil)
case *order.Detail:
if !m.orderManager.IsRunning() {
return nil
}
if !m.orderManager.Exists(d) {
err := m.orderManager.Add(d)
if err != nil {
Expand All @@ -310,6 +310,9 @@ func (m *WebsocketRoutineManager) websocketDataHandler(exchName string, data int
m.printOrderSummary(od, true)
}
case []order.Detail:
if !m.orderManager.IsRunning() {
return nil
}
for x := range d {
if !m.orderManager.Exists(&d[x]) {
err := m.orderManager.Add(&d[x])
Expand Down
7 changes: 1 addition & 6 deletions engine/websocketroutine_manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,7 @@ func TestWebsocketRoutineManagerSetup(t *testing.T) {
t.Errorf("error '%v', expected '%v'", err, errNilExchangeManager)
}

_, err = setupWebsocketRoutineManager(NewExchangeManager(), nil, nil, nil, false)
if !errors.Is(err, errNilOrderManager) {
t.Errorf("error '%v', expected '%v'", err, errNilOrderManager)
}

_, err = setupWebsocketRoutineManager(NewExchangeManager(), &OrderManager{}, nil, nil, false)
_, err = setupWebsocketRoutineManager(NewExchangeManager(), (*OrderManager)(nil), nil, nil, false)
if !errors.Is(err, errNilCurrencyPairSyncer) {
t.Errorf("error '%v', expected '%v'", err, errNilCurrencyPairSyncer)
}
Expand Down
1 change: 0 additions & 1 deletion engine/websocketroutine_manager_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
)

var (
errNilOrderManager = errors.New("nil order manager received")
errNilCurrencyPairSyncer = errors.New("nil currency pair syncer received")
errNilCurrencyConfig = errors.New("nil currency config received")
errNilCurrencyPairFormat = errors.New("nil currency pair format received")
Expand Down

0 comments on commit 3218982

Please sign in to comment.