Skip to content

Commit fec417b

Browse files
Youssef1313Youssef1313
authored
Follow-up to dotnet test parsing (#51331)
1 parent f6115a4 commit fec417b

File tree

2 files changed

+97
-11
lines changed

2 files changed

+97
-11
lines changed

‎src/Cli/dotnet/Commands/Test/VSTest/TestCommand.cs‎

Lines changed: 28 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// The .NET Foundation licenses this file to you under the MIT license.
33

44
using System.CommandLine;
5+
using System.Diagnostics;
56
using System.Diagnostics.CodeAnalysis;
67
using System.Runtime.Versioning;
78
using System.Text.RegularExpressions;
@@ -39,21 +40,41 @@ public static int Run(ParseResult parseResult)
3940
VSTestTrace.SafeWriteTrace(() => $"Argument list: '{commandLineParameters}'");
4041
}
4142

42-
// settings parameters are after -- (including --), these should not be considered by the parser
43-
string[] settings = [.. args.SkipWhile(a => a != "--")];
44-
// all parameters before --
45-
args = [.. args.TakeWhile(a => a != "--")];
43+
(args, string[] settings) = SeparateSettingsFromArgs(args);
4644

4745
// Fix for https://github.com/Microsoft/vstest/issues/1453
4846
// Run dll/exe directly using the VSTestForwardingApp
49-
if (ContainsBuiltTestSources(parseResult))
47+
// Note: ContainsBuiltTestSources need to know how many settings are there, to skip those from unmatched tokens
48+
// When we don't have settings, we pass 0.
49+
// When we have settings, we want to exclude the '--' as it doesn't end up in unmatched tokens, so we pass settings.Length - 1
50+
if (ContainsBuiltTestSources(parseResult, GetSettingsCount(settings)))
5051
{
5152
return ForwardToVSTestConsole(parseResult, args, settings, testSessionCorrelationId);
5253
}
5354

5455
return ForwardToMsbuild(parseResult, settings, testSessionCorrelationId);
5556
}
5657

58+
internal /*internal for testing*/ static (string[] Args, string[] Settings) SeparateSettingsFromArgs(string[] args)
59+
{
60+
// settings parameters are after -- (including --), these should not be considered by the parser
61+
string[] settings = [.. args.SkipWhile(a => a != "--")];
62+
// all parameters before --
63+
args = [.. args.TakeWhile(a => a != "--")];
64+
return (args, settings);
65+
}
66+
67+
internal /*internal for testing*/ static int GetSettingsCount(string[] settings)
68+
{
69+
if (settings.Length == 0)
70+
{
71+
return 0;
72+
}
73+
74+
Debug.Assert(settings[0] == "--", "Settings should start with --");
75+
return settings.Length - 1;
76+
}
77+
5778
private static int ForwardToMsbuild(ParseResult parseResult, string[] settings, string testSessionCorrelationId)
5879
{
5980
// Workaround for https://github.com/Microsoft/vstest/issues/1503
@@ -295,9 +316,9 @@ internal static int RunArtifactPostProcessingIfNeeded(string testSessionCorrelat
295316
}
296317
}
297318

298-
private static bool ContainsBuiltTestSources(ParseResult parseResult)
319+
internal /*internal for testing*/ static bool ContainsBuiltTestSources(ParseResult parseResult, int settingsLength)
299320
{
300-
for (int i = 0; i < parseResult.UnmatchedTokens.Count; i++)
321+
for (int i = 0; i < parseResult.UnmatchedTokens.Count - settingsLength; i++)
301322
{
302323
string arg = parseResult.UnmatchedTokens[i];
303324
if (!arg.StartsWith("-") &&

‎test/dotnet.Tests/CommandTests/Test/TestCommandParserTests.cs‎

Lines changed: 69 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,9 @@
11
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4-
#nullable disable
5-
6-
using System.CommandLine;
74
using Microsoft.DotNet.Cli.Commands.Test;
5+
using Microsoft.DotNet.Cli.Extensions;
6+
using TestCommand = Microsoft.DotNet.Cli.Commands.Test.TestCommand;
87

98
namespace Microsoft.DotNet.Cli.Test.Tests
109
{
@@ -14,7 +13,7 @@ public class TestCommandParserTests
1413
public void SurroundWithDoubleQuotesWithNullThrows()
1514
{
1615
Assert.Throws<ArgumentNullException>(() =>
17-
TestCommandParser.SurroundWithDoubleQuotes(null));
16+
TestCommandParser.SurroundWithDoubleQuotes(null!));
1817
}
1918

2019
[Theory]
@@ -74,5 +73,71 @@ public void VSTestCommandIncludesPropertiesOption()
7473
propertyOption.Should().NotBeNull("VSTest command should include CommonOptions.PropertiesOption to support /p Property=Value syntax");
7574
propertyOption.Aliases.Should().Contain("/p", "PropertiesOption should include /p alias for MSBuild compatibility");
7675
}
76+
77+
[Fact]
78+
public void DllDetectionShouldExcludeRunArgumentsAndGlobalProperties()
79+
{
80+
var parseResult = Parser.Parse("""test -p:"RunConfig=abd.dll" -- RunConfig=abd.dll -p:"RunConfig=abd.dll" --results-directory hey.dll""");
81+
var args = parseResult.GetArguments();
82+
83+
(args, string[] settings) = TestCommand.SeparateSettingsFromArgs(args);
84+
int settingsCount = TestCommand.GetSettingsCount(settings);
85+
settingsCount.Should().Be(4);
86+
87+
// Our unmatched tokens for this test case are only the settings (after the `--`).
88+
Assert.Equal(settingsCount, parseResult.UnmatchedTokens.Count);
89+
90+
Assert.Equal("--", settings[0]);
91+
Assert.Equal(settings.Length, settingsCount + 1);
92+
for (int i = 1; i <= settingsCount; i++)
93+
{
94+
Assert.Equal(settings[^i], parseResult.UnmatchedTokens[^i]);
95+
}
96+
97+
TestCommand.ContainsBuiltTestSources(parseResult, settingsCount).Should().Be(false);
98+
}
99+
100+
[Fact]
101+
public void DllDetectionShouldBeTrueWhenPresentAloneEvenIfDuplicatedInSettings()
102+
{
103+
var parseResult = Parser.Parse("""test abd.dll -- abd.dll""");
104+
var args = parseResult.GetArguments();
105+
106+
(args, string[] settings) = TestCommand.SeparateSettingsFromArgs(args);
107+
int settingsCount = TestCommand.GetSettingsCount(settings);
108+
settingsCount.Should().Be(1);
109+
110+
// Our unmatched tokens here are all the settings, plus the abd.dll before the `--`.
111+
Assert.Equal(settingsCount + 1, parseResult.UnmatchedTokens.Count);
112+
113+
Assert.Equal("--", settings[0]);
114+
Assert.Equal(settings.Length, settingsCount + 1);
115+
for (int i = 1; i <= settingsCount; i++)
116+
{
117+
Assert.Equal(settings[^i], parseResult.UnmatchedTokens[^i]);
118+
}
119+
120+
TestCommand.ContainsBuiltTestSources(parseResult, settingsCount).Should().Be(true);
121+
}
122+
123+
[Theory]
124+
[InlineData("abd.dll", true)]
125+
[InlineData("abd.dll --", true)]
126+
[InlineData("-dl:abd.dll", false)]
127+
[InlineData("-dl:abd.dll --", false)]
128+
[InlineData("-abcd:abd.dll", false)]
129+
[InlineData("-abcd:abd.dll --", false)]
130+
[InlineData("-p:abd.dll", false)]
131+
[InlineData("-p:abd.dll --", false)]
132+
public void DllDetectionShouldWorkWhenNoSettings(string testArgs, bool expectedContainsBuiltTestSource)
133+
{
134+
var parseResult = Parser.Parse($"test {testArgs}");
135+
var args = parseResult.GetArguments();
136+
137+
(args, string[] settings) = TestCommand.SeparateSettingsFromArgs(args);
138+
int settingsCount = TestCommand.GetSettingsCount(settings);
139+
settingsCount.Should().Be(0);
140+
TestCommand.ContainsBuiltTestSources(parseResult, settingsCount).Should().Be(expectedContainsBuiltTestSource);
141+
}
77142
}
78143
}

0 commit comments

Comments
 (0)