search

rspec / rspec-mocks

RSpec's 'test double' framework, with support for stubbing and mocking
https://rspec.info
MIT License
1.16k stars 357 forks source link

Document multiple calls with `allow_any_instance_of` #1510

Closed nycdotnet closed 1 year ago

nycdotnet commented 1 year ago

Hello,

I have attempted to improve the documentation for the multiple calls feature in combination with everyone's friend allow_any_instance_of. It does have some confusing behavior as identified in the docs. I hope that documenting the expected behavior here should at least mitigate confusion when used in combination with multiple call mocks.

Please let me know if you have any questions. I hope this PR is acceptable without having opened an issue first. I believe that this file corresponds with this document: https://relishapp.com/rspec/rspec-mocks/docs/configuring-responses/returning-a-value#specify-different-return-values-for-multiple-calls

Thank you.

nycdotnet commented 1 year ago

Sure. Is it ok if I link to it from here? Something like “See here for considerations when working with legacy code”. Thanks.

JonRowe commented 1 year ago

Its already linked to elsewhere? Do you find it difficult to find the page?

nycdotnet commented 1 year ago

Well since this example didn't exist, I do think it was difficult to find! :-)

I fully understand and respect the desire to keep this functionality compartmentalized since it's specifically for legacy that isn't easy to test otherwise. I think that's a worthy goal because it encourages people to write things in a better way.

My thought here is that this behavior feels more contextually specific to the behavior of and_return, so at least having a link to the legacy feature page with another discouragement would help to clarify how it's intended to work but would reinforce the "you should try to restructure your code if possible to make this not necessary" aka "don't do this" message. I'll put it together that way and see what you think. It's ok to delete the cross reference if you think it's not aligned with your goals.

Thank you!

nycdotnet commented 1 year ago

I am investigating the build failures.

nycdotnet commented 1 year ago

Running script/run_build passes locally now - sorry for the trouble.

I am curious if you have a thought on the links to .feature - should I remove that extension in the markdown or is it resolved in the generation? The link on the real site would not have .feature on it.

Thank you.

nycdotnet commented 1 year ago

@JonRowe sorry to trouble you, but is this PR now satisfactory? Thank you.

JonRowe commented 1 year ago

Thanks for the contribution, apologies it has taken so long, I'm doing some minor rewording and will then get this up on the docs.

nycdotnet commented 1 year ago

@JonRowe excellent - many thanks for the review.