[fix] Fix: use MSBuild output mode automatically with dotnet msbuild /t:VSTest when terminal logger is active by nohwnd · Pull Request #16058 · microsoft/vstest

nohwnd · 2026-05-24T13:04:38Z

🤖 This is an automated fix generated by the Issue Triage agent.

Root Cause

When running dotnet msbuild /t:VSTest with the MSBuild terminal logger enabled (_MSBUILDTLENABLED == '1'), test output from parallel MSBuild nodes was silently lost.

The cause: VsTestUseMSBuildOutput defaulted to False, so _VSTestConsole was 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 and the user has not set VsTestUseMSBuildOutput explicitly, default it to True. This causes _VSTestMSBuild (using VSTestTask2 / ToolTask) to be used, which routes output through MSBuild task logging — ensuring it is attributed and displayed correctly by the terminal logger.
Added $(MSBUILDENSURESTDOUTFORTASKPROCESSES) != '1' guard to the _VSTestMSBuild condition so it will not run when invoked from dotnet test (which sets this env var and uses _VSTestConsole for proper colorization).

Backward compatibility: Users who want the old console-colorized behavior can opt out with /p:VsTestUseMSBuildOutput=false.

Test

Added MSBuildLoggerIsUsedAutomaticallyWithDotnetMSBuildWhenTerminalLoggerIsActive acceptance test in DotnetTestMSBuildOutputTests that invokes dotnet msbuild -t:VSTest -tl:on without setting VsTestUseMSBuildOutput and asserts that test error output appears in stdout.

Also added InvokeDotnetMSBuildTest helper to IntegrationTestBase to support invoking dotnet msbuild with the patched dotnet in acceptance tests.

🔍 Triaged by Issue Repro Triage & Auto-Fix 🔍

…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>

Copilot

Pull request overview

This PR updates the MSBuild VSTest target selection logic so that when the MSBuild terminal logger is enabled, VSTest defaults to using MSBuild task-based logging output (instead of spawning vstest.console directly), preventing test output loss from parallel MSBuild nodes when running dotnet msbuild /t:VSTest.

Changes:

Default VsTestUseMSBuildOutput to True when the MSBuild terminal logger is active and the user hasn’t set the property explicitly.
Add a guard to keep using the console path when MSBUILDENSURESTDOUTFORTASKPROCESSES=1 (e.g., dotnet test scenario).
Add an acceptance test plus a new dotnet msbuild invocation helper for acceptance tests.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
`src/Microsoft.TestPlatform.Build/Microsoft.TestPlatform.targets`	Adjusts defaulting/target-selection logic to favor MSBuild task logging when terminal logger is active.
`test/Microsoft.TestPlatform.TestUtilities/IntegrationTestBase.cs`	Adds a helper to invoke `dotnet msbuild` via the patched dotnet used by acceptance tests.
`test/Microsoft.TestPlatform.Acceptance.IntegrationTests/DotnetTestMSBuildOutputTests.cs`	Adds an acceptance test intended to validate the new default behavior under terminal logger.

    <CallTarget Targets="_VSTestConsole" Condition="$(_MSBUILDTLENABLED) == '0' OR !$(VsTestUseMSBuildOutput) OR $(MSBUILDENSURESTDOUTFORTASKPROCESSES) == '1'" />
    <!-- Proper MSBuild integration, but no custom colorization -->
-    <!-- Use the new view whe terminal logger is enabled (_MSBUILDTLENABLED==0), and user did not opt out (VsTestUseMSBuildOutput == true) -->
-    <CallTarget Targets="_VSTestMSBuild" Condition="$(_MSBUILDTLENABLED) == '1' AND $(VsTestUseMSBuildOutput)" />
+    <!-- Use the new view when terminal logger is enabled (_MSBUILDTLENABLED==1), user did not opt out (VsTestUseMSBuildOutput == true),
+         and MSBUILDENSURESTDOUTFORTASKPROCESSES is not set (set by dotnet test, which uses the console path instead). -->
+    <CallTarget Targets="_VSTestMSBuild" Condition="$(_MSBUILDTLENABLED) == '1' AND $(VsTestUseMSBuildOutput) AND $(MSBUILDENSURESTDOUTFORTASKPROCESSES) != '1'" />


+    public void MSBuildLoggerIsUsedAutomaticallyWithDotnetMSBuildWhenTerminalLoggerIsActive(RunnerInfo runnerInfo)
+    {
+        SetTestEnvironment(_testEnvironment, runnerInfo);
+
+        var projectPath = GetIsolatedTestAsset("TerminalLoggerTestProject.csproj", runnerInfo.TargetFramework);
+        // 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));
+


nohwnd

Expert Review — PR #16058

The fix is small, focused, and the logic is correct. The two CallTarget conditions remain mutually exclusive across all relevant scenarios (TL on/off, dotnet test vs dotnet msbuild, user opt-out).

Key checks passed:

✅ No double-invocation possible — the _VSTestConsole and _VSTestMSBuild conditions are mutually exclusive
✅ Backward compatibility preserved — VsTestUseMSBuildOutput=False explicit opt-out still works; users on non-TL MSBuild are unaffected
✅ dotnet test isolation — the MSBUILDENSURESTDOUTFORTASKPROCESSES == '1' guard correctly routes dotnet test to _VSTestConsole
✅ Two-line property default ordering is correct — the _MSBUILDTLENABLED == '1' check sets True first, the unconditional fallback sets False only when still unset
✅ Acceptance test covers the exact scenario described in the fix

One minor finding (see inline): VSTEST_CONSOLE_PATH env var in InvokeDotnetMSBuildTest appears to be dead code for the dotnet msbuild path.

🧠 Reviewed by Expert Code Reviewer

🧠 Reviewed by Expert Code Reviewer 🧠

nohwnd · 2026-05-24T13:09:35Z

+
+        debugEnvironmentVariables["VSTEST_CONSOLE_PATH"] = vstestConsolePath;
+
+        IntegrationTestBase.ExecutePatchedDotnet("msbuild", arguments, out _standardTestOutput, out _standardTestError, out _runnerExitCode, debugEnvironmentVariables, workingDirectory);


[Environment Variable & Feature Flag Contracts] VSTEST_CONSOLE_PATH appears to be used by the dotnet test / _VSTestConsole path, not the dotnet msbuild / _VSTestMSBuild path (which reads VsTestConsolePath from the MSBuild property already appended as -p:VsTestConsolePath=...). Setting this env var here looks like dead code.

If ExecutePatchedDotnet/_VSTestMSBuild doesn't read VSTEST_CONSOLE_PATH, removing the line would be a clean-up. If it is read somewhere in the MSBuild task, a comment explaining why it's needed would help future readers.

This doesn't affect correctness (the MSBuild property path already ensures the right vstest.console is used), but it adds confusion.

Good catch — removed the dead VSTEST_CONSOLE_PATH assignment. The MSBuild property -p:VsTestConsolePath already handles routing the correct console path.

🔧 Iterated by PR Iteration Agent 🔧

The MSBuild path already uses -p:VsTestConsolePath to pass the console path as an MSBuild property. Setting VSTEST_CONSOLE_PATH env var here is dead code. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

nohwnd · 2026-05-24T13:13:27Z

Commit pushed: 8677ca1

🔧 Iterated by PR Iteration Agent 🔧

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

nohwnd · 2026-05-28T06:00:46Z

Commit pushed: 07a6ff6

🔧 Iterated by PR Iteration Agent 🔧

The test invoked 'dotnet msbuild -t:VSTest' without building the project first, so the test DLL did not exist and vstest.console failed with a file-not-found error instead of running tests. Change to '-t:Build;VSTest' to ensure the project is compiled before the test run, matching the typical real-world usage pattern. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

nohwnd · 2026-05-28T07:10:48Z

Commit pushed: 5911482

🔧 Iterated by PR Iteration Agent 🔧

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

nohwnd

Expert Review — PR #16058 (re-review after iteration)

This is a follow-up review covering commits pushed after the initial review on 2026-05-24.

New commits reviewed:

8677ca1 — Removed dead VSTEST_CONSOLE_PATH env var from InvokeDotnetMSBuildTest (addresses the finding from the prior review ✅)
07a6ff6 — CI retrigger only, no code changes
5911482 — Fixed acceptance test to use -t:Build;VSTest instead of -t:VSTest; the project was not compiled before test execution, causing the DLL to be missing ✅

All prior findings resolved. The PR is clean:

✅ .targets logic: _VSTestConsole and _VSTestMSBuild remain mutually exclusive across all scenarios
✅ VsTestUseMSBuildOutput two-step default ordering is correct
✅ MSBUILDENSURESTDOUTFORTASKPROCESSES == '1' guard correctly isolates dotnet test from dotnet msbuild
✅ InvokeDotnetMSBuildTest passes console path via MSBuild property (correct for the VSTestConsolePath lookup in targets)
✅ Acceptance test now builds before running tests via -t:Build;VSTest

🧠 Reviewed by Expert Code Reviewer 🧠

🧠 Reviewed by Expert Code Reviewer 🧠

The acceptance test was using -tl:on to activate the terminal logger, but in non-interactive (non-TTY) CI environments, the terminal logger may not set _MSBUILDTLENABLED=1 even when explicitly requested. This caused the auto-detection in Microsoft.TestPlatform.targets to not trigger, making VsTestUseMSBuildOutput remain False and the test assertions fail. Switch to explicitly setting /p:_MSBUILDTLENABLED=1 to directly simulate the terminal logger being active. This tests the auto-detection mechanism reliably regardless of TTY availability. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

nohwnd · 2026-06-02T06:18:41Z

Commit pushed: 05409a9

🔧 Iterated by PR Iteration Agent 🔧

nohwnd · 2026-06-02T06:18:47Z

Pushed a fix: changed the acceptance test to use /p:_MSBUILDTLENABLED=1 instead of -tl:on.

Root cause: In non-interactive (non-TTY) CI environments, the MSBuild terminal logger may not set the _MSBUILDTLENABLED=1 property even when explicitly forced with -tl:on. This caused the auto-detection in Microsoft.TestPlatform.targets to not trigger, so VsTestUseMSBuildOutput remained False, and the _VSTestConsole path was used instead of _VSTestMSBuild. The test assertions for TESTERROR and FailingTest ( then failed because those are only output by VSTestTask2 on the MSBuild path.

The fix uses /p:_MSBUILDTLENABLED=1 to directly simulate the terminal logger being active, testing the auto-detection mechanism reliably regardless of TTY availability.

🔧 Iterated by PR Iteration Agent 🔧

nohwnd

Expert Review — PR #16058 (re-review after latest commit)

This is a follow-up review covering the new commit pushed on 2026-06-02.

New commit reviewed:

05409a9 — Switch acceptance test from -tl:on to /p:_MSBUILDTLENABLED=1

Analysis:

The change is correct and well-motivated. -tl:on enables the MSBuild terminal logger, but in non-TTY CI environments MSBuild may suppress the terminal logger even when explicitly requested, leaving _MSBUILDTLENABLED unset. This would cause the auto-detection path in Microsoft.TestPlatform.targets to not trigger, making VsTestUseMSBuildOutput remain False and the test assertions fail spuriously.

By directly injecting /p:_MSBUILDTLENABLED=1, the test bypasses the TTY detection and exercises the auto-detection logic deterministically — exactly what an integration test should do.

✅ No code changes to .targets — the fix is purely a test reliability improvement
✅ The comment in the test file accurately explains why /p:_MSBUILDTLENABLED=1 is preferred over -tl:on
✅ All prior findings remain resolved; no new issues introduced

All checks passed. The PR is clean and ready for human review/merge.

🧠 Reviewed by Expert Code Reviewer 🧠

🧠 Reviewed by Expert Code Reviewer 🧠

Copilot AI review requested due to automatic review settings May 24, 2026 13:04

Copilot started reviewing on behalf of nohwnd May 24, 2026 13:04 View session

Copilot AI reviewed May 24, 2026

View reviewed changes

nohwnd commented May 24, 2026

View reviewed changes

This was referenced May 25, 2026

Daily Maintenance Digest — 2026-05-25 #16049

Closed

Daily Maintenance Digest — 2026-06-02 #16066

Open

Retrigger CI

07a6ff6

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

nohwnd mentioned this pull request May 28, 2026

[fix] Fix: use MSBuild output mode automatically with dotnet msbuild /t:VSTest when terminal logger is active #16057

Open

Copilot AI review requested due to automatic review settings May 28, 2026 07:10

Copilot started reviewing on behalf of nohwnd May 28, 2026 07:11 View session

Copilot AI reviewed May 28, 2026

View reviewed changes

nohwnd commented May 28, 2026

View reviewed changes

nohwnd commented Jun 2, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[fix] Fix: use MSBuild output mode automatically with dotnet msbuild /t:VSTest when terminal logger is active#16058

[fix] Fix: use MSBuild output mode automatically with dotnet msbuild /t:VSTest when terminal logger is active#16058
nohwnd wants to merge 5 commits into
mainfrom
fix/issue-5156-vstest-msbuild-terminal-logger-6a3807ae5ecbb354

nohwnd commented May 24, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

nohwnd left a comment

Uh oh!

nohwnd May 24, 2026

Uh oh!

nohwnd May 24, 2026

Uh oh!

nohwnd commented May 24, 2026

Uh oh!

nohwnd commented May 28, 2026

Uh oh!

nohwnd commented May 28, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

nohwnd left a comment

Uh oh!

nohwnd commented Jun 2, 2026

Uh oh!

nohwnd commented Jun 2, 2026

Uh oh!

nohwnd left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants


		debugEnvironmentVariables["VSTEST_CONSOLE_PATH"] = vstestConsolePath;

		IntegrationTestBase.ExecutePatchedDotnet("msbuild", arguments, out _standardTestOutput, out _standardTestError, out _runnerExitCode, debugEnvironmentVariables, workingDirectory);

Conversation

nohwnd commented May 24, 2026

Root Cause

Fix

Test

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

nohwnd left a comment

Choose a reason for hiding this comment

Expert Review — PR #16058

Uh oh!

nohwnd May 24, 2026

Choose a reason for hiding this comment

Uh oh!

nohwnd May 24, 2026

Choose a reason for hiding this comment

Uh oh!

nohwnd commented May 24, 2026

Uh oh!

nohwnd commented May 28, 2026

Uh oh!

nohwnd commented May 28, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

nohwnd left a comment

Choose a reason for hiding this comment

Expert Review — PR #16058 (re-review after iteration)

Uh oh!

nohwnd commented Jun 2, 2026

Uh oh!

nohwnd commented Jun 2, 2026

Uh oh!

nohwnd left a comment

Choose a reason for hiding this comment

Expert Review — PR #16058 (re-review after latest commit)

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants