Support environmentVariables section in testconfig.json (#5491)#8391
Open
Evangelink wants to merge 6 commits into
Open
Support environmentVariables section in testconfig.json (#5491)#8391Evangelink wants to merge 6 commits into
Evangelink wants to merge 6 commits into
Conversation
…sions.CrashDump The CrashDump extension only consumes `ApplicationStateGuard` and `PlatformResources` from `Microsoft.Testing.Platform` internals, both of which are already source-linked into the CrashDump project via `<Compile Include=...>` in its csproj. The InternalsVisibleTo entry is therefore unused and can be removed, continuing the series started in #7443, #7813, #7842 and #8180. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add a built-in ITestHostEnvironmentVariableProvider that reads the new �nvironmentVariablessection of testconfig.json and applies the entries to the test host child process via the existing test host controller process model. The provider is only enabled when the section is present and non-empty, so projects that do not use the feature pay no cost. Includes RFC 002 documenting the design, precedence, and caveats (discovery does not apply env vars, JSON null parsing differs by TFM, secrets should not be put in this file). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds support for declaring test-host environment variables via an environmentVariables section in testconfig.json, enabling a built-in provider to apply them by opting into the controller/child-process model.
Changes:
- Added
TestConfigurationEnvironmentVariableProviderand automatic registration inTestHostBuilderto applytestconfig.jsonenvironment variables to the child test host process. - Added strict schema validation for a JSON section via
JsonConfigurationProvider.GetSection, surfaced viaAggregatedConfiguration.GetTestConfigJsonSection. - Added unit + acceptance tests plus an RFC doc, and introduced corresponding resource strings.
Show a summary per file
| File | Description |
|---|---|
| test/UnitTests/Microsoft.Testing.Platform.UnitTests/TestHostControllers/TestConfigurationEnvironmentVariableProviderTests.cs | Unit tests covering enablement, name validation, update behavior, and validation contract of the new provider. |
| test/UnitTests/Microsoft.Testing.Platform.UnitTests/Configuration/ConfigurationManagerTests.cs | Unit tests for strict testconfig.json section schema validation + exposes reusable MemoryFileStream. |
| test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/TestConfigEnvironmentVariablesTests.cs | End-to-end acceptance coverage proving env vars reach the child test host and invalid shapes fail. |
| src/Platform/Microsoft.Testing.Platform/TestHostControllers/TestConfigurationEnvironmentVariableProvider.cs | Implements built-in environment variable provider backed by testconfig.json. |
| src/Platform/Microsoft.Testing.Platform/Hosts/TestHostBuilder.CommonServices.cs | Registers the built-in env var provider early to allow later providers to override. |
| src/Platform/Microsoft.Testing.Platform/Configurations/JsonConfigurationProvider.cs | Adds internal GetSection API with strict schema enforcement for flat object sections. |
| src/Platform/Microsoft.Testing.Platform/Configurations/AggregatedConfiguration.cs | Adds internal helper to read a section specifically from the JSON configuration provider. |
| src/Platform/Microsoft.Testing.Platform/Resources/PlatformResources.resx | Adds new error/description strings for JSON section validation and the provider. |
| src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.zh-Hant.xlf | Adds new resource IDs for the feature (currently untranslated). |
| src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.zh-Hans.xlf | Adds new resource IDs for the feature (currently untranslated). |
| src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.tr.xlf | Adds new resource IDs for the feature (currently untranslated). |
| src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.ru.xlf | Adds new resource IDs for the feature (currently untranslated). |
| src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.pt-BR.xlf | Adds new resource IDs for the feature (currently untranslated). |
| src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.pl.xlf | Adds new resource IDs for the feature (currently untranslated). |
| src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.ko.xlf | Adds new resource IDs for the feature (currently untranslated). |
| src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.ja.xlf | Adds new resource IDs for the feature (currently untranslated). |
| src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.it.xlf | Adds new resource IDs for the feature (currently untranslated). |
| src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.fr.xlf | Adds new resource IDs for the feature (currently untranslated). |
| src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.es.xlf | Adds new resource IDs for the feature (currently untranslated). |
| src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.de.xlf | Adds new resource IDs for the feature (currently untranslated). |
| src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.cs.xlf | Adds new resource IDs for the feature (currently untranslated). |
| src/Platform/Microsoft.Testing.Platform/Microsoft.Testing.Platform.csproj | Removes an InternalsVisibleTo entry (appears unrelated to feature). |
| docs/microsoft.testing.platform/002-TestConfig-EnvironmentVariables.md | RFC documenting design, precedence, schema, and limitations. |
Copilot's findings
- Files reviewed: 23/23 changed files
- Comments generated: 5
…nfig-environment-variables # Conflicts: # src/Platform/Microsoft.Testing.Platform/Microsoft.Testing.Platform.csproj
- JsonConfigurationProvider.GetSection: handle _singleValueData and _propertyToAllChildren independently so a partially-populated state cannot silently mask a malformed section, and detect empty arrays ([]) to reject them with the same error as non-empty arrays. - TestConfigurationEnvironmentVariableProvider.IsEnabledAsync: clear cached _entries when returning false so stale state cannot leak between calls. - Add unit test for empty-array section value. - Fix IDE0007 'use var' style errors in TestConfigEnvironmentVariablesTests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The merge with main accidentally swapped the Microsoft.Testing.Extensions.CrashDump InternalsVisibleTo entry with Microsoft.Testing.Extensions.AzureFoundry. Removing the CrashDump IVT broke the ForwardCompatibilityTests because the previously published (2.2.1) CrashDump extension still calls into Platform internals such as GetTestApplicationModuleInfo, which fails at runtime with MethodAccessException when the IVT is absent. Restore the CrashDump IVT and drop the stray AzureFoundry IVT (removed on main in #8384) so the csproj matches main. Also fix MD051 link-fragments issue in the testconfig environment variables RFC: update the anchor to `#edge-cases-and-limitations` to match the actual heading `Edge cases and limitations`. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
|
||
| testHostResult.AssertExitCodeIs(ExitCode.Success); | ||
| // Variable is not set, so the framework prints the sentinel. | ||
| testHostResult.AssertOutputContains("TEST_CONFIG_VAR_FROM_FILE=<unset>"); |
Member
Author
There was a problem hiding this comment.
Good catch — fixed in e256d4bac. The dummy test framework now prints TESTHOSTCONTROLLER_PARENTPID=<value-or-none> (read from the per-controller-PID-suffixed TESTINGPLATFORM_TESTHOSTCONTROLLER_PARENTPID_<pid> env var that TestHostControllersTestHost sets on the spawned child) and:
EnvironmentVariablesSection_DeclaredVariables_AreVisibleToTestHostProcessassertsPARENTPID=\d+(controller engaged).EnvironmentVariablesSection_Absent_TestHostRunsInProcessassertsPARENTPID=<none>(controller not engaged).
Verified locally on net462/net8.0/net10.0 — 9/9 pass.
…ance tests Addresses PR #8391 review feedback (discussion r3275834611). The `EnvironmentVariablesSection_Absent_TestHostRunsInProcess` test name advertised that the controller process model is not engaged when the environmentVariables section is absent, but the only assertion was that the variable was `<unset>` -- which would also pass if the controller model was wrongly engaged but no vars were applied. Now the dummy test framework prints the (per-controller-PID-suffixed) well-known `TESTINGPLATFORM_TESTHOSTCONTROLLER_PARENTPID_<pid>` env var, and both tests assert on it: - `EnvironmentVariablesSection_DeclaredVariables_AreVisibleToTestHostProcess` asserts the controller PARENTPID is set (controller engaged). - `EnvironmentVariablesSection_Absent_TestHostRunsInProcess` asserts the controller PARENTPID is `<none>` (controller not engaged). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves #5491.
What
Adds an
environmentVariablessection totestconfig.jsonso users can declare environment variables for the test host process in a declarative file instead of writing a customITestHostEnvironmentVariableProvider.{ "environmentVariables": { "DOTNET_ENVIRONMENT": "Development", "HEADED": "1" } }How
A built-in
TestConfigurationEnvironmentVariableProvideris registered automatically insideMicrosoft.Testing.Platform. It is only enabled when the section is present and non-empty, so projects that do not use the feature pay no cost. When enabled it opts into the existing test host controller process model: the current process becomes the controller, sets the variables onProcessStartInfo, and launches the test host as a child process.The provider is registered first in
TestHostBuilder.SetupCommonServicesAsyncand writes entries withisLocked: false, so user-registered providers and the VSTest-bridge runsettings provider can still override the values when both sources are present.The new internal
JsonConfigurationProvider.GetSectionenforces a strict schema:"environmentVariables": { "FOO": "bar" }"environmentVariables": { "FOO": 42, "BAR": true }"environmentVariables": {}"environmentVariables": "oops""environmentVariables": [1, 2]"environmentVariables": { "FOO": { "NESTED": "x" } }"environmentVariables": { "FOO=BAR": "x" }Documentation
docs/microsoft.testing.platform/002-TestConfig-EnvironmentVariables.mdis a full RFC covering the design, precedence, alternatives considered, and caveats (--list-testsdoes not apply env vars, cost of opting in, JSON null parser inconsistency across TFMs, secrets, no removal syntax).Tests
Microsoft.Testing.Platform.UnitTests:JsonConfigurationProvider.GetSectionschema validation (absent / empty / valid / scalar / array / nested-object / nested-array / empty-object-entry / numeric+bool coercion).IsEnabledAsynctrue/false paths, invalid name rejection,UpdateAsyncwrites unlocked & non-secret, validation always succeeds).Microsoft.Testing.Platform.Acceptance.IntegrationTestsverifying end-to-end that declared variables reach the test method, that an absent section does not trigger the controller, and that invalid shapes fail with a clear error.Compatibility
PublicAPI.Shipped.txt/PublicAPI.Unshipped.txt.Notes for review
DOTNET_*, profilers, ICU/globalization) only take effect at process startup, so the divergence would be a footgun.""instead.