Skip to content

Expose TestServer as public property on RequestBuilder #10396

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

Merged

Conversation

huysentruitw
Copy link
Contributor

Expose TestServer as public property on RequestBuilder.

Addresses #5947

@Tratcher
Copy link
Member

The code seems fine, but there's a problem with the intent. You did this so you could get access to the IWebHost and IServiceCollection. However, when we refactored TestServer for generic host it no longer holds a reference to either of those.

You might need to re-think the larger design here.

@carusology
Copy link

Thanks @huysentruitw for picking up this issue!

@Tratcher, I'm not sure I follow your comment.

When I look at the changeset you linked to, I see that there's still an public IWebHost Host property on L69 both after that changeset as well as in the latest pre-release of v3.0. When I look at the latest pre-release of the IWebHost definition, it still has a reference to the IServiceProvider.

With this changeset, this should complete the chain from RequestBuilder => TestServer => IWebHost => IServiceProvider. Is there something I'm missing? Are the constructors on TestServer that allow the injection of an IWebHostBuilder, and therefore a reference to its IWebHost, going to go away?

@Tratcher
Copy link
Member

New 3.0 apps don't use IWebHost anymore, they've moved to IHost instead (which doesn't have a property on TestServer due to the redesign).

The new 3.0 templates:
https://github.com/aspnet/AspNetCore/blob/master/src/ProjectTemplates/Web.ProjectTemplates/content/WebApi-CSharp/Program.cs

@carusology
Copy link

carusology commented May 21, 2019

Are the current v3.0 preview integration docs reflective of how we should be writing integration tests using the TestServer in ASP.NET Core v3? Assuming so, as long as we do not port our v2.X web application's TEntrypoint to use HostBuilder over WebHostBuilder, we will still have access to the TestServer via the public property on WebApplicationFactory, and the TestServer will still be hydrated with an IWebHostBuilder, and we'll continue to be able to the RequestBuilder that is being updated to expose the TestServer as part of this merge request, right? That certainly seems preferable to porting all of our existing integration tests to use HttpClient as well.

Assuming my above reading is correct, is there some benefit to using HttpClient over RequestBuilder to perform integration tests that we would be missing out on?

@Tratcher
Copy link
Member

Tratcher commented May 21, 2019

The WebApplicationFactory has been updated to work with either IHost or IWebHost. It's useful for testing larger components, especially full MVC apps.

TestServer is designed for unit testing individual middleware.

is there some benefit to using HttpClient over RequestBuilder

HttpClient has an ecosystem of libraries built around it for crafting requests and reading responses. That can be helpful for some scenarios. RequestBuilder is more helpful when you need to seed some of the server side structures directly.

When what you're really after here is the services, we should revisit making those available directly on TestServer and RequestBuilder. We don't need the IHost to do that, we could inject the IServiceCollection into the TestServer constructor.

@huysentruitw huysentruitw force-pushed the expose-testserver-on-requestbuilder branch 2 times, most recently from 7fb3c26 to c94358b Compare May 22, 2019 21:24
@huysentruitw
Copy link
Contributor Author

Anyone fancy to review my updates from 24 days ago? 😁

Copy link
Member

@Tratcher Tratcher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, hadn't noticed the updates. Looks better this time.

@Tratcher Tratcher self-assigned this Jun 15, 2019
@Tratcher Tratcher added this to the 3.0.0-preview7 milestone Jun 15, 2019
@Tratcher
Copy link
Member

@huysentruitw were you going to be able to finish this up?

@huysentruitw
Copy link
Contributor Author

@Tratcher I have a problem with this test when removing the parameter-less constructor for TestServer.

Now I need to pass a serviceProvider to new TestServer(serviceProvider), but it feels a bit weird to pass an empty service provider, while the .UseServer extension method adds the test server to an other service collection...

I think we need to remove this test as it probably demonstrates a usage we don't want to support. Or we could change the UseServer extension method so it takes a lambda instead, so we can do:

UseServer(serviceProvider => new TestServer(serviceProvider));

What do you think?

@Tratcher
Copy link
Member

@huysentruitw Replace the UseServer call with AddSingleton<IServer>(services => new TestServer(serviceProvider));

@huysentruitw huysentruitw force-pushed the expose-testserver-on-requestbuilder branch from c94358b to b4a836d Compare June 25, 2019 20:31
@Tratcher Tratcher merged commit 2508dfc into dotnet:master Jun 27, 2019
@Tratcher
Copy link
Member

Thanks

@carusology
Copy link

Thanks

No, thank YOU. This is a wonderful addition to TestServer that enables fluent API-styled extension methods.

I'm looking even more forward to ASP.NET Core v3!

h/t @Tratcher @huysentruitw

@huysentruitw huysentruitw deleted the expose-testserver-on-requestbuilder branch June 28, 2019 05:21
@amcasey amcasey added the area-hosting Includes Hosting label Jun 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-hosting Includes Hosting
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants