Add support for configuration settings per environment #84
Conversation
Co-authored-by: acraven <9272163+acraven@users.noreply.github.com>
Co-authored-by: acraven <9272163+acraven@users.noreply.github.com>
Co-authored-by: acraven <9272163+acraven@users.noreply.github.com>
| private string _originalDirectory; | ||
|
|
||
| [SetUp] | ||
| public void SetUp() |
There was a problem hiding this comment.
rename to setup_before_each_test
There was a problem hiding this comment.
Renamed to setup_before_each_test in c6a9a4c
| } | ||
|
|
||
| [TearDown] | ||
| public void TearDown() |
There was a problem hiding this comment.
rename to teardown_before_each_test
There was a problem hiding this comment.
Renamed to teardown_after_each_test in c6a9a4c
| { | ||
| // Get the application name prefix by creating a temporary host | ||
| // This is needed because the prefix is based on the application name with dots removed | ||
| var tempHost = new HostBuilder() |
There was a problem hiding this comment.
Just name this host. But as with each other instance, can you use using var host instead?
There was a problem hiding this comment.
Changed to use 'using var host' pattern throughout all tests in c6a9a4c
| [Test] | ||
| public void should_load_base_appsettings_json() | ||
| { | ||
| // Arrange |
There was a problem hiding this comment.
We don't usually name Arrange, Act and Assert blocks
There was a problem hiding this comment.
Removed Arrange, Act, and Assert comments from all tests in c6a9a4c
|
|
||
| // Act | ||
| var host = new HostBuilder() | ||
| .UseDefaultBuilder(Array.Empty<string>()) |
There was a problem hiding this comment.
Can you use empty array initializer instead?
There was a problem hiding this comment.
Changed to use empty array initializer [] throughout in c6a9a4c
| public void should_load_environment_variables_with_app_prefix() | ||
| { | ||
| // Arrange | ||
| var appSettingsPath = Path.Combine(_testDirectory, "appsettings.json"); |
There was a problem hiding this comment.
I'm not sure the appsettings.json file is relevant to the test
There was a problem hiding this comment.
Removed the unnecessary appsettings.json file from the environment variable test in c6a9a4c
| } | ||
|
|
||
| [Test] | ||
| public void should_prioritize_configuration_sources_correctly() |
There was a problem hiding this comment.
I think this is 2 separate tests; one to check env vars override appsettings and another to check command line args override env vars
There was a problem hiding this comment.
Split into two separate tests: should_override_appsettings_with_environment_variables and should_override_environment_variables_with_command_line_arguments in c6a9a4c
acraven
changed the title
… comments, split test Co-authored-by: acraven <9272163+acraven@users.noreply.github.com>
All feedback addressed in c6a9a4c:
All 347 tests passing. |
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.