[fix] Fix: use MSBuild output mode automatically with dotnet msbuild /t:VSTest when terminal logger is active#16057
Conversation
…est when terminal logger is active When running 'dotnet msbuild /t:VSTest' with the terminal logger enabled, test output from parallel MSBuild nodes was silently lost because VSTestTask wrote directly to stdout rather than through MSBuild's task logging system. Fix: default VsTestUseMSBuildOutput to True when the terminal logger is active (_MSBUILDTLENABLED == '1') so that VSTestTask2 (ToolTask-based) is used automatically. This routes test output through MSBuild's logging infrastructure, ensuring it is properly attributed and displayed. Users who need the old console-colorized behavior can opt out with /p:VsTestUseMSBuildOutput=false. The _VSTestMSBuild condition also now excludes the MSBUILDENSURESTDOUTFORTASKPROCESSES == '1' case (set by dotnet test) to avoid running both paths simultaneously. Fixes #5156 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR adjusts VSTest’s MSBuild integration so that when running dotnet msbuild -t:VSTest with the MSBuild Terminal Logger enabled, test output is routed through MSBuild task logging by default (to avoid losing output from parallel nodes). It also adds acceptance coverage and a test utility helper to invoke dotnet msbuild with the patched dotnet used in the acceptance test infrastructure.
Changes:
- Default
VsTestUseMSBuildOutputtoTruewhen the MSBuild terminal logger is active and the user hasn’t explicitly set it. - Guard
_VSTestMSBuildexecution whenMSBUILDENSURESTDOUTFORTASKPROCESSES=1to avoid using the MSBuild-output path in that mode. - Add an acceptance test + helper to run
dotnet msbuildin the patched-dotnet acceptance test environment.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/Microsoft.TestPlatform.Build/Microsoft.TestPlatform.targets |
Changes default selection logic to prefer MSBuild-output mode under terminal logger, with an additional guard for MSBUILDENSURESTDOUTFORTASKPROCESSES. |
test/Microsoft.TestPlatform.TestUtilities/IntegrationTestBase.cs |
Adds InvokeDotnetMSBuildTest helper to execute dotnet msbuild using the patched dotnet and inject VsTestConsolePath. |
test/Microsoft.TestPlatform.Acceptance.IntegrationTests/DotnetTestMSBuildOutputTests.cs |
Adds an acceptance test intended to validate automatic MSBuild-output mode selection for dotnet msbuild -t:VSTest -tl:on. |
| // Invoke via dotnet msbuild /t:VSTest with terminal logger on and no explicit VsTestUseMSBuildOutput property. | ||
| // The MSBuild output path should be used automatically when the terminal logger is active. | ||
| InvokeDotnetMSBuildTest($@"-t:VSTest {projectPath} -tl:on -nodereuse:false /p:PackageVersion={IntegrationTestEnvironment.LatestLocallyBuiltNugetVersion}", workingDirectory: Path.GetDirectoryName(projectPath)); | ||
|
|
||
| StdOutputContains("TESTERROR"); | ||
| StdOutputContains("FailingTest ("); | ||
| ExitCodeEquals(1); |
nohwnd
left a comment
nohwnd
left a comment
There was a problem hiding this comment.
🧠 Reviewed by expert-reviewer
Dimensions activated: Environment Variable & Feature Flag Contracts · Backward Compatibility & Rollback Safety · Acceptance Test Coverage Design · Build Script & Infrastructure Hygiene
Summary
The fix is correct. The MSBuild condition logic in the two CallTarget elements is mutually exclusive across all scenarios:
| Scenario | _VSTestConsole |
_VSTestMSBuild |
|---|---|---|
_MSBUILDTLENABLED unset |
✅ runs (!False) |
❌ (''≠'1') |
_MSBUILDTLENABLED=1, no user opt-in (the fix path) |
❌ | ✅ |
_MSBUILDTLENABLED=1, MSBUILDENSURESTDOUTFORTASKPROCESSES=1 (dotnet test) |
✅ (=='1') |
❌ ('1'≠'1' fails) |
_MSBUILDTLENABLED=0 |
✅ ('0'=='0') |
❌ |
User sets VsTestUseMSBuildOutput=False explicitly |
✅ | ❌ |
No double-execution, no silent no-ops, correct opt-out semantics.
The two-stage property defaulting pattern (TL-conditional True → fallback False) is idiomatic MSBuild and correctly evaluated at property evaluation time since _MSBUILDTLENABLED is an environment variable available before the first property pass.
The InvokeDotnetMSBuildTest helper correctly mirrors the InvokeDotnetTest pattern and the [TestCategory("Windows-Review")] restriction is appropriate given that patched dotnet is only available on Windows in this test infrastructure.
No blocking issues.
🧠 Reviewed by Expert Code Reviewer 🧠
🤖 This is an automated fix generated by the Issue Triage agent.
Fixes #5156
Root Cause
When running
dotnet msbuild /t:VSTestwith the MSBuild terminal logger enabled (_MSBUILDTLENABLED == '1'), test output from parallel MSBuild nodes was silently lost.The cause:
VsTestUseMSBuildOutputdefaulted toFalse, so_VSTestConsolewas always used. This target invokes vstest.console directly and writes to stdout — output that the terminal logger cannot properly attribute to individual projects when running on parallel MSBuild worker nodes.Fix
Microsoft.TestPlatform.targets: When the terminal logger is active (_MSBUILDTLENABLED == '1') and the user hasn't setVsTestUseMSBuildOutputexplicitly, default it toTrue. This causes_VSTestMSBuild(usingVSTestTask2/ToolTask) to be used, which properly routes output through MSBuild's task logging — ensuring it is attributed and displayed correctly by the terminal logger.Added
$(MSBUILDENSURESTDOUTFORTASKPROCESSES) != '1'guard to the_VSTestMSBuildcondition so it won't run when invoked fromdotnet test(which sets this env var and uses_VSTestConsolefor proper colorization).Backward compatibility: Users who want the old console-colorized behavior can opt out:
/p:VsTestUseMSBuildOutput=false.Test
Added
MSBuildLoggerIsUsedAutomaticallyWithDotnetMSBuildWhenTerminalLoggerIsActiveacceptance test inDotnetTestMSBuildOutputTeststhat invokesdotnet msbuild -t:VSTest -tl:onwithout settingVsTestUseMSBuildOutputand asserts that test error output appears in stdout.Also added
InvokeDotnetMSBuildTesthelper toIntegrationTestBaseto support invokingdotnet msbuildwith the patched dotnet in acceptance tests.