diff --git a/src/Cli/dotnet/Commands/Test/MTP/MSBuildUtility.cs b/src/Cli/dotnet/Commands/Test/MTP/MSBuildUtility.cs index fc1d2eaf2b35..e06c3c1259a6 100644 --- a/src/Cli/dotnet/Commands/Test/MTP/MSBuildUtility.cs +++ b/src/Cli/dotnet/Commands/Test/MTP/MSBuildUtility.cs @@ -101,6 +101,8 @@ public static BuildOptions GetBuildOptions(ParseResult parseResult) { LoggerUtility.SeparateBinLogArguments(parseResult.UnmatchedTokens, out var binLogArgs, out var otherArgs); + var (positionalProjectOrSolution, positionalTestModules) = GetPositionalArguments(otherArgs); + var msbuildArgs = parseResult.OptionValuesToBeForwarded(TestCommandParser.GetCommand()) .Concat(binLogArgs); @@ -122,9 +124,19 @@ public static BuildOptions GetBuildOptions(ParseResult parseResult) diagnosticOutputDirectory = Path.GetFullPath(diagnosticOutputDirectory); } + var projectOrSolutionOptionValue = parseResult.GetValue(MicrosoftTestingPlatformOptions.ProjectOrSolutionOption); + var testModulesFilterOptionValue = parseResult.GetValue(MicrosoftTestingPlatformOptions.TestModulesFilterOption); + + if ((projectOrSolutionOptionValue is not null && positionalProjectOrSolution is not null) || + testModulesFilterOptionValue is not null && positionalTestModules is not null) + { + throw new GracefulException(CliCommandStrings.CmdMultipleBuildPathOptionsErrorDescription); + } + PathOptions pathOptions = new( - parseResult.GetValue(MicrosoftTestingPlatformOptions.ProjectOrSolutionOption), + positionalProjectOrSolution ?? parseResult.GetValue(MicrosoftTestingPlatformOptions.ProjectOrSolutionOption), parseResult.GetValue(MicrosoftTestingPlatformOptions.SolutionOption), + positionalTestModules ?? parseResult.GetValue(MicrosoftTestingPlatformOptions.TestModulesFilterOption), resultsDirectory, configFile, diagnosticOutputDirectory); @@ -140,6 +152,83 @@ public static BuildOptions GetBuildOptions(ParseResult parseResult) msbuildArgs); } + private static (string? PositionalProjectOrSolution, string? PositionalTestModules) GetPositionalArguments(List otherArgs) + { + string? positionalProjectOrSolution = null; + string? positionalTestModules = null; + + // In case there is a valid case, users can opt-out. + // Note that the validation here is added to have a "better" error message for scenarios that will already fail. + // So, disabling validation is okay if the user scenario is valid. + bool throwOnUnexpectedFilePassedAsNonFirstPositionalArgument = Environment.GetEnvironmentVariable("DOTNET_TEST_DISABLE_SWITCH_VALIDATION") is not ("true" or "1"); + + for (int i = 0; i < otherArgs.Count; i++) + { + var token = otherArgs[i]; + if ((token.EndsWith(".sln", StringComparison.OrdinalIgnoreCase) || + token.EndsWith(".slnf", StringComparison.OrdinalIgnoreCase) || + token.EndsWith(".slnx", StringComparison.OrdinalIgnoreCase)) && File.Exists(token)) + { + if (i == 0) + { + positionalProjectOrSolution = token; + otherArgs.RemoveAt(0); + break; + } + else if (throwOnUnexpectedFilePassedAsNonFirstPositionalArgument) + { + throw new GracefulException(CliCommandStrings.TestCommandUseSolution); + } + } + else if ((token.EndsWith(".csproj", StringComparison.OrdinalIgnoreCase) || + token.EndsWith(".vbproj", StringComparison.OrdinalIgnoreCase) || + token.EndsWith(".fsproj", StringComparison.OrdinalIgnoreCase)) && File.Exists(token)) + { + if (i == 0) + { + positionalProjectOrSolution = token; + otherArgs.RemoveAt(0); + break; + } + else if (throwOnUnexpectedFilePassedAsNonFirstPositionalArgument) + { + throw new GracefulException(CliCommandStrings.TestCommandUseProject); + } + } + else if ((token.EndsWith(".dll", StringComparison.OrdinalIgnoreCase) || + token.EndsWith(".exe", StringComparison.OrdinalIgnoreCase)) && + File.Exists(token)) + { + if (i == 0) + { + positionalTestModules = token; + otherArgs.RemoveAt(0); + break; + } + else if (throwOnUnexpectedFilePassedAsNonFirstPositionalArgument) + { + throw new GracefulException(CliCommandStrings.TestCommandUseTestModules); + } + } + else if (Directory.Exists(token)) + { + if (i == 0) + { + positionalTestModules = token; + otherArgs.RemoveAt(0); + break; + } + else if (throwOnUnexpectedFilePassedAsNonFirstPositionalArgument) + { + throw new GracefulException(CliCommandStrings.TestCommandUseDirectoryWithSwitch); + } + } + } + + return (positionalProjectOrSolution, positionalTestModules); + } + + private static bool BuildOrRestoreProjectOrSolution(string filePath, BuildOptions buildOptions) { if (buildOptions.HasNoBuild) diff --git a/src/Cli/dotnet/Commands/Test/MTP/MicrosoftTestingPlatformTestCommand.cs b/src/Cli/dotnet/Commands/Test/MTP/MicrosoftTestingPlatformTestCommand.cs index d84264e6c49e..a17b3cabf6f1 100644 --- a/src/Cli/dotnet/Commands/Test/MTP/MicrosoftTestingPlatformTestCommand.cs +++ b/src/Cli/dotnet/Commands/Test/MTP/MicrosoftTestingPlatformTestCommand.cs @@ -37,8 +37,8 @@ public int Run(ParseResult parseResult, bool isHelp = false) private int RunInternal(ParseResult parseResult, bool isHelp) { - ValidationUtility.ValidateMutuallyExclusiveOptions(parseResult); - ValidationUtility.ValidateSolutionOrProjectOrDirectoryOrModulesArePassedCorrectly(parseResult); + BuildOptions buildOptions = MSBuildUtility.GetBuildOptions(parseResult); + ValidationUtility.ValidateMutuallyExclusiveOptions(parseResult, buildOptions.PathOptions); int degreeOfParallelism = GetDegreeOfParallelism(parseResult); var testOptions = new TestOptions( @@ -46,17 +46,14 @@ private int RunInternal(ParseResult parseResult, bool isHelp) IsDiscovery: parseResult.HasOption(MicrosoftTestingPlatformOptions.ListTestsOption), EnvironmentVariables: parseResult.GetValue(CommonOptions.EnvOption) ?? ImmutableDictionary.Empty); - BuildOptions buildOptions = MSBuildUtility.GetBuildOptions(parseResult); - - bool filterModeEnabled = parseResult.HasOption(MicrosoftTestingPlatformOptions.TestModulesFilterOption); TestApplicationActionQueue actionQueue; - if (filterModeEnabled) + if (buildOptions.PathOptions.TestModules is not null) { InitializeOutput(degreeOfParallelism, parseResult, testOptions); actionQueue = new TestApplicationActionQueue(degreeOfParallelism, buildOptions, testOptions, _output, OnHelpRequested); var testModulesFilterHandler = new TestModulesFilterHandler(actionQueue, _output); - if (!testModulesFilterHandler.RunWithTestModulesFilter(parseResult)) + if (!testModulesFilterHandler.RunWithTestModulesFilter(parseResult, buildOptions.PathOptions.TestModules)) { return ExitCode.GenericFailure; } diff --git a/src/Cli/dotnet/Commands/Test/MTP/Options.cs b/src/Cli/dotnet/Commands/Test/MTP/Options.cs index a5ead893d935..d311b62d9721 100644 --- a/src/Cli/dotnet/Commands/Test/MTP/Options.cs +++ b/src/Cli/dotnet/Commands/Test/MTP/Options.cs @@ -5,7 +5,7 @@ namespace Microsoft.DotNet.Cli.Commands.Test; internal record TestOptions(bool IsHelp, bool IsDiscovery, IReadOnlyDictionary EnvironmentVariables); -internal record PathOptions(string? ProjectOrSolutionPath, string? SolutionPath, string? ResultsDirectoryPath, string? ConfigFilePath, string? DiagnosticOutputDirectoryPath); +internal record PathOptions(string? ProjectOrSolutionPath, string? SolutionPath, string? TestModules, string? ResultsDirectoryPath, string? ConfigFilePath, string? DiagnosticOutputDirectoryPath); internal record BuildOptions( PathOptions PathOptions, @@ -14,5 +14,5 @@ internal record BuildOptions( Utils.VerbosityOptions? Verbosity, bool NoLaunchProfile, bool NoLaunchProfileArguments, - List UnmatchedTokens, + List TestApplicationArguments, IEnumerable MSBuildArgs); diff --git a/src/Cli/dotnet/Commands/Test/MTP/TestApplication.cs b/src/Cli/dotnet/Commands/Test/MTP/TestApplication.cs index be7a7371c707..4ad8ac073233 100644 --- a/src/Cli/dotnet/Commands/Test/MTP/TestApplication.cs +++ b/src/Cli/dotnet/Commands/Test/MTP/TestApplication.cs @@ -168,7 +168,7 @@ private string GetArguments() builder.Append($" {MicrosoftTestingPlatformOptions.DiagnosticOutputDirectoryOption.Name} {ArgumentEscaper.EscapeSingleArg(diagnosticOutputDirectoryPath)}"); } - foreach (var arg in _buildOptions.UnmatchedTokens) + foreach (var arg in _buildOptions.TestApplicationArguments) { builder.Append($" {ArgumentEscaper.EscapeSingleArg(arg)}"); } diff --git a/src/Cli/dotnet/Commands/Test/MTP/TestModulesFilterHandler.cs b/src/Cli/dotnet/Commands/Test/MTP/TestModulesFilterHandler.cs index 37f46fc0984d..635e311e9908 100644 --- a/src/Cli/dotnet/Commands/Test/MTP/TestModulesFilterHandler.cs +++ b/src/Cli/dotnet/Commands/Test/MTP/TestModulesFilterHandler.cs @@ -16,11 +16,9 @@ internal sealed class TestModulesFilterHandler(TestApplicationActionQueue action private readonly TestApplicationActionQueue _actionQueue = actionQueue; private readonly TerminalTestReporter _output = output; - public bool RunWithTestModulesFilter(ParseResult parseResult) + public bool RunWithTestModulesFilter(ParseResult parseResult, string testModules) { // If the module path pattern(s) was provided, we will use that to filter the test modules - string testModules = parseResult.GetValue(MicrosoftTestingPlatformOptions.TestModulesFilterOption)!; - // If the root directory was provided, we will use that to search for the test modules // Otherwise, we will use the current directory string? rootDirectory = Directory.GetCurrentDirectory(); diff --git a/src/Cli/dotnet/Commands/Test/MTP/ValidationUtility.cs b/src/Cli/dotnet/Commands/Test/MTP/ValidationUtility.cs index 1e16d5cea36a..f8b7c1320242 100644 --- a/src/Cli/dotnet/Commands/Test/MTP/ValidationUtility.cs +++ b/src/Cli/dotnet/Commands/Test/MTP/ValidationUtility.cs @@ -11,30 +11,31 @@ namespace Microsoft.DotNet.Cli.Commands.Test; internal static class ValidationUtility { - public static void ValidateMutuallyExclusiveOptions(ParseResult parseResult) + public static void ValidateMutuallyExclusiveOptions(ParseResult parseResult, PathOptions pathOptions) { - ValidatePathOptions(parseResult); - ValidateOptionsIrrelevantToModulesFilter(parseResult); + ValidatePathOptions(parseResult, pathOptions); - static void ValidatePathOptions(ParseResult parseResult) + ValidateOptionsIrrelevantToModulesFilter(parseResult, pathOptions.TestModules); + + static void ValidatePathOptions(ParseResult parseResult, PathOptions pathOptions) { var count = 0; - if (parseResult.HasOption(MicrosoftTestingPlatformOptions.TestModulesFilterOption)) + if (pathOptions.TestModules is not null) count++; - if (parseResult.HasOption(MicrosoftTestingPlatformOptions.SolutionOption)) + if (pathOptions.SolutionPath is not null) count++; - if (parseResult.HasOption(MicrosoftTestingPlatformOptions.ProjectOrSolutionOption)) + if (pathOptions.ProjectOrSolutionPath is not null) count++; if (count > 1) throw new GracefulException(CliCommandStrings.CmdMultipleBuildPathOptionsErrorDescription); } - static void ValidateOptionsIrrelevantToModulesFilter(ParseResult parseResult) + static void ValidateOptionsIrrelevantToModulesFilter(ParseResult parseResult, string? testModules) { - if (!parseResult.HasOption(MicrosoftTestingPlatformOptions.TestModulesFilterOption)) + if (testModules is null) { return; } @@ -95,47 +96,6 @@ private static bool TryGetProjectOrSolutionFromDirectory( return true; } - /// - /// Validates that arguments requiring specific command-line switches are used correctly for Microsoft.Testing.Platform. - /// Provides helpful error messages when users provide file/directory arguments without proper switches. - /// - public static void ValidateSolutionOrProjectOrDirectoryOrModulesArePassedCorrectly(ParseResult parseResult) - { - if (Environment.GetEnvironmentVariable("DOTNET_TEST_DISABLE_SWITCH_VALIDATION") is "true" or "1") - { - // In case there is a valid case, users can opt-out. - // Note that the validation here is added to have a "better" error message for scenarios that will already fail. - // So, disabling validation is okay if the user scenario is valid. - return; - } - - foreach (string token in parseResult.UnmatchedTokens) - { - // Check for .sln files - if ((token.EndsWith(".sln", StringComparison.OrdinalIgnoreCase) || - token.EndsWith(".slnx", StringComparison.OrdinalIgnoreCase)) && File.Exists(token)) - { - throw new GracefulException(CliCommandStrings.TestCommandUseSolution); - } - else if ((token.EndsWith(".csproj", StringComparison.OrdinalIgnoreCase) || - token.EndsWith(".vbproj", StringComparison.OrdinalIgnoreCase) || - token.EndsWith(".fsproj", StringComparison.OrdinalIgnoreCase)) && File.Exists(token)) - { - throw new GracefulException(CliCommandStrings.TestCommandUseProject); - } - else if ((token.EndsWith(".dll", StringComparison.OrdinalIgnoreCase) || - token.EndsWith(".exe", StringComparison.OrdinalIgnoreCase)) && - File.Exists(token)) - { - throw new GracefulException(CliCommandStrings.TestCommandUseTestModules); - } - else if (Directory.Exists(token)) - { - throw new GracefulException(CliCommandStrings.TestCommandUseDirectoryWithSwitch); - } - } - } - private static bool ValidateSolutionPath(string solutionFileOrDirectory, [NotNullWhen(true)] out string? solutionFile) { // If it's a directory, just check if it exists diff --git a/test/dotnet.Tests/CommandTests/Test/GivenDotnetTestBuildsAndRunsTestsWithDifferentOptions.cs b/test/dotnet.Tests/CommandTests/Test/GivenDotnetTestBuildsAndRunsTestsWithDifferentOptions.cs index 7883e1f6e647..2f662513d4a2 100644 --- a/test/dotnet.Tests/CommandTests/Test/GivenDotnetTestBuildsAndRunsTestsWithDifferentOptions.cs +++ b/test/dotnet.Tests/CommandTests/Test/GivenDotnetTestBuildsAndRunsTestsWithDifferentOptions.cs @@ -589,6 +589,25 @@ public void RunWithSolutionFilterContainingSharedProject_ShouldSkipSharedProject result.ExitCode.Should().Be(ExitCodes.AtLeastOneTestFailed); } + [InlineData(TestingConstants.Debug)] + [InlineData(TestingConstants.Release)] + [Theory] + public void RunWithSolutionFilterAsFirstUnmatchedToken_ShouldWork(string configuration) + { + TestAsset testInstance = _testAssetsManager.CopyTestAsset("MultiTestProjectSolutionWithSharedProject", Guid.NewGuid().ToString()).WithSource(); + + string testSolutionFilterPath = "TestProjectsWithShared.slnf"; + + CommandResult result = new DotnetTestCommand(Log, disableNewOutput: false) + .WithWorkingDirectory(testInstance.Path) + .Execute(testSolutionFilterPath, + TestCommandDefinition.ConfigurationOption.Name, configuration); + + result.StdOut.Should().Contain("TestProject.dll"); + result.StdOut.Should().NotContain("OtherTestProject.dll"); + result.ExitCode.Should().Be(ExitCodes.AtLeastOneTestFailed); + } + [InlineData(TestingConstants.Debug)] [InlineData(TestingConstants.Release)] [Theory] diff --git a/test/dotnet.Tests/CommandTests/Test/TestCommandValidationTests.cs b/test/dotnet.Tests/CommandTests/Test/TestCommandValidationTests.cs index 4c8a746eebc4..d5746358baca 100644 --- a/test/dotnet.Tests/CommandTests/Test/TestCommandValidationTests.cs +++ b/test/dotnet.Tests/CommandTests/Test/TestCommandValidationTests.cs @@ -32,7 +32,7 @@ public void TestCommandShouldValidateFileArgumentsAndProvideHelpfulMessages(stri var result = new DotnetTestCommand(Log, disableNewOutput: false) .WithWorkingDirectory(testDir.Path) - .Execute(filename); + .Execute("--my-arg", filename); result.ExitCode.Should().NotBe(0); if (!TestContext.IsLocalized()) @@ -58,7 +58,7 @@ public void TestCommandShouldValidateDirectoryArgumentAndProvideHelpfulMessage() var result = new DotnetTestCommand(Log, disableNewOutput: false) .WithWorkingDirectory(testDir.Path) - .Execute("test_directory"); + .Execute("--myarg", "test_directory"); result.ExitCode.Should().NotBe(0); if (!TestContext.IsLocalized()) @@ -86,7 +86,7 @@ public void TestCommandShouldValidateDllArgumentAndProvideHelpfulMessage() var result = new DotnetTestCommand(Log, disableNewOutput: false) .WithWorkingDirectory(testDir.Path) - .Execute("test.dll"); + .Execute("--my-arg", "test.dll"); result.ExitCode.Should().NotBe(0); if (!TestContext.IsLocalized())