Handle test parser forwarded args

src/Assets/TestProjects/VSTestMSBuildParameters/Tests.cs

@@ -0,0 +1,21 @@
+// Copyright (c) .NET Foundation and contributors. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+using System;
+using System.Collections;
+using System.Reflection;
+
+namespace TestNamespace
+{
+    [TestClass]
+    public class Tests
+    {
+        [TestMethod]
+        public void TestMSBuildParameters()
+        {
+            var assemblyInfoVersion = Assembly.GetExecutingAssembly().GetCustomAttribute<AssemblyInformationalVersionAttribute>().InformationalVersion;
+            Assert.AreEqual("1.2.3", assemblyInfoVersion);
+        }
+    }
+}

src/Assets/TestProjects/VSTestMSBuildParameters/VSTestMSBuildParameters.csproj

@@ -0,0 +1,20 @@
+<Project Sdk="Microsoft.NET.Sdk" ToolsVersion="15.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
+  <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), testAsset.props))\testAsset.props" />
+
+  <PropertyGroup>
+    <OutputType>Exe</OutputType>
+    <TargetFramework>$(CurrentTargetFramework)</TargetFramework>
+  </PropertyGroup>
+
+  <ItemGroup>
+    <PackageReference Include="MSTest.TestFramework" Version="$(MSTestVersion)" />
+    <PackageReference Include="MSTest.TestAdapter" Version="$(MSTestVersion)" />
+    <PackageReference Include="Microsoft.NET.Test.Sdk" Version="$(MicrosoftNETTestSdkPackageVersion)" />
+  </ItemGroup>
+
+  <ItemGroup>
+    <!-- Microsoft.NET.Test.Sdk package includes source files which shouldn't be automatically included. -->
+    <!-- Excluding those -->
+    <Compile Remove="pkgs\Microsoft.NET.Test.Sdk\**" />
+  </ItemGroup>
+</Project>

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

@@ -5,6 +5,7 @@
 using System.Collections.Generic;
 using System.CommandLine;
 using System.CommandLine.Parsing;
+using System.Diagnostics;
 using System.IO;
 using System.Linq;
 using Microsoft.DotNet.Cli;
@@ -56,10 +57,10 @@ public static int Run(ParseResult parseResult)
                 return ForwardToVSTestConsole(parseResult, args, settings, testSessionCorrelationId);
             }
 
-            return ForwardToMsbuild(parseResult, settings, testSessionCorrelationId);
+            return ForwardToMsbuild(parseResult, args, settings, testSessionCorrelationId);
         }
 
-        private static int ForwardToMsbuild(ParseResult parseResult, string[] settings, string testSessionCorrelationId)
+        private static int ForwardToMsbuild(ParseResult parseResult, string[] args, string[] settings, string testSessionCorrelationId)
         {
             // Workaround for https://github.com/Microsoft/vstest/issues/1503
             const string NodeWindowEnvironmentName = "MSBUILDENSURESTDOUTFORTASKPROCESSES";
@@ -118,29 +119,29 @@ private static TestCommand FromParseResult(ParseResult result, string[] settings
                 "-nologo"
             };
 
-            msbuildArgs.AddRange(result.OptionValuesToBeForwarded(TestCommandParser.GetCommand()));
+            // 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,
+            // by only taking values until the first item after `--`. (`--` is not present in the UnmatchedTokens ).
+            var unMatchedNonSettingsArgs = settings.Length > 1
+                ? result.UnmatchedTokens.TakeWhile(x => x != settings[1])
+                : result.UnmatchedTokens;
+
+            var parsedArgs =
+                result.OptionValuesToBeForwarded(TestCommandParser.GetCommand()) // all msbuild-recognized tokens
+                    .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)}" );
+            msbuildArgs.AddRange(parsedArgs);
 
             if (settings.Any())
             {
-                //workaround for correct -- logic
-                var commandArgument = result.GetValueForArgument(TestCommandParser.SlnOrProjectArgument);
-                if(!string.IsNullOrWhiteSpace(commandArgument) && !settings.Contains(commandArgument))
-                {
-                    msbuildArgs.Add(result.GetValueForArgument(TestCommandParser.SlnOrProjectArgument));
-                }
-
                 // skip '--' and escape every \ to be \\ and every " to be \" to survive the next hop
                 string[] escaped = settings.Skip(1).Select(s => s.Replace("\\", "\\\\").Replace("\"", "\\\"")).ToArray();
 
                 string runSettingsArg = string.Join(";", escaped);
                 msbuildArgs.Add($"-property:VSTestCLIRunSettings=\"{runSettingsArg}\"");
             }
-            else
-            {
-                var argument = result.GetValueForArgument(TestCommandParser.SlnOrProjectArgument);
-                if(!string.IsNullOrWhiteSpace(argument))
-                    msbuildArgs.Add(argument);
-            }
 
             string verbosityArg = result.ForwardedOptionValues<IReadOnlyCollection<string>>(TestCommandParser.GetCommand(), "verbosity")?.SingleOrDefault() ?? null;
             if (verbosityArg != null)

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

@@ -15,13 +15,7 @@ namespace Microsoft.DotNet.Cli
 {
     internal static class TestCommandParser
     {
-        public static readonly string DocsLink = "https://aka.ms/dotnet-test";
-
-        public static readonly Argument<string> SlnOrProjectArgument = new Argument<string>(CommonLocalizableStrings.SolutionOrProjectArgumentName)
-        {
-            Description = CommonLocalizableStrings.SolutionOrProjectArgumentDescription,
-            Arity = ArgumentArity.ZeroOrOne
-        };
+        public static readonly string DocsLink = "https://aka.ms/dotnet-test";        
 
         public static readonly Option<string> SettingsOption = new ForwardedOption<string>(new string[] { "-s", "--settings" }, LocalizableStrings.CmdSettingsDescription)
         {
@@ -130,7 +124,9 @@ private static Command ConstructCommand()
         {
             var command = new DocumentedCommand("test", DocsLink, LocalizableStrings.AppFullName);
             command.TreatUnmatchedTokensAsErrors = false;
-            command.AddArgument(SlnOrProjectArgument);
+
+            // We are on purpose not capturing the solution, project or directory here. We want to pass it to the
+            // MSBuild command so we are letting it flow.
 
             command.AddOption(SettingsOption);
             command.AddOption(ListTestsOption);

src/Tests/Microsoft.NET.TestFramework/Commands/TestCommand.cs

@@ -46,6 +46,12 @@ public TestCommand WithWorkingDirectory(string workingDirectory)
             return this;
         }
 
+        public TestCommand WithTraceOutput()
+        {
+            WithEnvironmentVariable("DOTNET_CLI_VSTEST_TRACE", "1");
+            return this;
+        }
+
         private SdkCommandSpec CreateCommandSpec(IEnumerable<string> args)
         {
             var commandSpec = CreateCommand(args);

src/Tests/dotnet-test.Tests/GivenDotnetTestBuildsAndRunsTestfromCsprojWithCorrectTestRunParameters.cs

@@ -34,12 +34,8 @@ public void GivenAProjectAndMultipleTestRunParametersItPassesThemToVStestConsole
                                         .WithWorkingDirectory(testProjectDirectory)
                                         .Execute(ConsoleLoggerOutputNormal.Concat(new[] {
                                             "--",
-                                            "TestRunParameters.Parameter(name=\"myParam\",",
-                                            "value=\"value\")",
-                                            "TestRunParameters.Parameter(name=\"myParam2\",",
-                                            "value=\"value", 
-                                            "with", 
-                                            "space\")"
+                                            "TestRunParameters.Parameter(name=\"myParam\",value=\"value\")",
+                                            "TestRunParameters.Parameter(name=\"myParam2\",value=\"value with space\")"
                                         }));
 
             // Verify
@@ -72,12 +68,8 @@ public void GivenADllAndMultipleTestRunParametersItPassesThemToVStestConsoleInTh
                                         .Execute(ConsoleLoggerOutputNormal.Concat(new[] {
                                             outputDll,
                                             "--",
-                                            "TestRunParameters.Parameter(name=\"myParam\",",
-                                            "value=\"value\")",
-                                            "TestRunParameters.Parameter(name=\"myParam2\",",
-                                            "value=\"value",
-                                            "with",
-                                            "space\")"
+                                            "TestRunParameters.Parameter(name=\"myParam\",value=\"value\")",
+                                            "TestRunParameters.Parameter(name=\"myParam2\",value=\"value with space\")"
                                         }));
 
             // Verify

src/Tests/dotnet-test.Tests/GivenDotnetTestContainsMSBuildParameters.cs

@@ -0,0 +1,52 @@
+// Copyright (c) .NET Foundation and contributors. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+
+using Microsoft.DotNet.Tools.Test.Utilities;
+using Xunit;
+using FluentAssertions;
+using Microsoft.DotNet.Cli.Utils;
+using System.IO;
+using System;
+using Microsoft.NET.TestFramework;
+using Microsoft.NET.TestFramework.Assertions;
+using Microsoft.NET.TestFramework.Commands;
+using Xunit.Abstractions;
+
+namespace Microsoft.DotNet.Cli.Test.Tests
+{
+    public class GivenDotnetTestContainsMSBuildParameters : SdkTest
+    {
+        private const string TestAppName = "VSTestMSBuildParameters";
+        private const string MSBuildParameter = "/p:Version=1.2.3";
+
+        public GivenDotnetTestContainsMSBuildParameters(ITestOutputHelper log) : base(log)
+        {
+        }
+
+        [InlineData($"{TestAppName}.csproj")]
+        [InlineData(null)]
+        [Theory]
+        public void ItPassesEnvironmentVariablesFromCommandLineParametersWhenRunningViaCsproj(string projectName)
+        {
+            var testAsset = _testAssetsManager.CopyTestAsset(TestAppName)
+                .WithSource()
+                .WithVersionVariables();
+
+            var testRoot = testAsset.Path;
+
+            CommandResult result = (projectName is null ? new DotnetTestCommand(Log) : new DotnetTestCommand(Log, projectName))
+                                    .WithWorkingDirectory(testRoot)
+                                    .Execute("--logger", "console;verbosity=detailed", MSBuildParameter);
+
+            if (!TestContext.IsLocalized())
+            {
+                result.StdOut
+                    .Should().Contain("Total tests: 1")
+                    .And.Contain("Passed: 1")
+                    .And.Contain("Passed TestMSBuildParameters");
+            }
+
+            result.ExitCode.Should().Be(0);
+        }
+    }
+}