Closed berwyn closed 5 years ago
Hey @berwyn
Thanks for the message. I'll take a look and see if I can work out whats going on.
Nick.
Hey @berwyn
I've spent the last few days looking into the issue you presented. Unfortunately, I don't have any provisions to get a Linux environment setup at this moment in time. Instead, I have tried and tested all variations I could think of that would lead to the issue via Visual Studio (Windows).
However.....nothing.....everything is working as expected.
I also took a look at the Bogus library and in particular the line output in the exception.
this.FinalizeActions[currentRuleSet] = rule;
I thought the currentRuleSet
might be null
, but then an ArgumentNullException
would be thrown instead of NullReferenceException
. Plus, under the hood, Bogus does apply the default
rule set if null
is provided.
As you said, it seems to be leaning towards the workings of .NET on Linux. You seem to have a simple work around, so if you are happy going with that. Otherwise I have no insight at the moment as to why it is happening.
One thought though....do you run your tests in parallel? Could it be a threading issue and some of the dictionaries used to track rules? Could you run your tests individually and see if the exception is still around?
Nick.
Just in case you have any future Linux-related issues, I'm just using Docker and the official .NET images to replicate this.
Anyway, that's a very good point. I'll double-check it, but I recall having the same issue running single tests. Let me verify that!
Hey there,
I was able to reproduce the issue on Ubuntu 16.04 even at the latest commit:
aa19cd0d0ae77045f194c0d1bafba959bc23f277
Author: berwyn <berwyn.codeweaver@gmail.com>
Date: Tue Jan 22 20:48:07 2019 -0500
Fix NullPointer on Linux
Heroku.NET.Tests.Connections.HerokuV3ConnectionTests.TestPostRequestGeneration [FAIL]
Hosting environment: Production
Content root path: /home/brian/test/heroku-dotnet/test/Heroku.NET.Tests/bin/Debug/netcoreapp2.1/
Now listening on: http://0.0.0.0:9095
Failed Heroku.NET.Tests.Connections.HerokuV3ConnectionTests.TestPostRequestGeneration
Error Message:
System.NullReferenceException : Object reference not set to an instance of an object.
Stack Trace:
at System.Collections.Generic.Dictionary`2.TryInsert(TKey key, TValue value, InsertionBehavior behavior)
at Bogus.Faker`1.FinishWith(Action`2 action) in C:\projects\bogus\Source\Bogus\Faker[T].cs:line 376
at AutoBogus.AutoFaker`1.PrepareFinish(AutoGenerateContext context)
at AutoBogus.AutoFaker`1.Generate(String ruleSets)
at Heroku.NET.Tests.Apps.MockApp.Create(String ruleset) in /home/brian/test/heroku-dotnet/test/Heroku.NET.Tests/Apps/MockApp.cs:line 27
at Heroku.NET.Tests.Connections.HerokuV3ConnectionTests.TestPostRequestGeneration() in /home/brian/test/heroku-dotnet/test/Heroku.NET.Tests/Connections/HerokuV3ConnectionTests.cs:line 77
--- End of stack trace from previous location where exception was thrown ---
Total tests: 11. Passed: 10. Failed: 1. Skipped: 0.
Test Run Failed.
Test execution time: 3.2518 Seconds
As @nickdodd79 mentioned, the issue is likely the parallelism with xunit.
Multi-threaded concurrent access to a global static Faker<T>
in MockApp.cs is likely the cause of the issue. As Nick also mentioned, the dictionaries used under the hood are basic Dictionary<T>
and aren't thread-safe. Threading issues are also usually a sign of the flaky behavior that you experienced with tests running okay on OSX and Windows, but not Linux probably due to differences in thread-scheduling.
So, there are a couple of solutions for us.
1) Use a Lazy<T>
for thread-safe initialization of the global static Faker<T>
object.
2) Create a dedicated instance of Faker<T>
for each test-request of a Faker<T>
I'd probably recommend the 2nd option because you might want to mutate the Faker<T>
more specifically later down the road for some specialized test and you don't want those mutations leaking into other tests as they would with a mutated global static object.
Details are below:
Use Lazy<T>
to create the global static.
internal static class MockApp
{
private static Lazy<Faker<App>> lazyFaker = new Lazy<Faker<App>>(CreateAutoFaker, isThreadSafe:true);
private static Faker<App> CreateAutoFaker()
{
var _faker = new AutoFaker<App>()
.RuleFor(a => a.GitUrl, f => f.Internet.Url())
.RuleFor(a => a.CreatedAt, f => f.Date.Past())
.RuleFor(a => a.UpdatedAt, f => f.Date.Recent())
.RuleFor(a => a.ArchivedAt, () => null);
_faker.RuleSet("archived", rules =>
{
rules.RuleFor(a => a.ArchivedAt, f => f.Date.Recent());
});
return _faker;
}
internal static App Create(string ruleset = "default")
{
return lazyFaker.Value.Generate(ruleset);
}
}
Dedicated object for each request for a Faker<T>
. Might be better to just return Faker<App>
instead of App
so you can mutate Faker<T>
a little more for specialized unit tests.
internal static class MockApp
{
internal static App Create(string ruleset = "default")
{
var _faker = new AutoFaker<App>()
.RuleFor(a => a.GitUrl, f => f.Internet.Url())
.RuleFor(a => a.CreatedAt, f => f.Date.Past())
.RuleFor(a => a.UpdatedAt, f => f.Date.Recent())
.RuleFor(a => a.ArchivedAt, () => null);
_faker.RuleSet("archived", rules =>
{
rules.RuleFor(a => a.ArchivedAt, f => f.Date.Recent());
});
return _faker.Generate(ruleset);
}
}
Also, check out the .Clone()
method on Faker<T>
for some more interesting test scenarios. :sunglasses:
And when you make the fix you should have some good success :tada:
...
Total tests: 11. Passed: 11. Failed: 0. Skipped: 0.
Test Run Successful.
Test execution time: 3.1639 Seconds
brian@linux:~/test/heroku-dotnet/test/Heroku.NET.Tests$
Hope that helps!
Thanks,
Brian
:car: :blue_car: "Let the good times roll..."
Hey @berwyn
The expert insights provided by @bchavez above, I think, has pinpointed where your issue lies. I hadn't noticed that the Faker
was being defined statically. In unit tests, shared context between each test can lead to anomalies and unexpected behaviours.
From the options above, I would also go for 2
as it means each test is isolated and run from a baseline.
Also, thanks for clarifying an approach for running in Linux via Docker. Unfortunately, this was my blocker as I couldn't get Docker for Windows to start up. I am however rebuilding another laptop so fingers crossed it works on there.
Thanks also to @bchavez for providing your input.
Nick.
Thanks for that excellent breakdown @bchavez, this is my first foray into xunit and I hadn't even thought about the concurrent access! Clearly this is a problem with my use of AutoBogus and not the library itself, so I'll go ahead and close the issue.
Thanks both for taking the time to help out!
I have a project that I'm building and I'm using Bogus + AutoBogus in my test suites. I personally develop on macOS and Windows and haven't experienced the issue on either, but when I use Ubuntu on Azure Pipelines, calling
AutoFaker<T>.Generate(null)
(orGenerate()
for that matter, they amount to the same),Generate()
throws:If, instead, I use
Generate("default")
to accomplish the same effet, everything works as expected and as they do on macOS and Windows. I'm not sure what the difference could be, unless MSCorlib has some weird implementation detail on Linux that makes the null string a problem.You can see this in action on my current working branch where
b3a5946
exhibits the issue andaa19cd0
fixes it.I'm using .NET Core 2.2.102 on macOS 10.14.1, Windows 10 1809, and Ubuntu 16.04.