Skip to content

WebApplication does not observe changes made with WebApplicationFactory<T> in tests #33876

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

Closed
martincostello opened this issue Jun 26, 2021 · 17 comments · Fixed by #34615
Closed
Assignees
Labels
area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc bug This issue describes a behavior which is not expected - a bug. feature-minimal-hosting Priority:0 Work that we can't release without
Milestone

Comments

@martincostello
Copy link
Member

Describe the bug

When using WebApplicationFactory/WebApplication in conjunction with WebApplicationFactory<T> for integration tests, the WebApplication created by WebApplicationFactory does not observe configuration changes made to the IWebHostBuilder by WebApplicationFactory<T>.

First, if the configuration is changed using ConfigureAppConfiguration(), the changes are not visible in the IConfiguration resolved by the WebApplication. This applies to adding new sources as well as clearing the existing sources. In both cases, the original configuration remains with the built application, and the changes applied for the tests by the WebApplicationFactory<T> are not visible to the application.

Calling serviceProvider.GetServices<IConfiguration>().Count() within the application returns a value of 2. It appears that both two IConfiguration implementations, one for the web host and another for WebApplication are registered with the service provider, with the WebApplication one being resolved by calls to GetService<IConfiguration>() meaning that the changes are not observed.

This can be worked around by using ConfigureServices() to clear the duplicate registration to leave the one required by the tests:

builder.ConfigureServices(services =>
{
    var descriptors = services.Where(p => p.ServiceType == typeof(IConfiguration)).ToList();
    services.Remove(descriptors[1]);
});

Secondly, changes to the content root are not observed by the application. If UseContentRoot() is used to change the directory to serve content by WebApplicationFactory<T>, the change is not visible to the WebApplication instance, which continues to use the path associated with where the test process was launched from.

This causes static content to not be found when requested.

To Reproduce

An application that reproduces this issue is available at https://github.com/martincostello/WebApplicationFactory-Config-Issues. To reproduce the issue:

  1. Clone https://github.com/martincostello/WebApplicationFactory-Config-Issues.git
  2. Run build.ps1

The following test failures are then observed:

  1. The Can_Override_Configuration test fails because the original value of Value is used from configuration ("a") instead of the one overridden by the tests ("b").
  2. The Can_Override_ContentRoot tests fails because a 404 is returned due to the application being unable to find the test.txt file to serve the static content.
  3. The Can_Remove_Configuration test fails because the original value of Value is used from configuration ("a") instead of an empty value of "" because all of the configuration sources were removed.

Running the application under the debugger in Visual Studio 2022 and requesting the /api or /test.txt resources in a browser show the application works as expected when run outside of the test project.

Similarly the Application_Returns_Defaults test also works correctly as no overrides are provided.

Further technical details

  • .NET SDK: 6.0.100-preview.6.21324.1 (I tried to use the latest preview 7 build but had issues running it in Visual Studio 2022)
  • Visual Studio 2022 Preview 1.1
@martincostello
Copy link
Member Author

martincostello commented Jun 27, 2021

I'm still having a dig around trying to work out what's going on with this, but I think it might be to do with the fact that WebApplicationBuilder has its own IWebHostBuilder implementation which isn't the one that the configuration callback in WebApplicationFactory<T> gets (it gets a Microsoft.AspNetCore.Hosting.GenericWebHostBuilder), so the test config sort of gets applied to the wrong thing.

@davidfowl
Copy link
Member

These issues are related to WebApplicationBuilder/WebApplication and might be related to #33082 but @halter73 might be able to pin point as he's working on this area right now.

The same logic works if you use top level statements with the HostBuilder directly:

Host.CreateDefaultBuilder()
    .ConfigureWebHostDefaults(builder =>
    {
        builder.Configure(app =>
        {
            app.UseStaticFiles();
            app.UseRouting();

            app.UseEndpoints(endpoints =>
            {
                endpoints.MapGet("/api", (IConfiguration config, IHostEnvironment environment, IServiceProvider serviceProvider) =>
                {
                    return new
                    {
                        configurations = serviceProvider.GetServices<IConfiguration>().Count(),
                        environment = environment.EnvironmentName,
                        message = config["Message"],
                        value = config["Value"],
                    };
                });
            });
        });
    })
    .Build()
    .Run();

I had a make a couple of tweaks to your tests but they all passed.

@martincostello
Copy link
Member Author

I think I'll stop banging my head against this one for now and see what Stephen makes of it.

@davidfowl
Copy link
Member

@martincostello keep finding these bugs, they are super helpful!

@martincostello
Copy link
Member Author

martincostello commented Jun 27, 2021

I think I've gotten as far as I can with my sample app for minimal actions that's how I've flushed out the various issues I've logged. I'll come back to it again when there's fixes available and/or later official preview builds released.

The changes are here if it helps you with any debugging/investigation: martincostello/dotnet-minimal-api-integration-testing#2

Two outstanding issues I've got that I've not logged yet as they're only apparent after several layers of workarounds and hacks:

  1. The tests always want to seem on ports 5000 and 5001 despite using builder.UseUrls("https://127.0.0.1:0") - I'm not sure if this is because I've broken something, or it's another symptom of this issue. This was a bug in my app's tests.
  2. The Razor page at the root URL no longer renders and just gives a 404 - again, not sure if I broke something or if it's due to the content root not being applied despite the call to UseContentRoot() on the IWebHostBuilder.

@BrennanConroy BrennanConroy added the bug This issue describes a behavior which is not expected - a bug. label Jun 28, 2021
@BrennanConroy BrennanConroy added this to the Next sprint planning milestone Jun 28, 2021
@ghost
Copy link

ghost commented Jun 28, 2021

Thanks for contacting us.

We're moving this issue to the Next sprint planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s).
If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues.
To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

martincostello added a commit to martincostello/aspnetcore that referenced this issue Jul 5, 2021
@rafikiassumani-msft rafikiassumani-msft added the Priority:0 Work that we can't release without label Jul 20, 2021
@davidfowl
Copy link
Member

@martincostello I think I've found the root cause here. Have you looked at filing those other issues?

@martincostello
Copy link
Member Author

Not yet.

Because there's been the other issues around configuration and also I've spotted other PRs related to fixing issues with the hosting environment's content root I wasn't sure if they were just further side-effects of the configuration being incorrect in the final running app.

I was going to circle back to them once there was some thought that the configuration issues should have been resolved, as then either those would resolve themselves, or they'd turn out to be additional issues.

@davidfowl
Copy link
Member

One realization though is that you won't be able to change configuration values if they are read directly before Build:

using Microsoft.EntityFrameworkCore;

var builder = WebApplication.CreateBuilder(args);

// This reads configuration values before there's a chance to mutate them by the test
var connectionString = builder.Configuration.GetConnectionString("Todos") ?? "Data Source=Todos.db";

builder.Services.AddDbContext<TodoDbContext>(o => o.UseSqlite(connectionString));

VS

using Microsoft.EntityFrameworkCore;

var builder = WebApplication.CreateBuilder(args);

// This reads configuration values much later, and they can be observed by tests
builder.Services.AddDbContext<TodoDbContext>(o => o.UseSqlite(builder.Configuration.GetConnectionString("Todos") ?? "Data Source=Todos.db"));

@JunTaoLuo @DamianEdwards this is relevant because the new provider focused overloads we added to EF take the connection string as a parameter value directly and won't be observable by tests.

@davidfowl
Copy link
Member

@martincostello Re-running your tests with my changes I had to make some test changes but only one test fails now, the UseContentRoot one which is related a similar issue but for anything exposed on the IWebHostEnvironment.

@martincostello
Copy link
Member Author

Ah cool - I wonder if that might be related to the issue with the Razor Pages 404 thing I mentioned in the comment further above?

@davidfowl
Copy link
Member

It's not, I've figured out that issue as well. The last piece of the puzzle is that we need to follow the same pattern for the IWebHostEnvironment. The one added by the WebApplicationBuilder isn't being updated to account for changes that happen later, so they aren't observed.

@martincostello
Copy link
Member Author

Interesting, so it's kinda all different cases of the same class of issue?

@davidfowl
Copy link
Member

Interesting, so it's kinda all different cases of the same class of issue?

Yep. Essentially, the WebApplicationBuilder is trying to be the source of truth for IConfiguration, the IServiceCollection, and the IWebHostEnvironment. The problem is that internally, it creates a HostBuilder which creates its own IConfiguration and IServiceCollection so we need to copy data back into the final builder as result.

@davidfowl
Copy link
Member

davidfowl commented Jul 25, 2021

So I think is a general problem using this new WebApplicationBuilder pattern. The fact that you can write code that isn't deferred that can read the state of the builder while it's being built is problematic for the testing pattern that wants to mutate various pieces of state before the application is built. For services this is normal since we have 2 distinct phases, but for config and environment (which comes from config) we allow you to read it before it's "done". This means that its impossible to make decisions based on the IWebHostEnvironment/IHostEnvironment (content root etc) and Configuration before building if you want test server mutations to change behavior before building. Mixing callbacks and imperative code is problematic 🤔 .

var builder = WebApplication.CreateBuilder(args);

if (builder.Environment.IsEnvironment("Testing"))
{
    builder.Services.AddSingleton<IThing, MyTestThing>();
}
else
{
    builder.Services.AddSingleton<IThing, MyProductionThing>();
}

var app = builder.Build(); // The WebApplicationFactory<T> plugs in here

app.MapGet("/", () => "Hello World");

app.Run();

interface IThing { }
class MyTestThing : IThing { }
class MyProductionThing : IThing { }

If the WebApplicationFactory tries to change the environment to to Testing here, it wouldn't work because it's too late and there's no hook that runs early enough.

cc @halter73

@martincostello
Copy link
Member Author

@davidfowl I've pulled this change into my test app with 6.0.100-rc.1.21374.7 and it resolves the issue with IConfiguration 👍

Overall I've got these three issues left with my test app with the latest RC1 build:

  1. Decouple WebApplicationFactory and TestServer #33846 - I'm working around this by using reflection to do what WebApplicationFactory<T> does internally.
  2. WebApplicationFactory<T> does not default to Development environment with minimal hosting #33889
  3. The Razor Pages 404 issue due to the IWebHostEnvironment issue you mentioned yesterday. I can open up a new issue to track this if you need/want one.

@martincostello
Copy link
Member Author

@davidfowl The changes you made in #34794 have fixed issues 2 and 3 above 🥳

@ghost ghost locked as resolved and limited conversation to collaborators Aug 28, 2021
@amcasey amcasey added area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc and removed area-runtime labels Jun 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc bug This issue describes a behavior which is not expected - a bug. feature-minimal-hosting Priority:0 Work that we can't release without
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants