Fix RuntimeError if user code calls patcher.stopall

[nicoddemus]

Fix #137

[blueyed]

Hmm, from #137 (comment) it sounds like it would be better if pytest-mock's mocks could be excluded? But that's maybe a) not trivial, and b) not good in general - likely you should not call stopall if you do not mean it..

[nicoddemus]

likely you should not call stopall if you do not mean it..

Agree, unfortunately there's no public API to check for that; __exit__ uses _is_started(self) to raise that exception, which I would use if it was public. I catching the exception and checking the actual message is safe, but brittle, as the message might change in future versions, so I think this is OK for now.

[craiga]

Thanks heaps for this, but I'm concerned that this might obfuscate other errors.

What would happen if some test called .stopall(), then some other test which relied on pytest-mock's internal mocks was called. Would it be clear what was going on?

[craiga]

This might be a separate issue. It's possible to get into this situation as-is.

[nicoddemus]

Thanks for the review!

I don't think so, as we are not catching any RuntimeError, we only ignore the very specific message... as I commented above, I think this is safe, but might break in future mock releases if they ever change the message. We should also switch to a public API that allows us to check if a patcher has already been stopped, if one is ever introduced.

[coveralls]

Coverage remained the same at ?% when pulling 24bce30 on nicoddemus:release-1.10.2 into 35ade1c on pytest-dev:master.

[tirkarthi]

Just to add to this the RuntimeError was removed in CPython and mock 3.0.0 due to similar concerns raised in #137 . Upstream issue : https://bugs.python.org/issue36366 .

[nicoddemus]

Thanks @tirkarthi for the pointer, created #147 to track it. 👍