Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a warning when trying to replace and already replaced function #281

Open
lumaxis opened this issue Aug 15, 2017 · 5 comments
Open

Add a warning when trying to replace and already replaced function #281

lumaxis opened this issue Aug 15, 2017 · 5 comments

Comments

@lumaxis
Copy link
Contributor

lumaxis commented Aug 15, 2017

We just ran into a strange issue where our tests accidentally replaced the same function twice where the second replace then replaced the testdouble function with yet another one. The td.reset() that is then run after each test only undid the second replace which left us still with with a replaced testdouble function for the next test.

I think it would be great if testdouble could help prevent/fix this situation somehow as - to us - it was very non-obvious what was going on and took some time to debug.

@searls
Copy link
Member

searls commented Aug 15, 2017

Oh wow, this is a really bad thing and I had never run into it before, but I completely understand your point. I definitely think we should do this.

@pfleidi
Copy link

pfleidi commented Aug 15, 2017

@searls Thanks for your quick response! Now that we know what we're looking for, we can make sure not to do a multiple replace()s on the same function any more. But for others, it might make sense to communicate that this can cause issues. 😄

@izikl
Copy link

izikl commented Jun 5, 2018

Instead of a warning, as the title suggests, I think it would be better to allow replacing the same method twice but still support reseting to the original method.

@searls
Copy link
Member

searls commented Jun 5, 2018

I am struggling to imagine why someone would want to reset the same method twice. If there's a case for that, I feel like that a more narrowly td.func-scoped reset would probably be a better fit.

In fact, as I've thought more about this, I feel like an error (as opposed to a warning) would be more appropriate, since the behavior folks will experience will be confusing at best and undefined at worst.

@natesilva
Copy link

We ran into this as well. It would be nice if td.replace would throw an error if the function was already stubbed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants