GenFu icon indicating copy to clipboard operation
GenFu copied to clipboard

PropertyFillers dictionary error from concurrent test runs

Open Prophasi opened this issue 7 years ago • 4 comments

When running dotnet test against a xUnit project with 4 total test methods (all of which use GenFu), I get the following error:

Error Message: System.ArgumentException : An item with the same key has already been added. Key: System.Nullable1[System.Guid] Stack Trace: at System.ThrowHelper.ThrowAddingDuplicateWithKeyArgumentException(Object key) at System.Collections.Generic.Dictionary2.Insert(TKey key, TValue value, Boolean add) at GenFu.FillerManager.GetFiller(PropertyInfo propertyInfo) in C:\Users\Jeremy\Dev\x\y\src\GenFu\src\GenFu\FillerManager.cs:line 168

When I run any of the tests in isolation (in my case, within VS Code), I don't get the error; I've commented out various combinations of the test, and while not all throw the exception, it seems clear that it's a concurrency issue. Via console output, I verified that one thread's Guid? entry got added in between the check and insert of a second thread.

Changing _genericPropertyFillersByPropertyType to a ConcurrentDictionary (and the Add(...) calls to TryAdd(...)) resolves the issue for me locally, although I haven't familiarized myself with the code enough to know if there are side effects.

Have you run into this? Is there something simpler I could be doing to dodge this kind of thing? Thanks in advance, and thanks for the handy lib!

Prophasi avatar Jun 19 '17 18:06 Prophasi

As another verification, all 4 of my tests run fine when I add this xUnit attribute to the test assembly:

[assembly: CollectionBehavior(DisableTestParallelization = true)]

I'll do that for now. In the long run, obviously, the parallelism would be good to have. I'm surprised no one else has run into this yet; makes me think that either I missed another pending issue or I'm missing something obvious (I'm pretty new to GenFu, xUnit, and .NET Core all, so it's a very real possibility).

Prophasi avatar Jun 20 '17 14:06 Prophasi

The same issue happens to me. I think they could use a ConcurrentDictionary instead of the default Dictionary implementation.

ghost avatar Jul 06 '17 18:07 ghost

We know that GenFu has some problems with running tests in parallel. Some of this is because we have the GenFu "Filler" configuration setup as static. Even if we fixed the dictionary issue above, we would still have other problems. Each unit test changes the configuration for particular classes which could cause conflicts between tests that are running at the same time. Ideally we would like to introduce the concept of scoped GenFu configurations.

dpaquette avatar Jul 10 '17 21:07 dpaquette

Any traction on this? Couldn't it be solved by simply putting the ThreadStaticAttribute on the field containing a reference to the configurations so each thread gets its own instance? We only have around 200 tests in our current solution and a random one of them fails nearly every test run because of this issue. I can disable parallelization for now but as the number of tests grows, taking advantage of parallelism becomes more and more important.

SonOfPirate avatar Oct 17 '18 18:10 SonOfPirate