A more robust test suite

[Inkybro]

Hey,

I was about to start writing a gem for Kraken trading this morning, when I ran across your repository here on GitHub.

I see you were last active about two weeks ago, so I hope you're still working on this, but I'm going to contribute a little by forking and implementing a more robust test suite, as well as fixing any bugs I come across during this implementation.

I will submit a pull request when I am done.

[leishman]

Hey Ethan! That would be awesome. Thank you so much.

A more robust test suite is definitely needed. I have been swamped with other work for the past 2 weeks so I haven't had a chance to get back to it. I'm looking forward to see what you do.

I plan on continuing to build out the Gem as well as the documentation. Any help is greatly appreciated. Thanks again.

[Inkybro]

Any time! Always happy to try and help =]

[Inkybro]

Good deal. Fair warning: I'm a little new to writing specs "properly" (in fact that's why I have trouble with it, I'm always asking things like 'do I really need to test that much?', or, 'how should I structure the tests?', etc.). That's one reason I was excited to see, in your "Future Updates" readme section, that you were in need of more specs. So, if you have any advice, etc. on tests or any problems with what I've done so far, please do let me know.

I haven't submitted a pull request quite yet, as I plan on catching the specs up to the current state of functionality first. However, if you want to see what I've done, you can have a look on my fork. I should have these initial specs + a few new API methods all working, most likely in the next 1-3 days.

P.S. Got specs implemented for nonce generation, and found a much better way of generating them (no Ruby 'sleep' necessary now).

[leishman]

This looks great! Thanks for adding the error raising to the client.

The nonce improvement is very nice. I didn't like the way it was before and I agree it's much better with a lower time resolution.

The test suite looks great. I am not a very experienced tester, so this is a learning experience for both of us. I like the way you have it structured.

The only things that I didn't really understand were all of the expect(result).not_to respond_to ... tests starting around line 125. Is there a reason you think this needs to be tested? Maybe you're thinking of something that I am not, but these seem to be overkill. Is it to ensure that our method isn't querying more than the specified asset pairs? Is this important? Something to think about.

Thanks again for the contributions. I'm looking forward to including your improvements into the project!

[Inkybro]

Concerning the tests starting @ line 125: yes, that is exactly right, although I can still see what you mean there. This is, like I said, something I've continuously had trouble with when trying to get more into writing and using specs. I fall victim to testing something that is (probably) already tested by the third-party in question (i.e. Kraken and their API).

Either way, surely it won't hurt? I'm not certain, but my FEELINGS tell me it is a "better" test suite as a result. It covers more ground. Still, I see what you mean, now that I think about it. If you think we should remove those parts, I am absolutely okay with that. Perhaps I need to go look @ some well-written spec suites and try to gain a better understanding of the typical conventions. I've not researched much into test-driven development, like I said, I've just read quite a few tutorials/articles on it.

Thanks for the kind words. Glad to help, in general, but especially here, since I was planning to port the official Kraken PHP library to Ruby, until I ran across this repo. Very glad to hear you dig the nonce improvements, as that must've been my most significant accomplishment today. Also glad to hear we're learning together here!

Once again, no worries, no thanks necessary! To me, this has been, is, and always will be my fun and my outlet. :)

[leishman]

@Inkybro I'm sorry I have been out of touch for so long. So do you want to put in a pull request for this?

[mhluska]

@Inkybro Is this still getting merged?