tradingview-alerts-processor icon indicating copy to clipboard operation
tradingview-alerts-processor copied to clipboard

✅ Adding tests for trading executor

Open thibaultyou opened this issue 2 years ago • 4 comments

thibaultyou avatar Sep 29 '21 13:09 thibaultyou

@mrsegen can I have some feedbacks on this one ?

I have some issues with open handles with Jest since I've played with setInterval function and timers, maybe you have a workaround for it ? (try running npm run test --detectOpenHandles)

thibaultyou avatar Sep 29 '21 13:09 thibaultyou

So far I had some success with at least getting it to pass with jest.useFakeTimers(); atop the file. Not sure yet if it's testing/verifying what's needed with that change. (If I'm remembering correctly... I'm away from the computer now and typing from memory.)

If going with it, check the jest documentation for it. I think I put a jest.useRealTimers(); within an afterEach(). That should be enough to get it passing.

Possible next steps, for subsequent iterations, if it makes sense for what's being tested:

  • Maybe see if it makes sense to do the expect(setTimeout).toHaveBeenCalledWith(this.processTrades, durationOfDelay) - similar to the example on the documentation site.

  • I was having trouble with it wanting setTimeout to be a mock. I wonder if 'use strict' is required for that to work?

  • I separated out the first parameter to a standalone processTrades (plural) function so that the toHaveBeenCalledWith might work.

  • It might be worth testing the durations - since the need to vary by exchange. I wonder if ccxt would eventually try to support that somehow, but if we can fix it here, no reason not to.

leifjones avatar Sep 30 '21 02:09 leifjones

@mrsegen thank you for the advices, your contributions are always highly appreciated 🙏

I'll try using statements in afterEach blocks this may fix the warnings. Testing durations may also be better using toHaveBeenCalledWith that using steps in timers, I'll try this out.

thibaultyou avatar Oct 01 '21 15:10 thibaultyou

So far, when I went deeper with that, and even making tests of the exchange services, I've been running into a strange errors during testing: Something about node thinking SpotExchangeService is undefined in lines such as: class BinanceExchangeService extends SpotExchangeService

Let me know if you run into that, and/or find a way around it. So far I've explored trying to address apparent circular dependencies. I haven't made much progress yet.

leifjones avatar Oct 01 '21 19:10 leifjones