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

Multiple TestServer instances seem to be sharing a context / seem to be pointing to the same server. #1541

Closed
S4lt5 opened this issue Jun 3, 2016 · 15 comments

Comments

@S4lt5
Copy link

S4lt5 commented Jun 3, 2016

Hello. I have a number of Integration Tests that use a shared TestServer instance and a TestStartup class extended from Startup that contains a Seed() method and uses InMemoryDatabase.

In dnx / RC1, all of my tests passed without issues.

I have a subset of the tests that launch a separate (clean) server and tests initial database values before the rest start fiddling with things.

When I upgraded to RC2, I use this to launch my test server

Server = new TestServer(new WebHostBuilder().UseContentRoot(System.IO.Directory.GetCurrentDirectory()).UseStartup<TestStartup>());

But what I find now is that if i run my tests by themselves, they pass, but when I run them together simultaniously, my separate TestServers get the same contexts.

Is there a way I can scope my Context such that I get a new one for each TestServer instance?
I'm getting my context in the Seed() -- called from Configure() like so:

var db = app.ApplicationServices.GetService<ApplicationDbContext>();

and it seems somewhat obvious that my app already has a context and returns the same instance

@S4lt5
Copy link
Author

S4lt5 commented Jun 3, 2016

I guess a better question would be: Can I spawn a completely separate TestServer to check the initial values? Depending on which tests run first, this fails or succeeds

@davidfowl
Copy link
Member

Each WebHostBuilder instance should be isolated. If you can provide a basic repro application we can take a look.

/cc @Tratcher @JunTaoLuo

@S4lt5
Copy link
Author

S4lt5 commented Jun 6, 2016

Allright, Will do. Just put it in a repo here and link it?

@Tratcher
Copy link
Member

Tratcher commented Jun 6, 2016

Yes please

@S4lt5
Copy link
Author

S4lt5 commented Jun 6, 2016

https://github.com/Yablargo/SharedContextRepro

There are two test classes, they create separate server instances. The TestServerInstance is straight out of my actual tests and worked in dnx/rc1. When I run the tests, my Seed() method gets called 3 different times across 2 instances. It seems to be the same context shared between two instances.

In my real tests, 90% of my tests use the same instance, the others create their own. All of the tests seem to share the same context regardless.

@Tratcher
Copy link
Member

Tratcher commented Jun 6, 2016

Sharing a TestServer instance statically across tests is not a good idea for unit tests. There's too much of a risk that the tests can have side-effects on eachother. Note xunit can execute tests in parallel, but by default it will only do this for tests in different classes.

I doubt you're getting the same DbContext instance, but you may be connecting to the same in-memory database. @bricelam does UseInMemoryDatabase create a single static instance in memory that may be inadvertently shared across unit tests?

@S4lt5
Copy link
Author

S4lt5 commented Jun 6, 2016

@Tratcher the seed data in reality is setup so that all of the tests don't overlap with eachother. I do quite a lot of crunching across a lot of tests and got it so that none of them step on eachother's toes, with the exception of the few that want to spawn their base instance and check the fresh data.

That makes more sense on your 2nd point -- any way I can get a 2nd instance explicitly?

I will say that, if I had it to do all over I'd have separated them out completely, but i spent a LOT of time getting all of the integration tests to work in parallel without any overlap. I hope I can get this figured out because I have a fantastic use case where I can use this in production since my RC2 app is basically preparing a distribution of a regular ole 462 app so I can go through all of the process of getting RC2 to work in a real production environment and get ahead of the curve as far as the new stack

Also, I want to note that it worked flawlessly in RC1 this way and my test run time was amazingly quick (Compared to what I am used to dealing with). I know there is more to it but my time for test runs in visual studio from rc1-rc2 feels like it got slower by a factor of 10 at least

@JunTaoLuo
Copy link
Contributor

JunTaoLuo commented Jun 7, 2016

Yup, this seems to be an issue with EF.

"Typically, EF creates a single IServiceProvider for all contexts of a given type in an AppDomain - meaning all context instances share the same InMemory database instance."

Seems like EF's in memory database is by default only one instance per AppDomain. Just as a sanity check, I converted the app to 1.0.0 (which has this fix dotnet/efcore#3253) allowing me to do something like:

            services.AddEntityFramework().AddDbContext<ApplicationDbContext>(options =>
                 options.UseInMemoryDatabase($"db{System.Threading.Thread.CurrentThread.ManagedThreadId}"));

This allows each test to have a unique database specified by its name using a thread id. With this fix, all tests passed when run in parallel. I'm not sure how this can be accomplished with EF RC2 though.

https://docs.efproject.net/en/latest/miscellaneous/testing.html seems to have some documentation for writing tests using EF's InMemoryDatabase so I'd take a look there.

@GeorgDangl
Copy link

Here's how I do it in RC2:

var services = new ServiceCollection();

services.AddEntityFrameworkInMemoryDatabase()
    .AddDbContext<MyDbContext>(x => x.UseInMemoryDatabase().UseInternalServiceProvider(services.BuildServiceProvider()));

var serviceProvider = services.BuildServiceProvider();
Context = serviceProvider.GetRequiredService<MyDbContext>();`

Special note here to .UseInternalServiceProvider(services.BuildServiceProvider()). It's comment says that "EF will create and manage a service provider if none is specified", which I think means that one service provider will be shared for the current AppDomain, meaning all InMemory databases get the same data store.

Here, I'm essentially creating a new service provider for the EF dbContext on each class initialization so I have separate data storages accross tests.

@S4lt5
Copy link
Author

S4lt5 commented Jun 28, 2016

@GeorgDangl Thanks. I'm trying this change now and will be pretty stoked if it fixes everything. I probably won't be moving to 1.0 for a few weeks while we still tie down everything stability-wise

@S4lt5
Copy link
Author

S4lt5 commented Jul 11, 2016

I'm using RTM now, and while this has fixed my 'shared-instance' issue, there are definately some bugs floating around still with this.

The upshot: most of my tests run synchronously.

The negative: The database instances appear to share the same primary key sequence between the various databases that are created at the same time.

I have a twofold problem at the moment:

if in my seed I use:

db.Things.Add(new Thing{whatever="foo"});

the new 'Thing' will have a seemingly random ID such as 5 or 6, depending on the number of concurrent databases exist, so I can't expect a particular version of my seed data.

If I explicitly set an Id value:

db.Things.Add(new Thing{whatever="foo", ID = 1});

When my controllers normally do a

db.Things.add(....)

it fails, and complains that there is an id conflict on ID 1, since my server's identity counter is at 1 (it doesn't check to see if this ID already exists)

@davidfowl
Copy link
Member

/cc @rowanmiller

@S4lt5
Copy link
Author

S4lt5 commented Jul 12, 2016

After some fixes yesterday, nearly everything works, with the quirk of the shared-sequences referred in the previous post.

What I ended up doing is seeding all of my values at arbitrary ID's (e.g. 1000,1001, etc), and the auto inserts start at 1,2,3 and there is not a conflict.

@ielcoro
Copy link

ielcoro commented Jul 20, 2016

I see the same behaviour as @Yablargo with sequences. Looks like they are all shared between InMemory instances.

@aspnet-hello
Copy link

This issue is being closed because it has not been updated in 3 months.

We apologize if this causes any inconvenience. We ask that if you are still encountering this issue, please log a new issue with updated information and we will investigate.

@ghost ghost locked as resolved and limited conversation to collaborators Dec 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants