dpeachpeach / kalshi-rust

Kalshi trading API client/wrapper written in rust.
https://docs.rs/kalshi/0.9.0/kalshi/
MIT License
17 stars 5 forks source link

Doctest examples fail to compile #7

Closed matteosantama closed 10 months ago

matteosantama commented 10 months ago

Running cargo test results in many failed doctests. Many of them are only partial snippets and need some setup code. Using the no_run tag also doesn't seem to work because the async functions won't compile (perhaps they need to be called in async blocks).

In the meantime, we can skip the doctests entirely with Cargo.toml (see here). I will update Cargo.toml and reference this issue as a tracker issue.

dpeachpeach commented 10 months ago

I recall earmarking this issue on a notepad while working on the cargo tests.

The problem AFAIK is that most of the methods tied to the kalshi struct require an authenticated user. I was hesitant to involve my credentials or specific 'anonymous' credentials for the tests as that would raise a serious security issue (Someone having access to my or a spare key could take actions to disrupt the exchange).

I wanted to include something helpful in the docs to show by example but I didn't want any doctests to run. I didn't do enough research on how to skip the doctests at the time but it looks like you solved it!

Great insight + great contribution, feel free to add this in your PR / create one specifically for this issue or you can let me know so I can change the line myself and push.

matteosantama commented 10 months ago

Yeah, I think the steps to a solution are

  1. Add necessary setup code to each snippet (which can be hidden by prepending the line with #)
  2. Adding no_run to the blocks that would require authentication. That way we check it compiles, at least.
  3. Figure out how to handle the async calls. Not quite sure what the best thing to do here is
dpeachpeach commented 10 months ago

Sure! Let's table this for now but include the no tests temporary fix.

Full disclosure: Right now I'm aiming for a 0.10.0 cargo release to squash some bugs / complete the fully featured RESTful API wrapper. (Which I plan to finish this weekend).

However, one big feature missing which I promised is websocket support, which I need to study up on and learn how to implement in Rust. (I would prefer to do this major part myself as this crate has largely been a learning experience for me).

We can save taking care of this bug for the 1.0.0 release which should be fully featured in all aspects (Websocket + restful). Since I plan to add a lot more doctest examples at that time, the permanent fix you're suggesting would be best be implemented then. The temporary fix of disabling cargo tests for now is sufficient.

matteosantama commented 10 months ago

Sounds good. Funny enough, I've been working on a WebSocket implementation myself for the last few weeks. It's functional, but probably not ready for public release yet. Let's see what you come up with and perhaps we can merge the best parts of both!

dpeachpeach commented 10 months ago

Sure thing @matteosantama! I actually was planning to do a decent amount of work this weekend, I can @ you whenever I complete it or get close and you can check it out if you're interested.

I checked out the pr and I'm going to check everything and then merge.