Skip to content
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

Incorrect guidance on ConfigureServices execution order #16000

Closed
SaebAmini opened this issue Dec 4, 2019 — with docs.microsoft.com · 11 comments · Fixed by #16421
Closed

Incorrect guidance on ConfigureServices execution order #16000

SaebAmini opened this issue Dec 4, 2019 — with docs.microsoft.com · 11 comments · Fixed by #16421
Assignees
Labels
doc-enhancement Pri2 Priority 2 Source - Docs.ms Docs Customer feedback via GitHub Issue

Comments

Copy link

In the section "Customize WebApplicationFactory", it says:

> The SUT's database context is registered in its Startup.ConfigureServices method. The test app's builder.ConfigureServices callback is executed after the app's Startup.ConfigureServices code is executed.

This is incorrect. If you follow the guide with breakpoints, you can see that it's the other way around and the test app's builder.ConfigureServices is run before the real Startup class's method. It's a quite dangerous guidance as well since the "real" services could end up being used for tests rather than the test versions.


Document Details

Do not edit this section. It is required for docs.microsoft.com ➟ GitHub issue linking.

@dotnet-bot dotnet-bot added ⌚ Not Triaged Source - Docs.ms Docs Customer feedback via GitHub Issue labels Dec 4, 2019
@guardrex
Copy link
Collaborator

guardrex commented Dec 4, 2019

Hello @SaebAmini ... Are you using ASP.NET Core 2.2 (or earlier) or 3.0 or later? We made some changes to the topic recently regarding order there, which changed with the 3.0 release. If you set the doc version selector back to 2.2, you'll see that line disappear.

@guardrex guardrex self-assigned this Dec 4, 2019
@guardrex guardrex added this to the Backlog milestone Dec 4, 2019
@guardrex guardrex added this to To do in Testing via automation Dec 4, 2019
@SaebAmini
Copy link
Author

I am using ASP.NET Core 2.2, and you're correct that it does disappear. So is this a breaking change between 2.2 and 3.0? if so, what's the reason for this breaking change?

@guardrex
Copy link
Collaborator

guardrex commented Dec 4, 2019

Stand-by ... I'll see if I can track it down.

@guardrex
Copy link
Collaborator

guardrex commented Dec 4, 2019

Might be explained here where Javier speaks about the new Generic Host (at 3.0) and old Web Host (prior to 3.0) 👉 dotnet/aspnetcore#13918 (comment)

@guardrex
Copy link
Collaborator

guardrex commented Dec 4, 2019

I'll see if there was an announcement on it next.

@guardrex
Copy link
Collaborator

guardrex commented Dec 4, 2019

I don't see a formal announcement ... it was brought to my attention by a member (or two) of the community who were having trouble getting service replacements to work and by Javier who explained it out and assisted with the topic updates. The work was done here 👉 #15096. If you flip that over to its diff, you can see exactly what the changes were, including the lines that you're asking about here.

Your best bet is to work from the 2.2 version of the topic and the 2.x sample app if you're not upgrading to 3.1 (released today 🎉). That 2.2 content and 2.x sample app should apply to your scenario, and ordering should be as you expect.

I'll close as answered, but ping me back here for further discussion if needed.

@guardrex guardrex closed this as completed Dec 4, 2019
Testing automation moved this from To do to Done Dec 4, 2019
@guardrex guardrex removed their assignment Dec 4, 2019
@guardrex guardrex removed this from Done in Testing Dec 4, 2019
@guardrex
Copy link
Collaborator

guardrex commented Dec 4, 2019

Oh ... and the closest that I could find on the two containers to one is the remark on this announcement 👉 aspnet/Announcements#353; but as you can see, it doesn't get into the weeds like Javier's answer on that issue does.

@SaebAmini
Copy link
Author

SaebAmini commented Dec 4, 2019

Wow, thanks so much for your amazingly quick response and great info @guardrex! if it wasn't for your detailed and relevant responses I would have suspected I'm getting automated answers! 😄

Back to the point, while the explanation and links make it clearer, I find this change super confusing, and I think it should be called out clearly in the docs that this has been a breaking change compared to previous versions because it totally flips the flow which could be dangerous.

What's more confusing to me is now I'm not sure whether in 2.2 it is actually meant for this to run before, because there's no mention of the order in the 2.2 version of the official docs, and looking back at other unofficial information, e.g. https://andrewlock.net/converting-integration-tests-to-net-core-3/, there is no mention of this order change, and the sample code is relying on the overriden method to run after (like 3.0).

@guardrex
Copy link
Collaborator

guardrex commented Dec 4, 2019

I can take a look, but there will be a delay. The 🎁 holidays ⛄️ are here, and I'm out for three weeks ... in two weeks WOOT! 😄 I'll schedule this for January.

@guardrex guardrex reopened this Dec 4, 2019
@guardrex guardrex self-assigned this Dec 4, 2019
@guardrex guardrex modified the milestones: Backlog, 2020 Q1 ends Mar 31 Dec 4, 2019
@guardrex guardrex added this to To do in Testing via automation Dec 4, 2019
@guardrex guardrex moved this from To do to Next up in Testing Dec 4, 2019
@scottaddie
Copy link
Member

This issue is closely related to https://github.com/aspnet/AspNetCore.Docs/issues/14973

@guardrex guardrex added this to To do in January 2020 via automation Dec 19, 2019
@Rick-Anderson
Copy link
Contributor

Moved to #16341: Integration tests MASTER issue

Testing automation moved this from Next up to Done Dec 28, 2019
January 2020 automation moved this from To do to Done Dec 28, 2019
@guardrex guardrex reopened this Jan 6, 2020
January 2020 automation moved this from Done to In progress Jan 6, 2020
January 2020 automation moved this from In progress to Done Jan 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc-enhancement Pri2 Priority 2 Source - Docs.ms Docs Customer feedback via GitHub Issue
Projects
No open projects
January 2020
  
Done
Testing
  
Done
Development

Successfully merging a pull request may close this issue.

5 participants