Remove call queue - Githubissues

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

Remove call queue #12

Closed jonathan-benn closed 7 years ago

jonathan-benn commented 7 years ago

Hi Kir,

Please let me know what you think about this refactoring.

Sincerely,

--Jonathan

note35 commented 7 years ago

Sorry for my slowly merge, please re-pull from master again in your branch to solve the conflicts.

jonathan-benn commented 7 years ago

Hi Kir,

I have rebased the branch, please take a look and let me know what you think of the refactoring.

--Jonathan

note35 commented 7 years ago

According to PEP8, variable's name should be normal_variable rather than normalVariable.

jonathan-benn commented 7 years ago

Hi Kir,

Ok, I updated my submission. Please let me know if there are any other issues.

Best Regards,

--Jonathan

note35 commented 7 years ago

Two more thing

one can be fixed later, the name of callId in class SpyCall.

second is putting SpyCall.next_spy_call_id = 0 into sandbox. I know current sandbox is not perfect. And you might spend time on figuring out how to achieve that. To be honest I need time to think about this too, thus I couldn't provide you a good suggestion here. In a word, sandbox is the most important design for making library easy to use. If user need to put some template code into each unit test framework's setUp() function, it completely breaks the design.

Actually, there is a section of scope in document, that part for me is a painful compromise. Since it breaks the design. So far, I didn't do deeper research on scope, but I am pretty sure for SpyCall.next_spy_call_id = 0, it doesn't need to make any compromise but can put it into sandbox.

jonathan-benn commented 7 years ago

Hi Kir,

I'll try moving SpyCall.next_spy_call_id = 0 to sandbox.py.
What is wrong with callId? I thought you wanted to keep the same API as in Sinon.JS? In Sinon.JS, it is SpyCall.callId. However, callId is an undocumented feature that you only discover if you try code like this at the JavaScript command line:

spy = sinon.spy()
spy()
spy.getCall(0).callId

Since it's an undocumented feature, we could just rename it to a more python-like call_id. However, my instinct would be to preserve the same API if possible, even the undocumented parts. What do you think?

note35 commented 7 years ago

About callId, thanks for giving me this idea.

I agree with you, keeping the original code is better. :)

note35 commented 7 years ago

One note here:

Why CALLQUEUE doesn't need to put into sandbox?

Because in sandbox, all inspectors will be destroyed after each test case, then CALLQUEUE will become empty at the same time. But for new implementation, we need to do reset SpyCall in sandbox since it's not directly related to inspectors anymore.

jonathan-benn commented 7 years ago

I have moved SpyCall.next_spy_call_id = 0 to sandbox.py