sinonjs / sinon

Test spies, stubs and mocks for JavaScript.
https://sinonjs.org/
Other
9.64k stars 771 forks source link

Improve documentation on Stubs #2062

Closed michaelisvy closed 4 weeks ago

michaelisvy commented 5 years ago

Hi, first of all I'd like to thank you for the great work on Sinon. I've been using it for a few weeks and it helps us a lot having better tests. I would be happy to work on this issue if you're interested.

Is your feature request related to a problem? Please describe. I was discussing with my colleagues about the fact that the sinon documentation is often not enough and that we need to search on the web / go on stackoverflow to get more examples before we can understand things.

Describe the solution you'd like Have better examples for the stub documentation page. Add some examples that would be less theoretical, closer to what we use on a real project. For instance, the example on stubs uses Spies (which will make people confused if they don't know what a Spy is).

I would be happy to draft something out if you are interested.

Describe alternatives you've considered NA

Additional context NA

fatso83 commented 5 years ago

Improvements to our docs are welcome. We have been discussing a different take on the API docs as well, more in line with how the Lodash docs work, but that's a huge undertaking not likely to happen anytime soon. Working beats perfect :-)

For extensive tutorial-like examples, we do have the how-to section, which have been in need of expansion for some time.

michaelisvy commented 5 years ago

thanks @fatso83 for the quick feedback. I see the example in the How to Section for stubs is pretty good. If that's ok for you, we will start by proposing a small contribution so we get used to the process (and you can see if that's suitable to your expectations). I'll be working on it with my friend and colleague @lootster

michaelisvy commented 5 years ago

hi, apology on the delay. I was playing with Sinon and will try to propose some changes by Friday night. Edit: I found the answer to the below question, I've used an afterEach block.

~~Quick question: all the things I've tried with Sinon stubs work well, except in one case: When using Sinon together with Jasmine, I understand that sinon.restore() should always be called before the expect() methods. Otherwise a failure to meet the expect() close will cause the next tests to fail. However, it doesn't work when my stub throws an Error. I need to do expect() first and restore() after.~~ This is shown in the code samples I've created here: https://github.com/michaelisvy/javascript-samples/blob/master/src/test-doubles-sinon/stub/song-discount/songWithDiscountService.spec.js

Great thanks, Michael.

michaelisvy commented 5 years ago

hi @fatso83, I was looking at the code samples in the documentation. I realise that the code samples in the howto section are much better than the ones in the stubs documentation (that is, the stubs link in the documentation section). However, the howto section is harder to find. Would it make sense to refactor the stubs documentation using some examples inspired from the howto section? Also, on top of what is in the howto, we could add the following to the documentation:

Please let me know if that makes sense and I'll be happy to work on a pull request.

Regards, Michael.

michaelisvy commented 5 years ago

hi, I was just wondering if you had any feedback and if you're interested for me to work on a pull request :)

fatso83 commented 5 years ago

Sure ... I'm a bit swamped with a new family situation and such, so I can't go into specifics on this, but I think there should be room for a kind of introduction to Sinon through various use cases, as you mentioned. But that already exists in many places, and we have linked to several, I believe. You can rip of the best parts and distill it into a PR.

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

michaelisvy commented 4 years ago

hello, we are preparing a pull request. Thanks!

On Tue, 29 Oct 2019 at 02:43, stale[bot] notifications@github.com wrote:

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/sinonjs/sinon/issues/2062?email_source=notifications&email_token=AAK7TCOXPWKOY5GX3YOUPT3QQ4XDXA5CNFSM4IFBQBN2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOECN7ACI#issuecomment-547090441, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAK7TCLTNT2ITLH23JO66U3QQ4XDXANCNFSM4IFBQBNQ .

michaelisvy commented 4 years ago

hello, we are preparing a pull request, thanks!

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

fatso83 commented 4 weeks ago

This was fixed in #2152