-
Notifications
You must be signed in to change notification settings - Fork 10k
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
ConfigureTestContainer not working with GenericHost #14907
Comments
There were reasons... I don't think ConfigureTestContainer was compatible. @javiercn might remember. |
@alefranz thanks for contacting us. In previous versions the reason we had When we moved to generic host, we changed that behavior and now the callbacks are queued in the order in which you register them, so when you do UseStartup the callback gets immediately queued. For that reason, if you are using generic host you shouldn't need the ConfigureTestContainer overload, you can simply call I don't know what work @Tratcher did to make it work for ConfigureTestServices. I think the main reason we didn't do it is because we might have thought it was a much less common use case. If ConfigureTestServices is supported on generic host, @Tratcher is ok with it, and it's not a big change, I'm supportive of a PR to make ConfigureTestContainer work too, if you want to give it a try. I don't think we will fix ourselves as it's not a common scenario and there is an alternative way to achieve the same thing as described above, so this only helps you in migration scenarios. Otherwise, I simply recommend closing the issue. Hope this helps. |
I will investigate the scope of a change to make it work. |
@javiercn I'm having a similar issue and have tried to make the updates that you suggested above. However, I'm having an issue because my Startup.ConfigureContainer method is always being called after my call to IHostBuilder.ConfigureContainer. It doesn't seem to matter what order I set them up with. |
@javiercn I did confirm that if I remove Startup.ConfigureContainer and instead put that code into a call to IHostBuilder.ConfigureContainer then it does appear to all get executed in the right order. |
@roend83 It's impossible for us to diagnose what's going on in your case without a minimal repro project. How DI works with the new container is explained a bit better here: |
AspNetCoreConfigurationIssue.zip |
@roend83 Thanks for the repro. @Tratcher I took a quick look at the repro and @roend83 seems legit. Can you take a look? public static IHostBuilder CreateHostBuilder(string[] args) =>
Host.CreateDefaultBuilder(args)
.UseServiceProviderFactory(new AutofacServiceProviderFactory())
.ConfigureWebHostDefaults(webBuilder => { webBuilder.UseStartup<Startup>(); })
.ConfigureContainer<ContainerBuilder>(builder =>
{
if (!Startup.IsContainerConfigured)
{
throw new Exception("Startup.ConfigureContainer hasn't been called yet'");
}
}); My understanding is that that should never throw as Startup.ConfigureContainer should run before the ConfigureContainer after it, isn't that the case? |
This gets hit first:
And this second:
ConfigureContainer actions are called in the order registered:
Ah, but UseStartup is delayed until ConfigureServices runs.
Trying to fix this would likely require moving the delayed call to UseStartup out of ConfigureServices. Not sure what other consequences that would have. |
I have the exact same issue. Is there a workaround that would allow us to bypass this issue? |
@GODBS I ended up going the super-hacky route, for now. I hope this is fixed in 3.1. internal static Action<ContainerBuilder> IntegTestOverrides = null;
public void ConfigureContainer(ContainerBuilder cb)
{
cb.RegisterModule(new ApiRootModule());
cb.RegisterType<StartupHttpPipeline>().As<IStartupHTTPPipeline>();
IntegTestOverrides?.Invoke(cb);
} |
@cdibbs no changes are planned for 3.1. |
Will a fix come with a later patch to 3.1? Requiring hacks to override dependencies in your tests seems like a pretty undesirable thing to enter LTS with. |
Unlikely. Patching is risky and anything with a viable workaround is unlikely to be patched. |
Take a look at this article for more information about AspNet Core 3.0 integration tests |
As @roend83 mentioned it works when adding third party DI registrations at the time the And the customization of the container during test is here |
The resolution I am using is below. public class TestApplicationFactory : WebApplicationFactory<Startup>
{
protected override IHost CreateHost(IHostBuilder builder)
{
builder.ConfigureContainer<ContainerBuilder>(containerBuilder =>
{
});
return base.CreateHost(builder);
}
} The |
I tried this. Seemed to face the same problem in my implementation... What am i missing? Using 3.1, we're using autofac and calling ConfigureContainer(ContainerBuilder builder) from startup. This seems to execute in the same order... |
All due respect, I am sorry but the provided answers to this issues are simply not satisfactory.
This does not work, as demonstrated in Tratcher's post here: #14907 (comment) @javiercn 's description in #14907 (comment) implies to be the intended behaviour is that None of the technique above are working, meaning that there is no apparent nor easy workaround. I'm excluding @cdibbs 's techniques which prevents testing in parallel. This issue prevents from having a proper way for tests to override dependencies, which is a huge issue for us (and I suspect many other). We have tons of tests doing exactly that and this bug prevents us from moving from 2.1 to 3.1 This is a big show-stopper for anyone who wants to properly test his application. |
I have investigated a fix but unfortunately, as well explained by @Tratcher , to fix this would have potential side effects so I parked my work. |
I agree with @Pvlerick that my workaround doesn't work when you run the tests in parallel (which is the norm). So, is my workaround really "viable?" Maybe not. It sounds like any fix would have side effects, but perhaps that just builds a case for doubling up on the normal amount of testing done for a patch like this. |
@cdibbs exactly. It's an ugly fix to try to make things work temporarily, this is not a solution that will be implemented across projects; I don't see myself explaining to other developers that this is how they should inject theirs mocks from now on, moving from a super-clean solution in 2.1 to a hack in 3.1. We need this addressed badly :-( |
I got it working by implementing custom factory for my container setup. In my case its Autofac. I have tweaked the IServiceProviderFacotry implementation of Autofac library. Code
Test Setuppublic class CustomWebApplicationFactory : WebApplicationFactory where
} |
@ssunkari I haven't tried this yet, but again I don't see this as a viable workaround. @anurse @davidfowl @DamianEdwards any views on this, do you think this can/will be patched? |
Thanks for contacting us. |
You can override container registrations in the following way, using Autofac as example: Configure your container in public static IHostBuilder CreateHostBuilder (string[] args)
{
return Host.CreateDefaultBuilder (args)
.UseServiceProviderFactory (new AutofacServiceProviderFactory())
.ConfigureContainer<ContainerBuilder>(b => { /* configure container here */ })
.ConfigureWebHostDefaults(builder => builder.UseStartup<Startup>());
} Override container registrations using ContainerBuilder in a derived WebApplicationFactory: public class TestWebApplicationFactory : WebApplicationFactory<Startup>
{
protected override IHost CreateHost(IHostBuilder builder)
{
builder.ConfigureContainer<ContainerBuilder>(b => { /* test overrides here */ });
return base.CreateHost(builder);
}
} |
I would replace overriding public class TestWebApplicationFactory : WebApplicationFactory<Startup>
{
....
private Action<ContainerBuilder> setupMockServicesAction;
public TestWebApplicationFactory WithMockServices(Action<ContainerBuilder> setupMockServices)
{
setupMockServicesAction = setupMockServices;
return this;
}
protected override IHost CreateHost(IHostBuilder builder)
{
builder.ConfigureContainer<ContainerBuilder>(containerBuilder =>
{
setupMockServicesAction?.Invoke(containerBuilder);
});
return base.CreateHost(builder);
}
....
} Then, my test method which resides inside an NUnit test fixture which needs to instantiate [TestFixture]
public class CustomExceptionHandlerTests
{
[Test]
public async Task HandleException_WhenApiThrowsException_MustConvertExceptionToInstanceOfProblemDetailsClass()
{
// Arrange phase
...
using var testWebApplicationFactory =
new TestWebApplicationFactory(...)
.WithMockServices(containerBuilder =>
{
// Ensure a mock implementation will be injected whenever a service requires an instance of the
// IGenerateJwtFlow interface.
containerBuilder
.RegisterType<GenerateJwtFlowWhichThrowsException>()
.As<IGenerateJwtFlow>()
.InstancePerLifetimeScope();
});
using HttpClient httpClient = testWebApplicationFactory.CreateClient();
...
}
} One can employ The full source code can be found here . |
Hi @jr01. That solution doesn't work for me:
In AutoFac version 6.2.0 I get UPDATE: Solved through this comment |
Thanks for contacting us. We're moving this issue to the |
Cleanest solution I could achieve based on previous answers: Create overrides: public class TestAutofacModule : Module
{
protected override void Load(ContainerBuilder builder)
{
builder.RegisterType<TestService>().As<IService>();
}
} Create new ServiceProviderFactory: public class TestServiceProviderFactory<T> : IServiceProviderFactory<ContainerBuilder> where T: Module, new()
{
private readonly AutofacServiceProviderFactory _factory = new();
public ContainerBuilder CreateBuilder(IServiceCollection services)
{
return _factory.CreateBuilder(services);
}
public IServiceProvider CreateServiceProvider(ContainerBuilder containerBuilder)
{
containerBuilder.RegisterModule(new T());
return _factory.CreateServiceProvider(containerBuilder);
}
} Use new Factory: var builder = new HostBuilder()
.UseServiceProviderFactory(new TestServiceProviderFactory<TestAutofacModule>())
... |
Based on #51381, this unsurprisingly also affects |
Describe the bug
When using
GenericHost
, in testsConfigureTestContainer
is not executed.To Reproduce
I've added a test in alefranz@282c153#diff-589b0cebe9e796b47e521f0318393df2R194-R208 to reproduce
Expected behavior
The delegate provided with
ConfigureTestContainer
should be executedAdditional context
This happen in 3.x
I'm looking at providing a fix for this but I would probably need some hint on the right place to address this.
Could you also confirm the behavior is not intentional?
@Tratcher I've seen you have worked to add support to
ConfigureTestServices
withGenericHost
in https://github.com/aspnet/AspNetCore/pull/6585/files#diff-48af505b9d348e7e52da534c4590aef1R36 - Do you think it would need a similar logic to address this as well?Thank you,
Alessio
The text was updated successfully, but these errors were encountered: