Skip to content
This repository has been archived by the owner on Jan 13, 2025. It is now read-only.

Never call BuildServiceProvider #465

Closed
Tratcher opened this issue Oct 6, 2020 · 13 comments
Closed

Never call BuildServiceProvider #465

Tratcher opened this issue Oct 6, 2020 · 13 comments
Assignees

Comments

@Tratcher
Copy link

Tratcher commented Oct 6, 2020

var sp = services.BuildServiceProvider();

This is an anti-pattern that causes issues with the service lifetimes (e.g. it can create multiple instances of singletons).

@javiercn can you suggest a better pattern for consuming services in a WebApplicationFactory fixture?

@javiercn
Copy link

javiercn commented Oct 6, 2020

Ugh, yes.

Just create an IStartupFilter and run the initialization code when it's called, you can inject dependencies to it and it only runs once when the host is being built. You can just leave the pipeline alone at that point.

https://github.com/dotnet/aspnetcore/blob/master/src/DefaultBuilder/src/ForwardedHeadersStartupFilter.cs#L10-L20

@ardalis
Copy link
Collaborator

ardalis commented Oct 6, 2020

@ardalis
Copy link
Collaborator

ardalis commented Oct 6, 2020

Does it help to point out that the serviceProvider in question is only used to seed the database before tests are run, and isn't part of the actual application or the app's service provider? It's literally only needed because EF needs a scope.

@javiercn
Copy link

javiercn commented Oct 6, 2020

It doesn't hurt, but modifying updating the sp and the pipeline is fair game

@Tratcher
Copy link
Author

Tratcher commented Oct 6, 2020

Does it help to point out that the serviceProvider in question is only used to seed the database before tests are run, and isn't part of the actual application or the app's service provider? It's literally only needed because EF needs a scope.

I don't follow. The linked doc also resolves services like an ILogger, creating duplicate singleton logging services in an app which has caused issues in the past. In the average test app this might be harmless, but it's not a pattern we should ever recommend.

@ardalis
Copy link
Collaborator

ardalis commented Oct 6, 2020

Fair enough and good point. We should update it there and here, then.

@fingers10
Copy link

@javiercn or @Tratcher please can you show modified example on how to rewrite code without calling services.BuildServiceProvider() using this eshoponweb project as example?

@Tratcher
Copy link
Author

Tratcher commented Nov 1, 2020

If this were a normal app, not a test fixture, the guidance would be to move that code into Program.Main between Build and Start/Run. That way the service collection has already been built by the host, but the app hasn't fully started yet.

For a WebApplicationFactory test fixture, it looks like you need to override protected virtual IHost CreateHost(IHostBuilder builder) and put this logic between Build and Start.
https://github.com/dotnet/aspnetcore/blob/584e06fcedc7f26f59d19305b68207ec982bd11a/src/Mvc/Mvc.Testing/src/WebApplicationFactory.cs#L371-L376

@adrianiftode
Copy link

adrianiftode commented Dec 19, 2020

@Tratcher overriding CreateHost works only if the API uses the .NET Generic Host, more exactly if Program.cs has the following method

public static IHostBuilder CreateHostBuilder(string[] args) => Host....

If this method exists, then CreateHost is called, and thus overriding this method makes sense. I assume this happens because this is how it finds the host builder, as per these lines of code

Once this is added the problem is I get a totally different issue, instead of resolving the in memory dbcontext, I get my tests running against the database!

adrianiftode pushed a commit to adrianiftode/InvoicingSystem that referenced this issue Dec 19, 2020
Following the suggestion of overriding the CreateHost to resolve
any needed services, I got first into an issue of not actually having
called the CreateHost I've overridden. This seems is called only once
the IWebHostBuilder is changed with IHostBuilder in Program.cs
(dotnet-architecture/eShopOnWeb#465)

After this change, the tests aren't using the InMemory database
anymore, but they actually run against the SqlServer, thus
ConfigureServices or ConfigureTestServices is not used as expected.
@ardalis ardalis self-assigned this Nov 5, 2021
@Blackclaws
Copy link

With .Net 6.0 you can now no longer count on ConfigureWebHost being called at all if you're using top level statements in your main server code. This is because you are now creating a DeferredHostBuilder. This one will still get passed to CreateHost however. You can then configure it as an IWebHostBuilder by manually calling ConfigureWebHost on it.

@ardalis
Copy link
Collaborator

ardalis commented Dec 2, 2021

@Tratcher Do you have any updated guidance on this? I would like to have this repo reflect the latest guidance for .NET 6 but the docs for integration tests still show the pattern used in this repo for configuring and seeding data before tests.

If/when the docs are updated or specific changes are suggested here I'll be happy to update this but for now I'm going to close this issue since I don't see any immediate resolution forthcoming. Thanks!

@Tratcher
Copy link
Author

Tratcher commented Dec 6, 2021

@ardalis I've filed dotnet/AspNetCore.Docs#24204 to address this in the docs.

@ardalis
Copy link
Collaborator

ardalis commented Dec 6, 2021

Thanks, I'm watching it. Once we have concrete guidance on how to perform data seeding without using BuildServiceProvider I'll update these samples accordingly.

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

6 participants