note35 / sinon

Standalone and test framework agnostic Python test spies, stubs and mocks (pronounced "sigh-non").
BSD 2-Clause "Simplified" License
13 stars 4 forks source link

Can now call non-pure stub directly #25

Closed jonathan-benn closed 7 years ago

jonathan-benn commented 7 years ago

I moved __get_wrapper from SinonSpy up to SinonBase, and fixed SinonBase.__call__

I also made a small fix to SpyCall... let me know if you want that in a separate fix

Fixes note35/sinon#15

note35 commented 7 years ago

For sure more tests will be better for software quality, however, I think those changes in TestSinonStub.py should be added to TestSinonBase.py. Besides, those changes can even be considered to be removed from TestSinonStub.py if the meaning of them isn't really related to stub.py.

jonathan-benn commented 7 years ago

Hi Kir,

For sure more tests will be better for software quality, however, I think those changes in TestSinonStub.py should be added to TestSinonBase.py. Besides, those changes can even be considered to be removed from TestSinonStub.py if the meaning of them isn't really related to stub.py.

Are you sure that's what you want? Getting a return value from executing the "pure" function only makes sense for the stub, that's why I put the tests in TestSinonStub.py. If I put the tests anywhere else, I'm afraid that future maintainers might have trouble finding them.

Best Regards,

--Jonathan

note35 commented 7 years ago

Are you sure that's what you want? Getting a return value from executing the "pure" function only makes sense for the stub, that's why I put the tests in TestSinonStub.py. If I put the tests anywhere else, I'm afraid that future maintainers might have trouble finding them.

I see, I understand your intention here now. I respect this reasonable decision. Just keeping this test in TestSinonStub.py is fine. Thanks!

jonathan-benn commented 7 years ago

Thanks! The refactor is very nice! Only a few naming issues are required.

I fixed the issues you brought up, and the Pull Request is ready for review.