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

System.InvalidOperationException : Collection was modified; enumeration operation may not execute. #28

Open
cwigley opened this issue Nov 17, 2016 · 3 comments

Comments

@cwigley
Copy link

cwigley commented Nov 17, 2016

While executing tests in parallel a race condition is hit if two collections try to create\retire a mock of the same type at the same time. For example, one test is creating another is retiring.

System.InvalidOperationException : Collection was modified; enumeration operation may not execute.
Stack Trace:
SomeTest [FAIL]
at System.ThrowHelper.ThrowInvalidOperationException(ExceptionResource resource)
at System.Collections.Generic.Dictionary2.Enumerator.MoveNext() at System.Linq.Enumerable.Any[TSource](IEnumerable1 source, Func2 predicate) at System.Linq.Enumerable.FirstOrDefault[TSource](IEnumerable1 source, Func2 predicate) at Telerik.JustMock.Core.Context.HierarchicalTestFrameworkContextResolver.RepositoryOperationsWeakRef.RetireRepository(Object key, MocksRepository repo) at Telerik.JustMock.Core.Context.HierarchicalTestFrameworkContextResolver.FindRepositoryInOps(RepositoryOperationsBase entryOps, Object entryKey) at Telerik.JustMock.Core.Context.HierarchicalTestFrameworkContextResolver.ResolveRepository(UnresolvedContextBehavior unresolvedContextBehavior) at Telerik.JustMock.Core.Context.MockingContext.ResolveRepository(UnresolvedContextBehavior unresolvedContextBehavior) at Telerik.JustMock.Mock.<Create>b__86[T]() at Telerik.JustMock.Core.ProfilerInterceptor.GuardInternal[T](Func1 guardedAction)

HierarchicalTestFrameworkContextResolver.cs

@tailsu
Copy link
Contributor

tailsu commented Nov 18, 2016

Can you post the source code of SomeTest? Is the JustMock profiler enabled during the test or not?

It seems that one test retires the repository of another before the latter has finished executing. I can't exactly say why this is happening, until I see the test in question. The probable reason is that the repository's entryKey is selected in a way that leads to a conflict, due to a bug in JustMock.

In general, JustMock does not support parallel test execution reliably. JustMock was foremost designed for elevated mocking (i.e. being able to mock everything). But you can't get test parallelization and asynchronous tests at the same time when running elevated. Imagine you mock DateTime.Now. If you have synchronous tests running in parallel, then you can safely assume that the mocking context is thread-local. If you have asynchronous tests running one after another, then you can safely store the mocking context in a global variable. If you want both asynchronous tests and test parallelization, then what happens if multiple tests from multiple threads arrange DateTime.Now to return different values? How do you know which arrangement belongs to which call stack and to which test?

Because support for asynchronous tests is a must and test parallelization is just nice to have, it was decided of course, that JustMock should just strive to reliably support the former at the expense of the latter.

Frameworks like Moq don't have to deal with this problem, because they can only ever create instances of mock objects and therefore can store the mocking context in the mock instance itself. When you're dealing with static mocks or partial mocks, you don't get the luxury of a private field where the context can be safely stored.

Now let's assume that you're not using the profiler (if you were, then you'd have posted a support ticket to Telerik, instead). In case that you use just JustMock Lite and don't care about the profiler. Why, then, doesn't JML store the current mocking context in the mock objects, like Moq? If it did, it could wholly dispense with this complicated context management machinery and support both async tests and test parallelization simultaneously.

The basic reason is that if it did, then it would change the semantics of the library, in particular that of arrangement lifetimes, depending on whether the profiler is enabled or not. The profiler makes sure that arrangement lifetimes are confined to the lifetime of a test. Libraries like Moq can simply tie the arrangement lifetime to the lifetime of the mock objects. JustMock (Lite or not) makes sure that arrangement lifetime is dependent on the test lifecycle, not on the mock objects. After all, it's even possible to have no mock objects in a test, e.g. a test with only static arrangements. You literally have nothing else to tie the lifetime of the arrangements to other than the test's call stack.

Changing the semantics could lead to change in test results, e.g. tests working without the profiler could start failing when the profiler is enabled. Of course, the Lite version could be engineered to wholly ignore the profiler, that is, tests written with JustMock Lite would work exactly in the same way, regardless of the profiler state. That would, however, lead to a discrepancy in behavior between JustMock Lite and regular JustMock, but without the profiler enabled. Another headache, especially for people migrating between JML and JM.

Maybe the most accurate solution would be to always tie the arrangement lifetime of mock objects to their lifetime and tie the lifetime only of static and partial arrangements to the test lifecycle. That would give us the best of both worlds: easy-to-reason and consistent lifetime semantics when running profiler-less and almost the same semantics as today for elevated tests. The only thing that would be lost is that mock objects will no longer be reusable between tests in elevated mode. The latter is a bad idea anyway, so that not that big of a loss and probably nobody does that anyway.

Unfortunately, this looks like a feature request for JustMock 3.

@cwigley
Copy link
Author

cwigley commented Nov 21, 2016

Thank you, I really appreciate the detailed response. To answer some of your questions, this particular instance of the described issue was with tests running under justmock lite without the profiler enabled.

We are a customer of just mock professional as well, but we have been experiencing issues where test order matters. This is the same I believe as

The only thing that would be lost is that mock objects will no longer be reusable between tests in elevated mode.

IE a test not using advanced mocking features creates a mock before a test creating a mock that is utilizing advanced. I opened an incident for this through the telerik portal and it was closed as a known issue.

We have hundreds of developers and thousands of tests and we are trying to get to a place where we are not having intermittent test failures. Unfortunately, for us, running tests in parallel isn't a nice to have it is a must have due to sheer volume. I have been chasing tests that mock the same types and adding them to the same collection, but this isn't something I can keep chasing.

Thanks again and I will keep my eye out for JustMock 3 announcements!

@vnarkar
Copy link

vnarkar commented Jun 24, 2021

Hi Tailsu, We use JustMock version 2019.3.910.4(https://www.telerik.com/support/whats-new/justmock/release-history/justmock-2019-r3). I have been following up on this issue Chris reported and I am still seeing the same behavior.
Do you know if the this version includes the changes related to the above feature request you mentioned?

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

No branches or pull requests

3 participants