Skip to content

Commit 91b64a2

Browse files
EvangelinkEvangelink
authored
Merge pull request #27961 from baronfel/handle-test-parser-forwarded-args
Closes microsoft/vstest#4014
2 parents 980d62a + 91c5e63 commit 91b64a2

File tree

7 files changed

+120
-33
lines changed

7 files changed

+120
-33
lines changed

‎src/Assets/TestProjects/VSTestMSBuildParameters/Tests.cs‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
// Copyright (c) .NET Foundation and contributors. All rights reserved.
2+
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
3+
4+
using Microsoft.VisualStudio.TestTools.UnitTesting;
5+
using System;
6+
using System.Collections;
7+
using System.Reflection;
8+
9+
namespace TestNamespace
10+
{
11+
[TestClass]
12+
public class Tests
13+
{
14+
[TestMethod]
15+
public void TestMSBuildParameters()
16+
{
17+
var assemblyInfoVersion = Assembly.GetExecutingAssembly().GetCustomAttribute<AssemblyInformationalVersionAttribute>().InformationalVersion;
18+
Assert.AreEqual("1.2.3", assemblyInfoVersion);
19+
}
20+
}
21+
}

‎src/Assets/TestProjects/VSTestMSBuildParameters/VSTestMSBuildParameters.csproj‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
<Project Sdk="Microsoft.NET.Sdk" ToolsVersion="15.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
2+
<Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), testAsset.props))\testAsset.props" />
3+
4+
<PropertyGroup>
5+
<OutputType>Exe</OutputType>
6+
<TargetFramework>$(CurrentTargetFramework)</TargetFramework>
7+
</PropertyGroup>
8+
9+
<ItemGroup>
10+
<PackageReference Include="MSTest.TestFramework" Version="$(MSTestVersion)" />
11+
<PackageReference Include="MSTest.TestAdapter" Version="$(MSTestVersion)" />
12+
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="$(MicrosoftNETTestSdkPackageVersion)" />
13+
</ItemGroup>
14+
15+
<ItemGroup>
16+
<!-- Microsoft.NET.Test.Sdk package includes source files which shouldn't be automatically included. -->
17+
<!-- Excluding those -->
18+
<Compile Remove="pkgs\Microsoft.NET.Test.Sdk\**" />
19+
</ItemGroup>
20+
</Project>

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

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -118,29 +118,29 @@ private static TestCommand FromParseResult(ParseResult result, string[] settings
118118
"-nologo"
119119
};
120120

121-
msbuildArgs.AddRange(result.OptionValuesToBeForwarded(TestCommandParser.GetCommand()));
121+
// Extra msbuild properties won't be parsed and so end up in the UnmatchedTokens list. In addition to those
122+
// properties, all the test settings properties are also considered as unmatched but we don't want to forward
123+
// these as-is to msbuild. So we filter out the test settings properties from the unmatched tokens,
124+
// by only taking values until the first item after `--`. (`--` is not present in the UnmatchedTokens).
125+
var unMatchedNonSettingsArgs = settings.Length > 1
126+
? result.UnmatchedTokens.TakeWhile(x => x != settings[1])
127+
: result.UnmatchedTokens;
128+
129+
var parsedArgs =
130+
result.OptionValuesToBeForwarded(TestCommandParser.GetCommand()) // all msbuild-recognized tokens
131+
.Concat(unMatchedNonSettingsArgs); // all tokens that the test-parser doesn't explicitly track (minus the settings tokens)
132+
133+
VSTestTrace.SafeWriteTrace(() => $"MSBuild args from forwarded options: {String.Join(", ", parsedArgs)}" );
134+
msbuildArgs.AddRange(parsedArgs);
122135

123136
if (settings.Any())
124137
{
125-
//workaround for correct -- logic
126-
var commandArgument = result.GetValueForArgument(TestCommandParser.SlnOrProjectArgument);
127-
if(!string.IsNullOrWhiteSpace(commandArgument) && !settings.Contains(commandArgument))
128-
{
129-
msbuildArgs.Add(result.GetValueForArgument(TestCommandParser.SlnOrProjectArgument));
130-
}
131-
132138
// skip '--' and escape every \ to be \\ and every " to be \" to survive the next hop
133139
string[] escaped = settings.Skip(1).Select(s => s.Replace("\\", "\\\\").Replace("\"", "\\\"")).ToArray();
134140

135141
string runSettingsArg = string.Join(";", escaped);
136142
msbuildArgs.Add($"-property:VSTestCLIRunSettings=\"{runSettingsArg}\"");
137143
}
138-
else
139-
{
140-
var argument = result.GetValueForArgument(TestCommandParser.SlnOrProjectArgument);
141-
if(!string.IsNullOrWhiteSpace(argument))
142-
msbuildArgs.Add(argument);
143-
}
144144

145145
string verbosityArg = result.ForwardedOptionValues<IReadOnlyCollection<string>>(TestCommandParser.GetCommand(), "verbosity")?.SingleOrDefault() ?? null;
146146
if (verbosityArg != null)

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

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,6 @@ internal static class TestCommandParser
1717
{
1818
public static readonly string DocsLink = "https://aka.ms/dotnet-test";
1919

20-
public static readonly Argument<string> SlnOrProjectArgument = new Argument<string>(CommonLocalizableStrings.SolutionOrProjectArgumentName)
21-
{
22-
Description = CommonLocalizableStrings.SolutionOrProjectArgumentDescription,
23-
Arity = ArgumentArity.ZeroOrOne
24-
};
25-
2620
public static readonly Option<string> SettingsOption = new ForwardedOption<string>(new string[] { "-s", "--settings" }, LocalizableStrings.CmdSettingsDescription)
2721
{
2822
ArgumentHelpName = LocalizableStrings.CmdSettingsFile
@@ -130,7 +124,9 @@ private static Command ConstructCommand()
130124
{
131125
var command = new DocumentedCommand("test", DocsLink, LocalizableStrings.AppFullName);
132126
command.TreatUnmatchedTokensAsErrors = false;
133-
command.AddArgument(SlnOrProjectArgument);
127+
128+
// We are on purpose not capturing the solution, project or directory here. We want to pass it to the
129+
// MSBuild command so we are letting it flow.
134130

135131
command.AddOption(SettingsOption);
136132
command.AddOption(ListTestsOption);

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,12 @@ public TestCommand WithWorkingDirectory(string workingDirectory)
4646
return this;
4747
}
4848

49+
public TestCommand WithTraceOutput()
50+
{
51+
WithEnvironmentVariable("DOTNET_CLI_VSTEST_TRACE", "1");
52+
return this;
53+
}
54+
4955
private SdkCommandSpec CreateCommandSpec(IEnumerable<string> args)
5056
{
5157
var commandSpec = CreateCommand(args);

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

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -34,12 +34,8 @@ public void GivenAProjectAndMultipleTestRunParametersItPassesThemToVStestConsole
3434
.WithWorkingDirectory(testProjectDirectory)
3535
.Execute(ConsoleLoggerOutputNormal.Concat(new[] {
3636
"--",
37-
"TestRunParameters.Parameter(name=\"myParam\",",
38-
"value=\"value\")",
39-
"TestRunParameters.Parameter(name=\"myParam2\",",
40-
"value=\"value",
41-
"with",
42-
"space\")"
37+
"TestRunParameters.Parameter(name=\"myParam\",value=\"value\")",
38+
"TestRunParameters.Parameter(name=\"myParam2\",value=\"value with space\")"
4339
}));
4440

4541
// Verify
@@ -72,12 +68,8 @@ public void GivenADllAndMultipleTestRunParametersItPassesThemToVStestConsoleInTh
7268
.Execute(ConsoleLoggerOutputNormal.Concat(new[] {
7369
outputDll,
7470
"--",
75-
"TestRunParameters.Parameter(name=\"myParam\",",
76-
"value=\"value\")",
77-
"TestRunParameters.Parameter(name=\"myParam2\",",
78-
"value=\"value",
79-
"with",
80-
"space\")"
71+
"TestRunParameters.Parameter(name=\"myParam\",value=\"value\")",
72+
"TestRunParameters.Parameter(name=\"myParam2\",value=\"value with space\")"
8173
}));
8274

8375
// Verify

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

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
// Copyright (c) .NET Foundation and contributors. All rights reserved.
2+
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
3+
4+
using Microsoft.DotNet.Tools.Test.Utilities;
5+
using Xunit;
6+
using FluentAssertions;
7+
using Microsoft.DotNet.Cli.Utils;
8+
using System.IO;
9+
using System;
10+
using Microsoft.NET.TestFramework;
11+
using Microsoft.NET.TestFramework.Assertions;
12+
using Microsoft.NET.TestFramework.Commands;
13+
using Xunit.Abstractions;
14+
15+
namespace Microsoft.DotNet.Cli.Test.Tests
16+
{
17+
public class GivenDotnetTestContainsMSBuildParameters : SdkTest
18+
{
19+
private const string TestAppName = "VSTestMSBuildParameters";
20+
private const string MSBuildParameter = "/p:Version=1.2.3";
21+
22+
public GivenDotnetTestContainsMSBuildParameters(ITestOutputHelper log) : base(log)
23+
{
24+
}
25+
26+
[InlineData($"{TestAppName}.csproj")]
27+
[InlineData(null)]
28+
[Theory]
29+
public void ItPassesEnvironmentVariablesFromCommandLineParametersWhenRunningViaCsproj(string projectName)
30+
{
31+
var testAsset = _testAssetsManager.CopyTestAsset(TestAppName)
32+
.WithSource()
33+
.WithVersionVariables();
34+
35+
var testRoot = testAsset.Path;
36+
37+
CommandResult result = (projectName is null ? new DotnetTestCommand(Log) : new DotnetTestCommand(Log, projectName))
38+
.WithWorkingDirectory(testRoot)
39+
.Execute("--logger", "console;verbosity=detailed", MSBuildParameter);
40+
41+
if (!TestContext.IsLocalized())
42+
{
43+
result.StdOut
44+
.Should().Contain("Total tests: 1")
45+
.And.Contain("Passed: 1")
46+
.And.Contain("Passed TestMSBuildParameters");
47+
}
48+
49+
result.ExitCode.Should().Be(0);
50+
}
51+
}
52+
}

0 commit comments

Comments
 (0)