diff --git a/eng/Versions.props b/eng/Versions.props index d60cd6039953..a08826dc55f4 100644 --- a/eng/Versions.props +++ b/eng/Versions.props @@ -38,14 +38,14 @@ 36 20 - 19 - 8 + $([MSBuild]::Add($(VersionFeature), 22)) + $([MSBuild]::Add($(VersionFeature), 11)) <_NET70ILLinkPackVersion>7.0.100-1.23211.1 - $([MSBuild]::Add($(VersionFeature80), 1)) - $([MSBuild]::Add($(VersionFeature90), 1)) + $(VersionFeature80) + $(VersionFeature90) diff --git a/src/Cli/dotnet/Commands/Test/VSTest/TestCommand.cs b/src/Cli/dotnet/Commands/Test/VSTest/TestCommand.cs index a17cc0031e13..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(args)) + // 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,18 +316,14 @@ internal static int RunArtifactPostProcessingIfNeeded(string testSessionCorrelat } } - private static bool ContainsBuiltTestSources(string[] args) + internal /*internal for testing*/ static bool ContainsBuiltTestSources(ParseResult parseResult, int settingsLength) { - for (int i = 0; i < args.Length; i++) + for (int i = 0; i < parseResult.UnmatchedTokens.Count - settingsLength; i++) { - string arg = args[i]; - if (arg.EndsWith(".dll", StringComparison.OrdinalIgnoreCase) || arg.EndsWith(".exe", StringComparison.OrdinalIgnoreCase)) + string arg = parseResult.UnmatchedTokens[i]; + if (!arg.StartsWith("-") && + (arg.EndsWith(".dll", StringComparison.OrdinalIgnoreCase) || arg.EndsWith(".exe", StringComparison.OrdinalIgnoreCase))) { - var previousArg = i > 0 ? args[i - 1] : null; - if (previousArg != null && CommonOptions.PropertiesOption.Aliases.Contains(previousArg)) - { - return false; - } return true; } } diff --git a/test/dotnet.Tests/CommandTests/Test/GivenDotnetTestBuildsAndRunsTestfromCsproj.cs b/test/dotnet.Tests/CommandTests/Test/GivenDotnetTestBuildsAndRunsTestfromCsproj.cs index 96b0e9edb73b..6c054f61ceed 100644 --- a/test/dotnet.Tests/CommandTests/Test/GivenDotnetTestBuildsAndRunsTestfromCsproj.cs +++ b/test/dotnet.Tests/CommandTests/Test/GivenDotnetTestBuildsAndRunsTestfromCsproj.cs @@ -836,6 +836,28 @@ public void PropertiesEndingWithDotDllShouldNotFail(string property) result.ExitCode.Should().Be(1); } + [Fact] + public void DistributedLoggerEndingWithDotDllShouldBePassedToMSBuild() + { + var testProjectDirectory = CopyAndRestoreVSTestDotNetCoreTestApp([]); + + CommandResult result = new DotnetTestCommand(Log, disableNewOutput: true) + .WithWorkingDirectory(testProjectDirectory) + .Execute(ConsoleLoggerOutputNormal.Concat(["-dl:my.dll"])); + + if (!TestContext.IsLocalized()) + { + // This ensures that this was passed to MSBuild and not vstest.console. + result.StdOut.Should().Contain("error MSB1021: Cannot create an instance of the logger my.dll."); + } + else + { + result.StdOut.Should().Contain("MSB1021"); + } + + result.ExitCode.Should().Be(1); + } + private string CopyAndRestoreVSTestDotNetCoreTestApp(object[] parameters, [CallerMemberName] string callingMethod = "") { // Copy VSTestCore project in output directory of project dotnet-vstest.Tests 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); + } } }