Skip to content

Conversation

@guardrex
Copy link
Collaborator

Fixes #8917

cc: @seshenoy

@guardrex guardrex requested a review from javiercn October 16, 2018 21:42
@javiercn
Copy link
Member

Can you provide an internal review topic? It's really challenging to review these changes in isolation

@javiercn
Copy link
Member

@guardrex After looking at the changes I don't think this is encouraging the right pattern. It's easier to just do TryAddXXX on startup and setup the services the normal way than rely on configure test services.

Configure test services is there for use in scenarios where the pattern mentioned above can't be used, such as when configuring options for testing. (And even then, you can now use IPostConfigureOptions to get around that in most cases).

The pattern mentioned above (calling plain ConfigureServices and using services.TryAdd in your startup code is the recommended approach to follow when you want to register services that you might need to override for testing).

@guardrex guardrex changed the title Testing on different environments [WIP] Testing on different environments Oct 19, 2018
@guardrex guardrex added the WIP label Oct 19, 2018
@guardrex
Copy link
Collaborator Author

guardrex commented Oct 19, 2018

@javiercn It was the first thing that came to mind: The app loads whatever services and pipeline config it requires for whatever environment it finds itself in depending on its environment (dev, staging, test, prod) ... in the normal way ... in the way that we clearly document in the App startup and Multiple environments topics, then the dev sets the environment for testing using the env var or by setting it in code in the test project the way that I show. That's super-easy to understand and comports well with what we currently document about running apps in various environments.

The pattern mentioned above (calling plain ConfigureServices and using services.TryAdd in your startup code is the recommended approach to follow when you want to register services that you might need to override for testing).

Even now just reading that, it's not super clear to me, so I'll think it over this weekend.

@javiercn
Copy link
Member

@guardrex Also for environment specific setups you can do Configure<>Services in your startup and setup the environment in the WebHostBuilder. Those two approaches are better than relying on ConfigureTestServices

@guardrex
Copy link
Collaborator Author

WRT Configure{Environment}Services ... we do document it, but I'm concerned about showing devs too many ways of handling environments (i.e., not being opinionated enough). The current thrust of doc samples and content is to use if logic. Actually, I have a better idea for this section now that we've discussed it. Let's just punt the reader (via link) over to the Multiple environments topic for that bit. We don't need it here. We just need to point to what we currently have.

WRT controlling the environment for tests, are you just saying to strip out the piece that explains how to set it explicitly in ConfigureWebHost and WithWebHostBuilder? ... just tell the reader to set the environment on the testing server with the env var and leave it at that? If so, that's easy. Then if they prefer to use ConfigureWebHost and/or WithWebHostBuilder approaches, they're welcome to discover them on their own (i.e., IntelliSense).

@guardrex guardrex changed the title [WIP] Testing on different environments Testing on different environments Oct 20, 2018
@guardrex guardrex removed the WIP label Oct 20, 2018
@guardrex
Copy link
Collaborator Author

@javiercn How about this (on the last commit) ...

### Set the environment

If the SUT requires special testing configuration when tests are executed:

* Specify the environment on the test system (for example, set the `ASPNETCORE_ENVIRONMENT` environment variable to a value that reflects a staging or testing environment, such as `Staging` or `Test`).
* Provide code in [Startup](xref:fundamentals/startup) that loads services and configures the request pipeline based on its environment.

For more information, see <xref:fundamentals/environments>.

@javiercn
Copy link
Member

Specify the environment on the test system (for example, set the ASPNETCORE_ENVIRONMENT environment variable to a value that reflects a staging or testing environment, such as Staging or Test).

Why would you do this instead of just calling webhostbuilder.UseEnvironment?

@guardrex
Copy link
Collaborator Author

Why would you do this instead of just calling webhostbuilder.UseEnvironment?

Wouldn't calling UseEnvironment be hard-coding the environment into the app? ... unless you meant in CustomWebApplicationFactory or WithWebHostBuilder ... that's what I did on the 1st commit (but I had the impression that you didn't want to mention those approaches).

Suppose that I configure the app based on its environment in ConfigureServices and Configure based on Development, Staging (testing), and Production. The app reacts to whatever environment it's in. All I have to do is set the environment in the testing environment (i.e., set the ASPNETCORE_ENVIRONMENT env var and call CreateDefaultBuilder). That seems super-simple to understand.

Alternatively, I could have set the environment for the app based on what I showed in the first commit ... builder.UseEnvironment("Staging") (with CustomWebApplicationFactory) or builder.UseSetting("Environment", "Staging"); (WithWebHostBuilder). I tested this approach, and it worked.

@guardrex
Copy link
Collaborator Author

@javiercn Should I close this PR? I have other issues to address on the topic, so we can move on to the next one if there's no need to address the environment-related issue (#8917).

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

Labels

None yet

3 participants