freerange / mocha

A mocking and stubbing library for Ruby
https://mocha.jamesmead.org
Other
1.22k stars 158 forks source link

Consider thread-safety in Mocha #152

Open floehopper opened 11 years ago

floehopper commented 11 years ago

Currently all Mocha state is stored in the Mockery.instance singleton. Perhaps there ought to be a Mockery instance per thread.

I can see how this makes sense e.g. when running multiple tests in parallel using threads. But it might not be desirable if you are trying to test multi-threaded behaviour in a single test. However, I'd probably argue that the latter is an integration test and should not be using mock objects.

floehopper commented 11 years ago

Actually not all Mocha state is stored in the Mockery instance, because for partial-mocks, Mocha modifies the relevant class in order to intercept method invocations. The Ruby class definitions are presumably common to all threads.

floehopper commented 11 years ago

As noted in the jMock documentation:

The root of the problem is trying to use mock objects for integration testing. Mock objects are used for unit testing in the traditional sense: to test units in isolation from other parts of the system. Threads, however, by their very nature, require some kind of integration test. Concurrency and synchronisation are system-wide concerns and code that creates threads must make use of operating system facilities to do so.

However, it ought to be possible to provide equivalents of some of the facilities that jMock provides e.g. DeterministicExecutor, DeterministicScheduler, ThreadingPolicy.

The latter may only be possible when Mocha is constrained by Mocha::Configuration.prevent(: stubbing_method_on_non_mock_object) i.e. when only "pure" mock objects are used and Ruby class definitions are left untouched.

floehopper commented 11 years ago

Note that I think jMock (in its default configuration) raises an exception if an attempt is made to use a stub in a different thread to that in which it was created.

shelmire commented 3 years ago

@floehopper I have some interest in reviving this as part of my upgrade path for a legacy Rails app. We make heavy use of Mocha in our test suite (which is unfortunately riddled with test environment customization) and it seems to be giving us problems, which I suspect are due to thread safety issues. I have not yet looked through the Mocha source code; are there any new thoughts on how you might do this? I can think of some ways to mock classes based on which test is running (tracking some sort of test context) rather than thread id in case a test or code invokes multiple threads but I'm not sure how that would fit into the current mocha framework.

floehopper commented 3 years ago

@shelmire

Hi. Thanks for your interest in Mocha. I had a bit of a go at this a while ago (in #361) in response to some questions from @zenspider who I seem to remember was in a similar position to you. It might be worth asking him what he ended up doing.

While I don't want to discourage you from attempting to make Mocha multi-threaded, as you can see from my last comment on that PR, I think it would be a substantial undertaking, especially as Mocha was originally written without this in mind. In fact, I can't see any reason to have this functionality available except in a legacy codebase, which is why it hasn't been a priority.

I wonder if you might be better off making some more limited changes to Mocha which means it raises an exception when it detects some undesirable thread-related scenarios. You might be able to use that PR as a basis for that.

If you do attempt some work in this area, I will do my best to help you, but I'm unlikely to have a lot of time to spend on it in the near future.