Respect working directory of 'dotnet test' when determining results directory

[Youssef1313]

Depends on dotnet/sdk#52570

DOTNET_CLI_TEST_COMMAND_WORKING_DIRECTORY (set by dotnet test) is now used as the base directory for both PlatformResultDirectory (defaulting to {cwd}/TestResults) and PlatformCurrentWorkingDirectory/PlatformTestHostWorkingDirectory, preserving the invariant that all output paths share a common root when running under dotnet test. The --results-directory CLI argument continues to take precedence over the environment variable.

[nohwnd]

None of the tests are testing with the new variable, they just provide empty environment.

[Evangelink]

@copilot resolve the merge conflicts in this pull request

[Copilot]

@copilot resolve the merge conflicts in this pull request

Merge conflicts resolved. Two files had conflicts:

• ConfigurationManager.cs: merged to use configurationProvidersArray from main while keeping the _environment parameter our PR added
• ExecutableInfo.cs: restored the Workspace property that main maintains (which our cleanup commit had removed)

None of the tests are testing with the new variable, they just provide empty environment.

Fixed in commit 1fea1ee. Replaced new SystemEnvironment() with Mock<IEnvironment> throughout the tests so environment variables are fully controlled, and added two new tests that explicitly set DOTNET_CLI_TEST_COMMAND_WORKING_DIRECTORY and verify the results directory is based on it:

• IndexerTest_DotnetCliTestCommandWorkingDirectorySet_UsedAsResultDirectoryBase
• CheckTestResultsDirectoryOverrideAndCreateItAsync_DotnetCliTestCommandWorkingDirectorySet_UsedAsResultDirectoryBase

[Copilot]

Pull request overview

Updates Microsoft.Testing.Platform configuration/logging to base the default TestResults directory on the dotnet test working directory (via a new environment variable), rather than only the test host’s current working directory.

Changes:

• Add support for DOTNET_CLI_TEST_COMMAND_WORKING_DIRECTORY when computing the default results directory (and diagnostic log default directory).
• Thread IEnvironment through ConfigurationManager → AggregatedConfiguration to read the new environment variable.
• Update and extend unit tests to cover the new results-directory base behavior.

Show a summary per file

File	Description
test/UnitTests/MSTest.Engine.UnitTests/Adapter_ExecuteRequestAsyncTests.cs	Updates AggregatedConfiguration construction to include IEnvironment.
test/UnitTests/Microsoft.Testing.Platform.UnitTests/TestApplicationBuilderTests.cs	Updates AggregatedConfiguration construction to include IEnvironment.
test/UnitTests/Microsoft.Testing.Platform.UnitTests/Configuration/ConfigurationManagerTests.cs	Updates ConfigurationManager construction to include IEnvironment.
test/UnitTests/Microsoft.Testing.Platform.UnitTests/Configuration/AggregatedConfigurationTests.cs	Adds coverage for DOTNET_CLI_TEST_COMMAND_WORKING_DIRECTORY affecting default TestResults base.
src/Platform/Microsoft.Testing.Platform/TestHostControllers/TestHostControllersManager.cs	Removes now-unused configuration-derived working directory retrieval.
src/Platform/Microsoft.Testing.Platform/Services/CurrentTestApplicationModuleInfo.cs	Uses explicit new ExecutableInfo(...) construction.
src/Platform/Microsoft.Testing.Platform/Hosts/TestHostBuilder.cs	Passes IEnvironment into the updated ConfigurationManager and removes unused working-directory code.
src/Platform/Microsoft.Testing.Platform/Helpers/EnvironmentVariableConstants.cs	Introduces DOTNET_CLI_TEST_COMMAND_WORKING_DIRECTORY constant.
src/Platform/Microsoft.Testing.Platform/Configurations/ConfigurationManager.cs	Adds IEnvironment dependency and forwards it to AggregatedConfiguration.
src/Platform/Microsoft.Testing.Platform/Configurations/AggregatedConfiguration.cs	Uses the new env var as the base for default results directory calculation.
src/Platform/Microsoft.Testing.Platform/Builder/TestApplication.cs	Uses the new env var as the base for the default diagnostic log directory.

Copilot's findings

• Files reviewed: 11/11 changed files
• Comments generated: 2

[Evangelink]

Summary

Workflow: Test Expert Reviewer 🧪
Date: 2026-05-11
Repository: microsoft/testfx

Key Findings

The PR introduces good coverage for the new DOTNET_CLI_TEST_COMMAND_WORKING_DIRECTORY env-var feature. The two new tests (IndexerTest_DotnetCliTestCommandWorkingDirectorySet_UsedAsResultDirectoryBase and CheckTestResultsDirectoryOverrideAndCreateItAsync_DotnetCliTestCommandWorkingDirectorySet_UsedAsResultDirectoryBase) are well-structured and verify the core happy path.

However, three issues are worth addressing:

1. [Coverage — important] No test verifies that --results-directory takes precedence over DOTNET_CLI_TEST_COMMAND_WORKING_DIRECTORY when both are present. The production code has an explicit comment explaining this priority is critical for the Retry extension, but there's no regression guard.

2. [Structure — minor] The async env-var test sets up mockTestApplicationModuleInfo stubs that are never called (dead setups), because the env-var path short-circuits before the module-info members are needed.

3. [Structure — pre-existing, minor] Two existing tests (_GetDirectoryFromCommandLineProvider and _GetDirectoryFromStore) are byte-for-byte identical — one of them appears to be a mislabeled duplicate or an incomplete test.

Recommendations

• Add a test that sets both DOTNET_CLI_TEST_COMMAND_WORKING_DIRECTORY and --results-directory and asserts the CLI option wins.
• Remove the dead mockTestApplicationModuleInfo setups from CheckTestResultsDirectoryOverrideAndCreateItAsync_DotnetCliTestCommandWorkingDirectorySet_UsedAsResultDirectoryBase.
• Investigate (and fix) the two identical CheckTestResultsDirectoryOverrideAndCreateItAsync_ResultsDirectoryIsNull_Get* tests.

---

Generated by Test Expert Reviewer

🧪 Test quality reviewed by Test Expert Reviewer 🧪

[Evangelink]

Summary

Workflow: pr-expert-reviewer
Date: 2026-05-11
Repository: microsoft/testfx

Key Findings

[Correctness – Medium] Asymmetry between PlatformResultDirectory and PlatformCurrentWorkingDirectory

GetResultsDirectoryCore now uses DOTNET_CLI_TEST_COMMAND_WORKING_DIRECTORY as the base for the result folder, but GetCurrentWorkingDirectoryCore does not. After this change, in any dotnet test scenario where no --results-directory is given:

• config[PlatformResultDirectory] → {dotnet_test_cwd}/TestResults
• config[PlatformCurrentWorkingDirectory] → {app_binary_dir} (or config-provider value)

Extensions that derive paths relative to PlatformCurrentWorkingDirectory (coverage files, attachments, TRX custom locations) will land in a different directory tree than the main results. This breaks the implicit contract that results files live under a common root. See inline comment for the precise location.

Positive Observations

• Correct priority ordering is maintained: --results-directory CLI arg still wins over the new env var, which wins over configuration providers, which wins over the default. No regression in the priority chain.
• Diagnostic logger path receives the same fix via CreateFileLoggerIfDiagnosticIsEnabled, so diagnostic logs go alongside results even before configuration is built — this is the right level to apply it given the init order.
• Dead code removal in TestHostBuilder.cs and TestHostControllersManager.cs (ApplicationStateGuard.Ensure(currentWorkingDirectory is not null)) is safe: GetCurrentWorkingDirectoryCore() always returns non-null due to its fallback chain, so these assertions could never fail. Removing them is correct.
• Test coverage for the new env-var behavior is present in AggregatedConfigurationTests — both the indexer path and CheckTestResultsDirectoryOverrideAndCreateItAsync are exercised.
• nameof pattern for the new constant is correct and self-validating.

Recommendations

1. Decide whether PlatformCurrentWorkingDirectory should also shift to DOTNET_CLI_TEST_COMMAND_WORKING_DIRECTORY when the env var is set, or document the intentional asymmetry with a code comment so extension authors are not surprised.
2. (Minor/optional) Consider adding a Path.IsPathRooted guard when reading the env var — if dotnet test ever sends an empty or relative value, Path.Combine("", "TestResults") silently produces a relative path resolved from the process CWD, which is harder to diagnose than an early validation failure.

---

Generated by pr-expert-reviewer 🧠

🧠 Reviewed by Expert Code Reviewer 🧠

[Evangelink]

@copilot address review comments

[Copilot]

@copilot address review comments

Addressed in commit a322950. Here's a summary of all changes made:

Production code (AggregatedConfiguration.cs):

• Applied DOTNET_CLI_TEST_COMMAND_WORKING_DIRECTORY in GetCurrentWorkingDirectoryCore() (between config providers and the app directory fallback), so PlatformCurrentWorkingDirectory and PlatformResultDirectory share the same base when running under dotnet test.

Tests (AggregatedConfigurationTests.cs):

• Fixed CheckTestResultsDirectoryOverrideAndCreateItAsync_ResultsDirectoryIsNull_GetDirectoryFromStore — was a duplicate of the CLI arg test; now actually uses a FakeConfigurationProvider to supply the results directory from the configuration store.
• Removed dead mockTestApplicationModuleInfo setups from CheckTestResultsDirectoryOverrideAndCreateItAsync_DotnetCliTestCommandWorkingDirectorySet_UsedAsResultDirectoryBase (env var short-circuits before module info is needed), and added an assertion that PlatformCurrentWorkingDirectory also resolves to the env var value.
• Added IndexerTest_ResultsDirectoryCliArgTakesPrecedenceOverDotnetCliTestCommandWorkingDirectory — verifies --results-directory wins over the env var for PlatformResultDirectory.
• Added CheckTestResultsDirectoryOverrideAndCreateItAsync_ResultsDirectoryCliArgTakesPrecedenceOverDotnetCliTestCommandWorkingDirectory — same priority check via the full CheckTestResultsDirectoryOverrideAndCreateItAsync path.
• Extended IndexerTest_DotnetCliTestCommandWorkingDirectorySet_UsedAsWorkingDirectoryBase to assert PlatformCurrentWorkingDirectory and PlatformTestHostWorkingDirectory also resolve to the env var value.

[Copilot]

Copilot's findings

• Files reviewed: 11/11 changed files
• Comments generated: 1

[Copilot]

Copilot's findings

• Files reviewed: 12/12 changed files
• Comments generated: 2

[Evangelink]

@copilot resolve the merge conflicts in this pull request

[Copilot]

@copilot resolve the merge conflicts in this pull request

Merge conflicts resolved. The only conflict was in TestHostBuilder.cs — main had refactored the class into focused partial modules (#8201). Resolution: took main's refactored structure (which delegates BuildAsync to SetupCommonServicesAsync/BuildHostAsync in partial files) and preserved our environment parameter addition to ConfigurationManager.

[Evangelink]

@copilot address review comments

[Copilot]

@copilot address review comments

Addressed in the latest commit. Two changes:

1. Diagnostic directory coverage (TestApplication.cs:308-314): Extracted the directory computation logic into an internal GetDiagnosticDefaultDirectory method, then added tests in TestApplicationDiagnosticVerbosityTests.cs:

   • GetDiagnosticDefaultDirectory_WhenDotnetCliTestCommandWorkingDirectorySet_UsesEnvVarAsBase — env var is used when set
   • GetDiagnosticDefaultDirectory_WhenDotnetCliTestCommandWorkingDirectoryNullOrWhitespace_FallsBackToAppDirectory — null/empty/whitespace all fall back to the app directory

2. Whitespace fallback coverage (AggregatedConfiguration.cs:121-124): Added IndexerTest_DotnetCliTestCommandWorkingDirectoryIsWhitespace_FallsBackToTestApplicationDirectory (with DataRow for empty and whitespace) verifying all three keys fall back to the app directory.