Enable msbuild terminal logger for dotnet test

src/Cli/dotnet/commands/dotnet-test/Program.cs

@@ -2,9 +2,9 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System.CommandLine;
-
 using Microsoft.DotNet.Cli;
 using Microsoft.DotNet.Cli.Utils;
+using Microsoft.DotNet.Cli.Utils.Extensions;
 
 namespace Microsoft.DotNet.Tools.Test
 {
@@ -62,8 +62,28 @@ private static int ForwardToMsbuild(ParseResult parseResult, string[] settings,
             string previousNodeWindowSetting = Environment.GetEnvironmentVariable(NodeWindowEnvironmentName);
             try
             {
-                Environment.SetEnvironmentVariable(NodeWindowEnvironmentName, "1");
-                int exitCode = FromParseResult(parseResult, settings, testSessionCorrelationId).Execute();
+                var properties = GetUserSpecifiedExplicitMSBuildProperties(parseResult);
+                var hasUserMSBuildOutputProperty = properties.TryGetValue("VsTestUseMSBuildOutput", out var propertyValue);
+
+                string[] additionalBuildProperties;
+                if (!hasUserMSBuildOutputProperty)
+                {
+                    additionalBuildProperties = ["--property:VsTestUseMSBuildOutput=true"];
+                }
+                else if (propertyValue.ToLowerInvariant() == "true")
+                {
+                    // User specified the property themselves. Do nothing.
+                    additionalBuildProperties = Array.Empty<string>();
+                }
+                else
+                {
+                    // User explicitly disabled the new logger. Use workarounds needed for old logger.
+                    // Workaround for https://github.com/Microsoft/vstest/issues/1503
+                    Environment.SetEnvironmentVariable(NodeWindowEnvironmentName, "1");
+                    additionalBuildProperties = ["-nodereuse:false"];
+                }
+
+                int exitCode = FromParseResult(parseResult, settings, testSessionCorrelationId, additionalBuildProperties).Execute();
 
                 // We run post processing also if execution is failed for possible partial successful result to post process.
                 exitCode |= RunArtifactPostProcessingIfNeeded(testSessionCorrelationId, parseResult, FeatureFlag.Instance);
@@ -115,20 +135,13 @@ public static TestCommand FromArgs(string[] args, string testSessionCorrelationI
                 testSessionCorrelationId = $"{Environment.ProcessId}_{Guid.NewGuid()}";
             }
 
-            return FromParseResult(parseResult, settings, testSessionCorrelationId, msbuildPath);
+            return FromParseResult(parseResult, settings, testSessionCorrelationId, Array.Empty<string>(), msbuildPath);
         }
 
-        private static TestCommand FromParseResult(ParseResult result, string[] settings, string testSessionCorrelationId, string msbuildPath = null)
+        private static TestCommand FromParseResult(ParseResult result, string[] settings, string testSessionCorrelationId, string[] additionalBuildProperties, string msbuildPath = null)
         {
             result.ShowHelpOrErrorIfAppropriate();
 
-            var msbuildArgs = new List<string>()
-            {
-                "-target:VSTest",
-                "-nodereuse:false", // workaround for https://github.com/Microsoft/vstest/issues/1503
-                "-nologo"
-            };
-
             // Extra msbuild properties won't be parsed and so end up in the UnmatchedTokens list. In addition to those
             // properties, all the test settings properties are also considered as unmatched but we don't want to forward
             // these as-is to msbuild. So we filter out the test settings properties from the unmatched tokens,
@@ -142,6 +155,13 @@ private static TestCommand FromParseResult(ParseResult result, string[] settings
                     .Concat(unMatchedNonSettingsArgs); // all tokens that the test-parser doesn't explicitly track (minus the settings tokens)
 
             VSTestTrace.SafeWriteTrace(() => $"MSBuild args from forwarded options: {string.Join(", ", parsedArgs)}");
+
+            var msbuildArgs = new List<string>(additionalBuildProperties)
+            {
+                "-target:VSTest",
+                "-nologo",
+            };
+
             msbuildArgs.AddRange(parsedArgs);
 
             if (settings.Any())
@@ -276,5 +296,37 @@ private static void SetEnvironmentVariablesFromParameters(TestCommand testComman
                 testCommand.EnvironmentVariable(name, value);
             }
         }
+
+        /// <returns>A case-insensitive dictionary of any properties passed from the user and their values.</returns>
+        private static Dictionary<string, string> GetUserSpecifiedExplicitMSBuildProperties(ParseResult parseResult)
+        {
+            Dictionary<string, string> globalProperties = new(StringComparer.OrdinalIgnoreCase);
+           IEnumerable<string> globalPropEnumerable = parseResult.UnmatchedTokens;
+            foreach (var unmatchedToken in globalPropEnumerable)
+            {
+                var propertyPairs = MSBuildPropertyParser.ParseProperties(unmatchedToken);
+                foreach (var propertyKeyValue in propertyPairs)
+                {
+                    string propertyName;
+                    if (propertyKeyValue.key.StartsWith("--property:", StringComparison.OrdinalIgnoreCase)
+                        || propertyKeyValue.key.StartsWith("/property:", StringComparison.OrdinalIgnoreCase))
+                    {
+                        propertyName = propertyKeyValue.key.RemovePrefix().Substring("property:".Length);
+                    }
+                    else if (propertyKeyValue.key.StartsWith("-p:", StringComparison.OrdinalIgnoreCase)
+                        || propertyKeyValue.key.StartsWith("/p:", StringComparison.OrdinalIgnoreCase))
+                    {
+                        propertyName = propertyKeyValue.key.RemovePrefix().Substring("p:".Length);
+                    }
+                    else
+                    {
+                        continue;
+                    }
+
+                    globalProperties[propertyName] = propertyKeyValue.value;
+                }
+            }
+            return globalProperties;
+        }
     }
 }

test/Microsoft.NET.TestFramework/Commands/DotnetTestCommand.cs

@@ -5,9 +5,15 @@ namespace Microsoft.NET.TestFramework.Commands
 {
     public class DotnetTestCommand : DotnetCommand
     {
-        public DotnetTestCommand(ITestOutputHelper log, params string[] args) : base(log)
+        public DotnetTestCommand(ITestOutputHelper log, bool disableNewOutput, params string[] args) : base(log)
         {
             Arguments.Add("test");
+            if (disableNewOutput)
+            {
+                Arguments.Add("--property:VsTestUseMSBuildOutput=false");
+                Arguments.Add("-tl:false");
+            }
+
             Arguments.AddRange(args);
         }
     }

test/dotnet-test.Tests/GivenDotnetTestBuildsAndRunsArtifactPostProcessing.cs

@@ -29,7 +29,7 @@ public void ArtifactPostProcessing_SolutionProjects(bool merge)
 
             string runsettings = GetRunsetting(testInstance.Path);
 
-            CommandResult result = new DotnetTestCommand(Log)
+            CommandResult result = new DotnetTestCommand(Log, disableNewOutput: true)
                                     .WithWorkingDirectory(testInstance.Path)
                                     .WithEnvironmentVariable(FeatureFlag.DISABLE_ARTIFACTS_POSTPROCESSING, "0")
                                     .Execute(
@@ -55,7 +55,7 @@ public void ArtifactPostProcessing_TestContainers(bool merge)
 
             new PublishCommand(Log, Path.Combine(testInstance.Path, "sln.sln")).Execute("/p:Configuration=Release").Should().Pass();
 
-            CommandResult result = new DotnetTestCommand(Log)
+            CommandResult result = new DotnetTestCommand(Log, disableNewOutput: false)
                                     .WithWorkingDirectory(testInstance.Path)
                                     .WithEnvironmentVariable(FeatureFlag.DISABLE_ARTIFACTS_POSTPROCESSING, "0")
                                     .WithEnvironmentVariable("DOTNET_CLI_VSTEST_TRACE", "1")

test/dotnet-test.Tests/GivenDotnetTestBuildsAndRunsTestFromCsprojForMultipleTFM.cs

@@ -31,7 +31,7 @@ public void MStestMultiTFM()
                 .Execute()
                 .Should().Pass();
 
-            var result = new DotnetTestCommand(Log, "-r", runtime)
+            var result = new DotnetTestCommand(Log, disableNewOutput: true, "-r", runtime)
                 .WithWorkingDirectory(testProjectDirectory)
                 .Execute(ConsoleLoggerOutputNormal);
 
@@ -68,7 +68,7 @@ public void XunitMultiTFM()
                 .Pass();
 
             // Call test
-            CommandResult result = new DotnetTestCommand(Log)
+            CommandResult result = new DotnetTestCommand(Log, disableNewOutput: true)
                                        .WithWorkingDirectory(testProjectDirectory)
                                        .Execute(ConsoleLoggerOutputNormal);
 
@@ -111,7 +111,7 @@ public void ItCreatesMergedCoverageFileForMultiTargetedProject()
             }
 
             // Call test
-            CommandResult result = new DotnetTestCommand(Log, ConsoleLoggerOutputNormal)
+            CommandResult result = new DotnetTestCommand(Log, disableNewOutput: true, ConsoleLoggerOutputNormal)
                                        .WithWorkingDirectory(testProjectDirectory)
                                        .Execute("--collect", "Code Coverage", "--results-directory", resultsDirectory);
 
@@ -131,7 +131,7 @@ public void ItCanTestAMultiTFMProjectWithImplicitRestore()
 
             string projectDirectory = Path.Combine(testInstance.Path, "XUnitProject");
 
-            new DotnetTestCommand(Log, ConsoleLoggerOutputNormal)
+            new DotnetTestCommand(Log, disableNewOutput: true, ConsoleLoggerOutputNormal)
                .WithWorkingDirectory(projectDirectory)
                .Execute("--framework", ToolsetInfo.CurrentTargetFramework)
                .Should().Pass();
@@ -179,7 +179,7 @@ public void TestSlnWithMultitargetedProject()
                 .Should()
                 .Pass();
 
-            new DotnetTestCommand(Log, ConsoleLoggerOutputNormal)
+            new DotnetTestCommand(Log, disableNewOutput: true, ConsoleLoggerOutputNormal)
                .WithWorkingDirectory(testAsset.TestRoot)
                .Execute()
                .Should().Pass();

test/dotnet-test.Tests/GivenDotnetTestBuildsAndRunsTestFromDll.cs

@@ -34,7 +34,7 @@ public void TestsFromAGivenContainerShouldRunWithExpectedOutput()
             var outputDll = Path.Combine(buildCommand.GetOutputDirectory(configuration: configuration).FullName, $"{testAppName}.dll");
 
             // Call vstest
-            var result = new DotnetTestCommand(Log)
+            var result = new DotnetTestCommand(Log, disableNewOutput: false)
                 .Execute(outputDll, "--logger:console;verbosity=normal");
             if (!TestContext.IsLocalized())
             {
@@ -68,7 +68,7 @@ public void ItSetsDotnetRootToTheLocationOfDotnetExecutableWhenRunningDotnetTest
             var outputDll = Path.Combine(testRoot, "bin", configuration, ToolsetInfo.CurrentTargetFramework, $"{testAppName}.dll");
 
             // Call dotnet test + dll
-            var result = new DotnetTestCommand(Log)
+            var result = new DotnetTestCommand(Log, disableNewOutput: true)
                 .Execute(outputDll, "--logger:console;verbosity=normal");
 
             result.ExitCode.Should().Be(1);
@@ -98,7 +98,7 @@ public void TestsFromAGivenContainerAndArchSwitchShouldFlowToVsTestConsole()
             var outputDll = Path.Combine(testRoot, "bin", configuration, ToolsetInfo.CurrentTargetFramework, $"{testAppName}.dll");
 
             // Call vstest
-            var result = new DotnetTestCommand(Log)
+            var result = new DotnetTestCommand(Log, disableNewOutput: true)
                 .Execute(outputDll, "--arch", "wrongArchitecture");
             if (!TestContext.IsLocalized())
             {
@@ -122,7 +122,7 @@ public void MissingOutputDllAndArgumentsEndWithDllOrExeShouldFailInMSBuild(strin
                 .Execute()
                 .Should().Pass();
 
-            var result = new DotnetTestCommand(Log)
+            var result = new DotnetTestCommand(Log, disableNewOutput: true)
                 .Execute(arg);
             if (!TestContext.IsLocalized())
             {