Replies: 1 comment 1 reply
-
|
Hi, there are valid concerns here but ultimately I don't think this is the way to go. Tastytrade has been known to change their API without warning, so live tests help catch those issues. Also, the library is pretty stable so PRs are usually small enough for me to see immediately whether they'd leak credentials or not. I actually don't require tests to pass before merging and often run them locally myself instead. If you don't feel comfortable running them you don't have to! There are also elements that would be significantly harder to mock (streamers). If Tastytrade had a good sandbox system we'd use that but it's pretty limited unfortunately. |
Beta Was this translation helpful? Give feedback.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
-
Hello @Graeme22,
seeing the current testing requiring an active account and the need to have specific symbols in it, on top of the need to configure oauth access, it felt to me quite intrusive, and also raised concerns about security. Any one contributing would need to trust (but verify) that the current testing is save, since access to personal life accounts is given.
Learning about performing unit tests though, I came across the typical advise to rather mock test the code instead of life test against remote api's.
Several reasons are given for that, ...
There might be use for an unrelated test of the tastytrade API but just to look for changes on tastytrade side, general interface correctness, and test data generation / updates. But that does not need to be public, it is just used to update the mock tests of your API code as needed. But that logic does not need to be part of this repository. It would be just an independent helper logic.
To show how it could be implemented I created a sample on my repository
maneum1@2fd227b
The two functions I wrote properly test the get account logic without making remote calls to tastytrade.
What do you think about this?
Thanks
Beta Was this translation helpful? Give feedback.
All reactions