Skip to content

Adding ConfigureTestServices#18391

Merged
guardrex merged 5 commits intodotnet:masterfrom
michal-ciechan:patch-1
May 20, 2020
Merged

Adding ConfigureTestServices#18391
guardrex merged 5 commits intodotnet:masterfrom
michal-ciechan:patch-1

Conversation

@michal-ciechan
Copy link
Copy Markdown
Contributor

Fixes #18286

This Issue is now part of #16341

builder.ConfigureServices (At least in AspNetCore 3.1) no longer is run after the Startup.ConfigureServices. Updating the documentation to reflect this, and mentioning of ConfigureTestServices extension method.

`builder.ConfigureServices` (At least in AspNetCore 3.1) no longer is run after the `Startup.ConfigureServices`. Updating the documentation to reflect this, and mentioning of `ConfigureTestServices` extension method.
@Rick-Anderson Rick-Anderson requested a review from guardrex May 20, 2020 02:37
@guardrex guardrex removed their request for review May 20, 2020 10:38
Copy link
Copy Markdown
Collaborator

@guardrex guardrex left a comment

Choose a reason for hiding this comment

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

Thanks for working on this @michal-ciechan.

We need to address this more specifically for the Web Host. The PR is changing the guidance for the Generic Host, which is the recommended host for all apps. ASP.NET Core 2.1 with the Web Host will be supported until mid-2021, but it's not recommended that devs continue to use it.

Since the current paragraph calls out the behavioral change clearly for 3.0 and the Generic Host, I recommend that you leave the current paragraph as it is. Add a new paragraph under it that starts 'For SUTs that use the [Web Host}(xref:fundamentals/host/web-host), ...' and then provide the new information from there.

Updating following from PR comments regarding Generic Host / Web Host
Copy link
Copy Markdown
Contributor Author

@michal-ciechan michal-ciechan left a comment

Choose a reason for hiding this comment

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

I am assuming that the last section changes in are fine regarding Customize WebApplicationFactory are ok considering that the sample uses WebHostBuilder?

Or should the Sample be updated to use HostBuilder (GenericHost)?

Sorry if am completely wrong. Wasn't aware that there is a new Generic Host builder in .NET Core 3!


1. Inherit from `WebApplicationFactory` and override [ConfigureWebHost](/dotnet/api/microsoft.aspnetcore.mvc.testing.webapplicationfactory-1.configurewebhost). The [IWebHostBuilder](/dotnet/api/microsoft.aspnetcore.hosting.iwebhostbuilder) allows the configuration of the service collection with [ConfigureServices](/dotnet/api/microsoft.aspnetcore.hosting.istartup.configureservices):
1. Inherit from `WebApplicationFactory` and override [ConfigureWebHost](/dotnet/api/microsoft.aspnetcore.mvc.testing.webapplicationfactory-1.configurewebhost). The [IWebHostBuilder](/dotnet/api/microsoft.aspnetcore.hosting.iwebhostbuilder) allows the configuration of the service collection with [ConfigureServices](/dotnet/api/microsoft.aspnetcore.hosting.istartup.configureservices) which is executed before the app's `Startup.ConfigureServices`. The [IWebHostBuilder](/dotnet/api/microsoft.aspnetcore.hosting.iwebhostbuilder) allows overriding and modiying registered services in the service collection with [ConfigureTestServices](/dotnet/api/microsoft.aspnetcore.testhost.webhostbuilderextensions.configuretestservices):

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I guess this is correct considering the example and context currently uses Web Host rather than Generic Host?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I can see the sample at CustomWebApplicationFactory uses Generic Host builder in the SUT, but ConfigureWebHost in the test. Is that correct as I can see the ConfigureHost(IHostBuilder builder) virtual method in WebApplicationFactory which I would assume is for Generic Host

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Yes, this only appears in the 2.x part of the doc, so this should be ok.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

... and the sample code below this is 2.x.

If you're saying that the 2.x code is using ConfigureHost (or vice-versa the 3.x code is using ConfigureWebHost), then there might be a problem here. AFAIK, this is all ok given that ...

  • IIRC, we have proper sign-offs from engineering on this content.
  • There haven't been any comments coming in that the code examples/text are wrong between 2.x and 3.x.

I'm 🏃😅 right now on high priority issues, so I'd prefer not to look unless you think there is a problem. If so, let me know ... but I'll need to schedule this for next week.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sorry for my ignorance and this stupid question, but could you expand on "this only appear in the 2.x part of the doc"?

Reason I am asking is because I can see ConfigureWebHost in the 3.x samples, and in the doc @docs.microsoft.com I have ASP.NET Core 3.1 selected.

I think I am missing something.

Either way, if you say it's ok I'll take your word for it 👍😃

Copy link
Copy Markdown
Collaborator

@guardrex guardrex May 20, 2020

Choose a reason for hiding this comment

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

The way these docs work with the version selector on the live site is that there are these markdown lines that set which text is shown. You can see them sprinkled around in the docs ...

::: moniker range=">= aspnetcore-3.0"

3.x content here

::: moniker-end

::: moniker range="< aspnetcore-3.0"

2.x content here

::: moniker-end

When a reader selects the version on the live site, they see the text in the matching version block ("moniker range").

... and sample cross-links in there (usually) have a 2.x segment for 2.x and 3.x for 3.x.

As long as the code is right for that version of the text, it should be ok. We occasionally goof up and show the wrong code or have an incorrect coding approach in a sample app, but the community is pretty quick on issuing a good 💀 death threat 💀 if we don't fix it. 😄

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Awesome thanks for the clarification! Is it worth adding this info to the Contribution Guidelines linked form the root README? (I am assuming the same syntax/logic is used across all docs in this repo?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

It applies to all of Microsoft docs everywhere. I'm not sure how they've decided to handle it company-wide/.NET Foundation-wide.

I've asked about adding a lot of content to the local guidelines (e.g., #2617), but there hasn't been time and/or consensus among the team to do so.

Copy link
Copy Markdown
Collaborator

@guardrex guardrex left a comment

Choose a reason for hiding this comment

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

Looks great @michal-ciechan. Thanks for working on this! I'll update the tracking issue that the work was done.

I'll check on the sample approach for 3.x asap on #18400.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Customize WebApplicationFactory clarification on ConfigureServices

2 participants