bbgo: fix isNewerOrderUpdate check and tests

pkg/bbgo/activeorderbook.go

@@ -276,7 +276,10 @@ func (b *ActiveOrderBook) Update(order types.Order) {
 
 	// if order update time is too old, skip it
 	if previousOrder, ok := b.orders.Get(order.OrderID); ok {
-		if isNewerOrderUpdate(previousOrder, order) {
+		// the arguments ordering is important here
+		// if we can't detect which is newer, isNewerOrderUpdate returns false
+		// if you pass two same objects to isNewerOrderUpdate, it returns false
+		if !isNewerOrderUpdate(order, previousOrder) {
 			log.Infof("[ActiveOrderBook] order #%d updateTime %s is out of date, skip it", order.OrderID, order.UpdateTime)
 			b.mu.Unlock()
 			return
@@ -387,7 +390,9 @@ func (b *ActiveOrderBook) add(order types.Order) {
 		// if the pending order update time is newer than the adding order
 		// we should use the pending order rather than the adding order.
 		// if pending order is older, than we should add the new one, and drop the pending order
+		log.Infof("found pending order update")
 		if isNewerOrderUpdate(pendingOrder, order) {
+			log.Infof("pending order update is newer")
 			order = pendingOrder
 		}
 

pkg/bbgo/activeorderbook_test.go

@@ -7,37 +7,62 @@ import (
 	"github.com/stretchr/testify/assert"
 
 	"github.com/c9s/bbgo/pkg/fixedpoint"
+	. "github.com/c9s/bbgo/pkg/testing/testhelper"
 	"github.com/c9s/bbgo/pkg/types"
 )
 
 func TestActiveOrderBook_pendingOrders(t *testing.T) {
 	now := time.Now()
+	t1 := now
+	t2 := now.Add(time.Millisecond)
 
-	ob := NewActiveOrderBook("")
+	ob := NewActiveOrderBook("BTCUSDT")
 
 	filled := false
 	ob.OnFilled(func(o types.Order) {
 		filled = true
 	})
 
-	// if we received filled order first
-	// should be added to pending orders
-	ob.Update(types.Order{
+	quantity := Number("0.01")
+	orderUpdate1 := types.Order{
 		OrderID: 99,
 		SubmitOrder: types.SubmitOrder{
 			Symbol:       "BTCUSDT",
 			Side:         types.SideTypeBuy,
 			Type:         types.OrderTypeLimit,
-			Quantity:     number(0.01),
-			Price:        number(19000.0),
+			Quantity:     quantity,
+			Price:        Number(19000.0),
 			AveragePrice: fixedpoint.Zero,
 			StopPrice:    fixedpoint.Zero,
 		},
-		Status:       types.OrderStatusFilled,
-		CreationTime: types.Time(now),
-		UpdateTime:   types.Time(now),
-	})
+		ExecutedQuantity: Number(0.0),
+		Status:           types.OrderStatusNew,
+		CreationTime:     types.Time(t1),
+		UpdateTime:       types.Time(t1),
+	}
 
+	orderUpdate2 := types.Order{
+		OrderID: 99,
+		SubmitOrder: types.SubmitOrder{
+			Symbol:       "BTCUSDT",
+			Side:         types.SideTypeBuy,
+			Type:         types.OrderTypeLimit,
+			Quantity:     quantity,
+			Price:        Number(19000.0),
+			AveragePrice: fixedpoint.Zero,
+			StopPrice:    fixedpoint.Zero,
+		},
+		ExecutedQuantity: quantity,
+		Status:           types.OrderStatusFilled,
+		CreationTime:     types.Time(t1),
+		UpdateTime:       types.Time(t2),
+	}
+
+	assert.True(t, isNewerOrderUpdate(orderUpdate2, orderUpdate1), "orderUpdate2 should be newer than orderUpdate1")
+
+	// if we received filled order first
+	// should be added to pending orders
+	ob.Update(orderUpdate2)
 	assert.Len(t, ob.pendingOrderUpdates.Orders(), 1)
 
 	o99, ok := ob.pendingOrderUpdates.Get(99)
@@ -45,23 +70,9 @@ func TestActiveOrderBook_pendingOrders(t *testing.T) {
 		assert.Equal(t, types.OrderStatusFilled, o99.Status)
 	}
 
-	// should be added to pending orders
-	ob.Add(types.Order{
-		OrderID: 99,
-		SubmitOrder: types.SubmitOrder{
-			Symbol:       "BTCUSDT",
-			Side:         types.SideTypeBuy,
-			Type:         types.OrderTypeLimit,
-			Quantity:     number(0.01),
-			Price:        number(19000.0),
-			AveragePrice: fixedpoint.Zero,
-			StopPrice:    fixedpoint.Zero,
-		},
-		Status:       types.OrderStatusNew,
-		CreationTime: types.Time(now),
-		UpdateTime:   types.Time(now),
-	})
-
+	// when adding the older order update to the book,
+	// it should trigger the filled event once the order is registered to the active order book
+	ob.Add(orderUpdate1)
 	assert.True(t, filled, "filled event should be fired")
 }