Skip to content
This repository was archived by the owner on Feb 1, 2024. It is now read-only.

Feat/binance exchange ws get trade history/get latest trade cursor #719

Open
wants to merge 17 commits into
base: master
Choose a base branch
from
Prev Previous commit
Next Next commit
avoid testing when short flag is set
tibrn committed Jul 19, 2021
commit 0c41ff34345b93eaccc0be403de649fe2f8ef8ac
1 change: 1 addition & 0 deletions plugins/binanceExchange_ws.go
Original file line number Diff line number Diff line change
@@ -664,6 +664,7 @@ func (beWs *binanceExchangeWs) readOrders(orders []common.PriceLevel, pair *mode

// GetTradeHistory impl
func (beWs *binanceExchangeWs) GetTradeHistory(pair model.TradingPair, maybeCursorStart interface{}, maybeCursorEnd interface{}) (*api.TradeHistoryResult, error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍


symbol, err := pair.ToString(beWs.assetConverter, beWs.delimiter)
if err != nil {
return nil, fmt.Errorf("error converting symbol to string: %s", err)
18 changes: 18 additions & 0 deletions plugins/binanceExchange_ws_test.go
Original file line number Diff line number Diff line change
@@ -21,6 +21,11 @@ func Test_createStateEvents(t *testing.T) {
}

func Test_binanceExchangeWs_GetTickerPrice(t *testing.T) {

if testing.Short() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a little confused about this, why did we add this back?

some context:
when this is here it says don't run it in the CI pipeline. for public APIs such as GetTickerPrice we want to run in the CI pipeline so we should not have this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PS: If the reason is that makeBinanceWs now starts the user stream during construction then we may need a better way to solve that while still allowing us to run the public API tests in the CI pipeline.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I will see how I can make some refactor to use these tests in CI pipeline

return
}

pair := model.TradingPair{Base: model.XLM, Quote: model.BTC}
pairs := []model.TradingPair{pair}

@@ -56,6 +61,10 @@ func Test_binanceExchangeWs_GetTickerPrice(t *testing.T) {

func Test_binanceExchangeWs_GetOrderBook(t *testing.T) {

if testing.Short() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here too, I think we don't need this, see above

return
}

testBinanceExchangeWs, e := makeBinanceWs(emptyAPIKeyBinance)
if !assert.NoError(t, e) {
return
@@ -96,6 +105,11 @@ func Test_binanceExchangeWs_GetOrderBook(t *testing.T) {
}

func Test_binanceExchangeWs_GetLatestTradeCursor(t *testing.T) {

if testing.Short() {
return
}

startIntervalSecs := time.Now().Unix()

testBinanceExchangeWs, err := makeBinanceWs(emptyAPIKeyBinance)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this test pass without using valid API keys? -- I think this test should fail without valid keys but I'm not sure (see comment above about throwing an error when we can't open up the cursor stream)

@@ -130,6 +144,10 @@ func Test_binanceExchangeWs_GetLatestTradeCursor(t *testing.T) {

func Test_binanceExchangeWs_GetTradeHistory(t *testing.T) {

if testing.Short() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

makes sense to put this here because we can't run this in the CI pipeline since it needs API keys

were you able to test this with your own binance API keys?

return
}

testBinanceExchangeWs, err := makeBinanceWs(emptyAPIKeyBinance)

if !assert.NoError(t, err) {