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

Trouble with class with long property. #113

Open
snaebjorn13 opened this issue Jun 14, 2017 · 8 comments
Open

Trouble with class with long property. #113

snaebjorn13 opened this issue Jun 14, 2017 · 8 comments

Comments

@snaebjorn13
Copy link

I've got a POCO class Company with numerous int and string properties, and one long property. When I run my unit tests (which generate a list of said class using A.ListOf();), they fail with the stacktrace below.

This seems to be somewhat sporadic, as sometimes all tests pass without any trouble.

When I change the long property to int, then it runs without trouble every time.

I know that I can configure a filler as a work around, but it would be nice if I didn't have to.

[xUnit.net 00:00:01.5242592] System.ArgumentException : An item with the same key has already been added. Key: System.Int64
[xUnit.net 00:00:01.5254390] Stack Trace:
[xUnit.net 00:00:01.5269463] at System.ThrowHelper.ThrowAddingDuplicateWithKeyArgumentException(Object key)
[xUnit.net 00:00:01.5270510] at System.Collections.Generic.Dictionary`2.Insert(TKey key, TValue value, Boolean add)
[xUnit.net 00:00:01.5271207] at GenFu.FillerManager.GetFiller(PropertyInfo propertyInfo)
[xUnit.net 00:00:01.5271753] at GenFu.GenFu.SetPropertyValue(Object instance, PropertyInfo property)
[xUnit.net 00:00:01.5272184] at GenFu.GenFu.New(Object instance)
[xUnit.net 00:00:01.5272792] at GenFu.GenFu.BuildList(Type type, Int32 itemCount)
[xUnit.net 00:00:01.5273261] at GenFu.GenFu.ListOfT

@MisterJames
Copy link
Owner

Thanks for the report @snaebjorn13. Would you mind posting the class (or an approximate of it) here that I can use to repro the issue? Or perhaps do a PR with a failing test?

@Prophasi
Copy link

@snaebjorn13 Out of curiosity, have you tried running your tests without parallelism? I just posted issue #117 with the same stack trace, showing parallelism to be the problem. It would explain why it's sporadic for you, too. Parallelism is enabled by default in current versions of xUnit. To disable it, I used this attribute in my test assembly:

[assembly: CollectionBehavior(DisableTestParallelization = true)]

Now that I look at the code again, this makes sense: FillerManager adds default fillers for many types in ResetFillers(); notable in your case, int and string are included, so they're in the filler dictionary by default. However, long isn't, so at runtime GenFu inspects your class and adds it to the fillers dictionary, but with parallelism there's a race condition of multiple tests trying to do it at the same time. I have the same problem, but with Guid?.

As I noted in #117, making _genericPropertyFillersByPropertyType a ConcurrentDictionary solved the problem - I just haven't dived in enough to know if there are unwanted side effects.

@snaebjorn13
Copy link
Author

That worked, thank you!

@stimms
Copy link
Collaborator

stimms commented Oct 25, 2017

So it would seem we have some parallelism issues in GenFu. Unfortunately, the way we configure GenFu by using a static is in opposition to allowing for parallelism. To fix this we'd need to do a pretty significant refactoring and API change. There are probably some smart ways to support both static and private configuration for GenFu but I'm, honestly, leaning towards dropping the static configuration because of these sorts of unintended side-effects. It makes the API a bit less terse but correctness > slick looking,

This would require a 2.0 release.

@DemoBytom
Copy link

Is there any ETA on how long it might take to make GenFu paraller-friendly? And if it's even on the roadmap? My team is considering adopting GenFu for our test suites, but I need to give an estimate for how long our tests wouldn't be able to run in parallel, since it is one of the requirements that we have.

@marcoregueira
Copy link
Contributor

marcoregueira commented Nov 29, 2017 via email

@dpaquette
Copy link
Collaborator

We would very much appreciate a fix if you are able to contribute

@marcoregueira
Copy link
Contributor

marcoregueira commented Nov 30, 2017

Just added a pull request for making type registration thread safe. #133. Check the PR for details.

That would solve #132 too.

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

7 participants