diff --git a/src/Cli/dotnet/Commands/Test/VSTest/TestCommand.cs b/src/Cli/dotnet/Commands/Test/VSTest/TestCommand.cs index 0011141287c6..9a1700f82a80 100644 --- a/src/Cli/dotnet/Commands/Test/VSTest/TestCommand.cs +++ b/src/Cli/dotnet/Commands/Test/VSTest/TestCommand.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.CommandLine; +using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Runtime.Versioning; using System.Text.RegularExpressions; @@ -39,14 +40,14 @@ public static int Run(ParseResult parseResult) VSTestTrace.SafeWriteTrace(() => $"Argument list: '{commandLineParameters}'"); } - // settings parameters are after -- (including --), these should not be considered by the parser - string[] settings = [.. args.SkipWhile(a => a != "--")]; - // all parameters before -- - args = [.. args.TakeWhile(a => a != "--")]; + (args, string[] settings) = SeparateSettingsFromArgs(args); // Fix for https://github.com/Microsoft/vstest/issues/1453 // Run dll/exe directly using the VSTestForwardingApp - if (ContainsBuiltTestSources(parseResult)) + // Note: ContainsBuiltTestSources need to know how many settings are there, to skip those from unmatched tokens + // When we don't have settings, we pass 0. + // When we have settings, we want to exclude the '--' as it doesn't end up in unmatched tokens, so we pass settings.Length - 1 + if (ContainsBuiltTestSources(parseResult, GetSettingsCount(settings))) { return ForwardToVSTestConsole(parseResult, args, settings, testSessionCorrelationId); } @@ -54,6 +55,26 @@ public static int Run(ParseResult parseResult) return ForwardToMsbuild(parseResult, settings, testSessionCorrelationId); } + internal /*internal for testing*/ static (string[] Args, string[] Settings) SeparateSettingsFromArgs(string[] args) + { + // settings parameters are after -- (including --), these should not be considered by the parser + string[] settings = [.. args.SkipWhile(a => a != "--")]; + // all parameters before -- + args = [.. args.TakeWhile(a => a != "--")]; + return (args, settings); + } + + internal /*internal for testing*/ static int GetSettingsCount(string[] settings) + { + if (settings.Length == 0) + { + return 0; + } + + Debug.Assert(settings[0] == "--", "Settings should start with --"); + return settings.Length - 1; + } + private static int ForwardToMsbuild(ParseResult parseResult, string[] settings, string testSessionCorrelationId) { // Workaround for https://github.com/Microsoft/vstest/issues/1503 @@ -295,9 +316,9 @@ internal static int RunArtifactPostProcessingIfNeeded(string testSessionCorrelat } } - private static bool ContainsBuiltTestSources(ParseResult parseResult) + internal /*internal for testing*/ static bool ContainsBuiltTestSources(ParseResult parseResult, int settingsLength) { - for (int i = 0; i < parseResult.UnmatchedTokens.Count; i++) + for (int i = 0; i < parseResult.UnmatchedTokens.Count - settingsLength; i++) { string arg = parseResult.UnmatchedTokens[i]; if (!arg.StartsWith("-") && diff --git a/test/dotnet.Tests/CommandTests/Test/TestCommandParserTests.cs b/test/dotnet.Tests/CommandTests/Test/TestCommandParserTests.cs index 9b86d3c92c4c..1c780c37bc14 100644 --- a/test/dotnet.Tests/CommandTests/Test/TestCommandParserTests.cs +++ b/test/dotnet.Tests/CommandTests/Test/TestCommandParserTests.cs @@ -1,10 +1,9 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -#nullable disable - -using System.CommandLine; using Microsoft.DotNet.Cli.Commands.Test; +using Microsoft.DotNet.Cli.Extensions; +using TestCommand = Microsoft.DotNet.Cli.Commands.Test.TestCommand; namespace Microsoft.DotNet.Cli.Test.Tests { @@ -14,7 +13,7 @@ public class TestCommandParserTests public void SurroundWithDoubleQuotesWithNullThrows() { Assert.Throws(() => - TestCommandParser.SurroundWithDoubleQuotes(null)); + TestCommandParser.SurroundWithDoubleQuotes(null!)); } [Theory] @@ -74,5 +73,71 @@ public void VSTestCommandIncludesPropertiesOption() propertyOption.Should().NotBeNull("VSTest command should include CommonOptions.PropertiesOption to support /p Property=Value syntax"); propertyOption.Aliases.Should().Contain("/p", "PropertiesOption should include /p alias for MSBuild compatibility"); } + + [Fact] + public void DllDetectionShouldExcludeRunArgumentsAndGlobalProperties() + { + var parseResult = Parser.Parse("""test -p:"RunConfig=abd.dll" -- RunConfig=abd.dll -p:"RunConfig=abd.dll" --results-directory hey.dll"""); + var args = parseResult.GetArguments(); + + (args, string[] settings) = TestCommand.SeparateSettingsFromArgs(args); + int settingsCount = TestCommand.GetSettingsCount(settings); + settingsCount.Should().Be(4); + + // Our unmatched tokens for this test case are only the settings (after the `--`). + Assert.Equal(settingsCount, parseResult.UnmatchedTokens.Count); + + Assert.Equal("--", settings[0]); + Assert.Equal(settings.Length, settingsCount + 1); + for (int i = 1; i <= settingsCount; i++) + { + Assert.Equal(settings[^i], parseResult.UnmatchedTokens[^i]); + } + + TestCommand.ContainsBuiltTestSources(parseResult, settingsCount).Should().Be(false); + } + + [Fact] + public void DllDetectionShouldBeTrueWhenPresentAloneEvenIfDuplicatedInSettings() + { + var parseResult = Parser.Parse("""test abd.dll -- abd.dll"""); + var args = parseResult.GetArguments(); + + (args, string[] settings) = TestCommand.SeparateSettingsFromArgs(args); + int settingsCount = TestCommand.GetSettingsCount(settings); + settingsCount.Should().Be(1); + + // Our unmatched tokens here are all the settings, plus the abd.dll before the `--`. + Assert.Equal(settingsCount + 1, parseResult.UnmatchedTokens.Count); + + Assert.Equal("--", settings[0]); + Assert.Equal(settings.Length, settingsCount + 1); + for (int i = 1; i <= settingsCount; i++) + { + Assert.Equal(settings[^i], parseResult.UnmatchedTokens[^i]); + } + + TestCommand.ContainsBuiltTestSources(parseResult, settingsCount).Should().Be(true); + } + + [Theory] + [InlineData("abd.dll", true)] + [InlineData("abd.dll --", true)] + [InlineData("-dl:abd.dll", false)] + [InlineData("-dl:abd.dll --", false)] + [InlineData("-abcd:abd.dll", false)] + [InlineData("-abcd:abd.dll --", false)] + [InlineData("-p:abd.dll", false)] + [InlineData("-p:abd.dll --", false)] + public void DllDetectionShouldWorkWhenNoSettings(string testArgs, bool expectedContainsBuiltTestSource) + { + var parseResult = Parser.Parse($"test {testArgs}"); + var args = parseResult.GetArguments(); + + (args, string[] settings) = TestCommand.SeparateSettingsFromArgs(args); + int settingsCount = TestCommand.GetSettingsCount(settings); + settingsCount.Should().Be(0); + TestCommand.ContainsBuiltTestSources(parseResult, settingsCount).Should().Be(expectedContainsBuiltTestSource); + } } }