-
Notifications
You must be signed in to change notification settings - Fork 10.3k
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
Add a new UseKestel API to the WebApplicationFactory #60635
base: main
Are you sure you want to change the base?
Conversation
This is an amazing idea! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Overview
This PR introduces an alternative design for enabling real server usage with WebApplicationFactory by adding a new API that leverages Kestrel. Key changes include:
- Adding a new test class (RealServerBackedIntegrationTests.cs) that validates the real server integration.
- Introducing a new WebApplicationFactory subclass (KestrelBasedWapFactory.cs) that automatically enables Kestrel.
- Updating the core WebApplicationFactory implementation to conditionally use a Kestrel server versus the traditional TestServer.
Reviewed Changes
File | Description |
---|---|
src/Mvc/test/Mvc.FunctionalTests/RealServerBackedIntegrationTests.cs | New integration tests for real server backed functionality. |
src/Mvc/test/Mvc.FunctionalTests/KestrelBasedWapFactory.cs | A factory that configures the WebApplicationFactory to use Kestrel. |
src/Mvc/Mvc.Testing/src/WebApplicationFactory.cs | Modifications to support the new UseKestrel API, including updated server creation, client configuration, and error handling. |
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Throw if the UseKestrel() is called after server initialization
src/Mvc/test/Mvc.FunctionalTests/RealServerBackedIntegrationTests.cs
Outdated
Show resolved
Hide resolved
- Allow configuring Kestrel options.
…st startup model.
{ | ||
// Have to do this, as the ClientOptions.BaseAddress will be set to point to the kestrel server, | ||
// and it may not match the original base address value. | ||
client.BaseAddress = ClientOptions.BaseAddress; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this potentially confusing that the options are being ignored?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one is a bit tricky. I thought about this a bit and there are two paths I could think of:
- We rely on the ClientOptions to be populated correctly by the client. This would allow us to avoid thsi reassignment. Unfortunately, it won't work in the
dynamic port assignment
scenario as the port for the server will be assigned only after it starts. - We override the value - the current solution. This accounts for the
dynamic port assignment
scenario and we update the port of the client options once the server starts (see theTryExtractHostAddress
method). Because many users callCreateClient
(or a variation of it) without explicitly starting the server, the CreateClient call will try to initialize the server and the value in the ClientOptions can be outdated. Once the server started, and before the client is created, we update the BaseAddress in ClientOptions. However, because the ClientOptions.BaseAddress have been passed as a parameter during this call stack, the old value is being set for the created client in CreateDefaultClient method. Hence, we have to override it with the updated value.
UseKestrel(...)
APIs to the WebApplicationFactory type. The API configures the WAF, so that later during initialization it will use Kestrel, instead of a TestServer for WebHostBuilder-based applications.EnsureServer()
private method toStartServer()
and made it public, so that consumers can call it directly, without the need of creating a client, as in many situations customers didn't need a client to interact with.This is an alternative design to enabling real server usage with WebApplicationFactory, which was considered here: #60247
Fixes #4892