Add validation for command-line arguments related to new dotnet test

src/Cli/dotnet/commands/dotnet-test/LocalizableStrings.resx

@@ -455,7 +455,10 @@ See https://aka.ms/dotnet-test/mtp for more information.</value>
     <value>The provided project file has an invalid extension: {0}.</value>
   </data>
   <data name="CmdMultipleBuildPathOptionsErrorDescription" xml:space="preserve">
-    <value>Specify either the project, solution or directory option.</value>
+    <value>Specify either the project, solution, directory, or test modules option.</value>
+  </data>
+  <data name="CmdOptionCannotBeUsedWithTestModulesDescription" xml:space="preserve">
+    <value>The options architecture, configuration, framework, operating system, and runtime cannot be used with '--test-modules' option.</value>
   </data>
   <data name="CmdNoAnsiDescription" xml:space="preserve">
     <value>Disable ANSI output.</value>

src/Cli/dotnet/commands/dotnet-test/MSBuildHandler.cs

@@ -12,45 +12,45 @@ namespace Microsoft.DotNet.Cli;
 
 internal sealed class MSBuildHandler : IDisposable
 {
-    private readonly List<string> _args;
+    private readonly BuildOptions _buildOptions;
     private readonly TestApplicationActionQueue _actionQueue;
     private readonly TerminalTestReporter _output;
 
     private readonly ConcurrentBag<TestApplication> _testApplications = new();
     private bool _areTestingPlatformApplications = true;
 
-    public MSBuildHandler(List<string> args, TestApplicationActionQueue actionQueue, TerminalTestReporter output)
+    public MSBuildHandler(BuildOptions buildOptions, TestApplicationActionQueue actionQueue, TerminalTestReporter output)
     {
-        _args = args;
+        _buildOptions = buildOptions;
         _actionQueue = actionQueue;
         _output = output;
     }
 
-    public bool RunMSBuild(BuildOptions buildOptions)
+    public bool RunMSBuild()
     {
-        if (!ValidationUtility.ValidateBuildPathOptions(buildOptions, _output))
+        if (!ValidationUtility.ValidateBuildPathOptions(_buildOptions, _output))
         {
             return false;
         }
 
         int msBuildExitCode;
         string path;
-        PathOptions pathOptions = buildOptions.PathOptions;
+        PathOptions pathOptions = _buildOptions.PathOptions;
 
         if (!string.IsNullOrEmpty(pathOptions.ProjectPath))
         {
             path = PathUtility.GetFullPath(pathOptions.ProjectPath);
-            msBuildExitCode = RunBuild(path, isSolution: false, buildOptions);
+            msBuildExitCode = RunBuild(path, isSolution: false);
         }
         else if (!string.IsNullOrEmpty(pathOptions.SolutionPath))
         {
             path = PathUtility.GetFullPath(pathOptions.SolutionPath);
-            msBuildExitCode = RunBuild(path, isSolution: true, buildOptions);
+            msBuildExitCode = RunBuild(path, isSolution: true);
         }
         else
         {
             path = PathUtility.GetFullPath(pathOptions.DirectoryPath ?? Directory.GetCurrentDirectory());
-            msBuildExitCode = RunBuild(path, buildOptions);
+            msBuildExitCode = RunBuild(path);
         }
 
         if (msBuildExitCode != ExitCode.Success)
@@ -62,7 +62,7 @@ public bool RunMSBuild(BuildOptions buildOptions)
         return true;
     }
 
-    private int RunBuild(string directoryPath, BuildOptions buildOptions)
+    private int RunBuild(string directoryPath)
     {
         (bool solutionOrProjectFileFound, string message) = SolutionAndProjectUtility.TryGetProjectOrSolutionFilePath(directoryPath, out string projectOrSolutionFilePath, out bool isSolution);
 
@@ -72,16 +72,16 @@ private int RunBuild(string directoryPath, BuildOptions buildOptions)
             return ExitCode.GenericFailure;
         }
 
-        (IEnumerable<TestModule> projects, bool restored) = GetProjectsProperties(projectOrSolutionFilePath, isSolution, buildOptions);
+        (IEnumerable<TestModule> projects, bool restored) = GetProjectsProperties(projectOrSolutionFilePath, isSolution);
 
         InitializeTestApplications(projects);
 
         return restored ? ExitCode.Success : ExitCode.GenericFailure;
     }
 
-    private int RunBuild(string filePath, bool isSolution, BuildOptions buildOptions)
+    private int RunBuild(string filePath, bool isSolution)
     {
-        (IEnumerable<TestModule> projects, bool restored) = GetProjectsProperties(filePath, isSolution, buildOptions);
+        (IEnumerable<TestModule> projects, bool restored) = GetProjectsProperties(filePath, isSolution);
 
         InitializeTestApplications(projects);
 
@@ -114,7 +114,7 @@ private void InitializeTestApplications(IEnumerable<TestModule> modules)
                 throw new UnreachableException($"This program location is thought to be unreachable. Class='{nameof(MSBuildHandler)}' Method='{nameof(InitializeTestApplications)}'");
             }
 
-            var testApp = new TestApplication(module, _args);
+            var testApp = new TestApplication(module, _buildOptions);
             _testApplications.Add(testApp);
         }
     }
@@ -133,11 +133,11 @@ public bool EnqueueTestApplications()
         return true;
     }
 
-    private (IEnumerable<TestModule> Projects, bool Restored) GetProjectsProperties(string solutionOrProjectFilePath, bool isSolution, BuildOptions buildOptions)
+    private (IEnumerable<TestModule> Projects, bool Restored) GetProjectsProperties(string solutionOrProjectFilePath, bool isSolution)
     {
         (IEnumerable<TestModule> projects, bool isBuiltOrRestored) = isSolution ?
-            MSBuildUtility.GetProjectsFromSolution(solutionOrProjectFilePath, buildOptions) :
-            MSBuildUtility.GetProjectsFromProject(solutionOrProjectFilePath, buildOptions);
+            MSBuildUtility.GetProjectsFromSolution(solutionOrProjectFilePath, _buildOptions) :
+            MSBuildUtility.GetProjectsFromProject(solutionOrProjectFilePath, _buildOptions);
 
         LogProjectProperties(projects);
 

src/Cli/dotnet/commands/dotnet-test/TestApplication.cs

@@ -12,7 +12,7 @@ namespace Microsoft.DotNet.Cli;
 internal sealed class TestApplication : IDisposable
 {
     private readonly TestModule _module;
-    private readonly List<string> _args;
+    private readonly BuildOptions _buildOptions;
 
     private readonly List<string> _outputData = [];
     private readonly List<string> _errorData = [];
@@ -35,10 +35,10 @@ internal sealed class TestApplication : IDisposable
 
     public TestModule Module => _module;
 
-    public TestApplication(TestModule module, List<string> args)
+    public TestApplication(TestModule module, BuildOptions buildOptions)
     {
         _module = module;
-        _args = args;
+        _buildOptions = buildOptions;
     }
 
     public void AddExecutionId(string executionId)
@@ -85,22 +85,17 @@ private ProcessStartInfo CreateProcessStartInfo(bool isDll, TestOptions testOpti
     }
 
     private string GetFileName(TestOptions testOptions, bool isDll)
-    {
-        if (testOptions.HasFilterMode || !IsArchitectureSpecified(testOptions))
-        {
-            return isDll ? Environment.ProcessPath : _module.RunProperties.RunCommand;
-        }
-
-        return Environment.ProcessPath;
-    }
+        => isDll ? Environment.ProcessPath : _module.RunProperties.RunCommand;
 
     private string GetArguments(TestOptions testOptions, bool isDll)
     {
-        if (testOptions.HasFilterMode || !IsArchitectureSpecified(testOptions))
+        if (testOptions.HasFilterMode || !isDll || !IsArchitectureSpecified(testOptions))
         {
             return BuildArgs(testOptions, isDll);
         }
 
+        // We fallback to dotnet run only when we have a dll and an architecture is specified.
+        // TODO: Is this a valid case?
         return BuildArgsWithDotnetRun(testOptions);
     }
 
@@ -312,6 +307,11 @@ private string BuildArgsWithDotnetRun(TestOptions testOptions)
         builder.Append($" {CommonOptions.NoRestoreOption.Name}");
         builder.Append($" {TestingPlatformOptions.NoBuildOption.Name}");
 
+        // TODO: Instead of passing Architecture and Configuration this way, pass _buildOptions.MSBuildArgs
+        // _buildOptions.MSBuildArgs will include all needed global properties.
+        // TODO: Care to be taken when dealing with -bl.
+        // We will want to adjust the file name here.
+
         if (!string.IsNullOrEmpty(testOptions.Architecture))
         {
             builder.Append($" {CommonOptions.ArchitectureOption.Name} {testOptions.Architecture}");
@@ -341,8 +341,9 @@ private void AppendCommonArgs(StringBuilder builder, TestOptions testOptions)
             builder.Append($" {TestingPlatformOptions.HelpOption.Name} ");
         }
 
-        builder.Append(_args.Count != 0
-            ? _args.Aggregate((a, b) => $"{a} {b}")
+        var args = _buildOptions.UnmatchedTokens;
+        builder.Append(args.Count != 0
+            ? args.Aggregate((a, b) => $"{a} {b}")
             : string.Empty);
 
         builder.Append($" {CliConstants.ServerOptionKey} {CliConstants.ServerOptionValue} {CliConstants.DotNetTestPipeOptionKey} {_pipeNameDescription.Name} {_module.RunProperties.RunArguments}");

src/Cli/dotnet/commands/dotnet-test/TestModulesFilterHandler.cs

@@ -11,18 +11,16 @@ namespace Microsoft.DotNet.Cli;
 
 internal sealed class TestModulesFilterHandler
 {
-    private readonly List<string> _args;
     private readonly TestApplicationActionQueue _actionQueue;
     private readonly TerminalTestReporter _output;
 
-    public TestModulesFilterHandler(List<string> args, TestApplicationActionQueue actionQueue, TerminalTestReporter output)
+    public TestModulesFilterHandler(TestApplicationActionQueue actionQueue, TerminalTestReporter output)
     {
-        _args = args;
         _actionQueue = actionQueue;
         _output = output;
     }
 
-    public bool RunWithTestModulesFilter(ParseResult parseResult)
+    public bool RunWithTestModulesFilter(ParseResult parseResult, BuildOptions buildOptions)
     {
         // If the module path pattern(s) was provided, we will use that to filter the test modules
         string testModules = parseResult.GetValue(TestingPlatformOptions.TestModulesFilterOption);
@@ -55,7 +53,7 @@ public bool RunWithTestModulesFilter(ParseResult parseResult)
 
         foreach (string testModule in testModulePaths)
         {
-            var testApp = new TestApplication(new TestModule(new RunProperties(testModule, null, null), null, null, null, true, true), _args);
+            var testApp = new TestApplication(new TestModule(new RunProperties(testModule, null, null), null, null, null, true, true), buildOptions);
             // Write the test application to the channel
             _actionQueue.Enqueue(testApp);
         }

src/Cli/dotnet/commands/dotnet-test/TestingPlatformCommand.cs

@@ -30,28 +30,30 @@ public int Run(ParseResult parseResult)
         bool hasFailed = false;
         try
         {
+            ValidationUtility.ValidateMutuallyExclusiveOptions(parseResult);
+
             PrepareEnvironment(parseResult, out TestOptions testOptions, out int degreeOfParallelism);
 
             InitializeOutput(degreeOfParallelism, parseResult, testOptions.IsHelp);
 
             InitializeActionQueue(degreeOfParallelism, testOptions, testOptions.IsHelp);
 
             BuildOptions buildOptions = MSBuildUtility.GetBuildOptions(parseResult, degreeOfParallelism);
-            _msBuildHandler = new(buildOptions.UnmatchedTokens, _actionQueue, _output);
-            TestModulesFilterHandler testModulesFilterHandler = new(buildOptions.UnmatchedTokens, _actionQueue, _output);
+            _msBuildHandler = new(buildOptions, _actionQueue, _output);
+            TestModulesFilterHandler testModulesFilterHandler = new(_actionQueue, _output);
 
             _eventHandlers = new TestApplicationsEventHandlers(_executions, _output);
 
             if (testOptions.HasFilterMode)
             {
-                if (!testModulesFilterHandler.RunWithTestModulesFilter(parseResult))
+                if (!testModulesFilterHandler.RunWithTestModulesFilter(parseResult, buildOptions))
                 {
                     return ExitCode.GenericFailure;
                 }
             }
             else
             {
-                if (!_msBuildHandler.RunMSBuild(buildOptions))
+                if (!_msBuildHandler.RunMSBuild())
                 {
                     return ExitCode.GenericFailure;
                 }

src/Cli/dotnet/commands/dotnet-test/ValidationUtility.cs

@@ -1,24 +1,63 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
-using Microsoft.DotNet.Tools.Test;
+using System.CommandLine;
+using Microsoft.DotNet.Cli.Extensions;
+using Microsoft.DotNet.Cli.Utils;
 using Microsoft.Testing.Platform.OutputDevice.Terminal;
 
+using LocalizableStrings = Microsoft.DotNet.Tools.Test.LocalizableStrings;
+
 namespace Microsoft.DotNet.Cli;
 
 internal static class ValidationUtility
 {
-    public static bool ValidateBuildPathOptions(BuildOptions buildPathOptions, TerminalTestReporter output)
+    public static void ValidateMutuallyExclusiveOptions(ParseResult parseResult)
     {
-        PathOptions pathOptions = buildPathOptions.PathOptions;
-        if ((!string.IsNullOrEmpty(pathOptions.ProjectPath) && !string.IsNullOrEmpty(pathOptions.SolutionPath)) ||
-            (!string.IsNullOrEmpty(pathOptions.ProjectPath) && !string.IsNullOrEmpty(pathOptions.DirectoryPath)) ||
-            (!string.IsNullOrEmpty(pathOptions.SolutionPath) && !string.IsNullOrEmpty(pathOptions.DirectoryPath)))
+        ValidatePathOptions(parseResult);
+        ValidateOptionsIrrelevantToModulesFilter(parseResult);
+
+        static void ValidatePathOptions(ParseResult parseResult)
         {
-            output.WriteMessage(LocalizableStrings.CmdMultipleBuildPathOptionsErrorDescription);
-            return false;
+            var count = 0;
+            if (parseResult.HasOption(TestingPlatformOptions.TestModulesFilterOption))
+                count++;
+
+            if (parseResult.HasOption(TestingPlatformOptions.DirectoryOption))
+                count++;
+
+            if (parseResult.HasOption(TestingPlatformOptions.SolutionOption))
+                count++;
+
+            if (parseResult.HasOption(TestingPlatformOptions.ProjectOption))
+                count++;
+
+            if (count > 1)
+                throw new GracefulException(LocalizableStrings.CmdMultipleBuildPathOptionsErrorDescription);
         }
 
+        static void ValidateOptionsIrrelevantToModulesFilter(ParseResult parseResult)
+        {
+            if (!parseResult.HasOption(TestingPlatformOptions.TestModulesFilterOption))
+            {
+                return;
+            }
+
+            if (parseResult.HasOption(CommonOptions.ArchitectureOption) ||
+                parseResult.HasOption(TestingPlatformOptions.ConfigurationOption) ||
+                parseResult.HasOption(TestingPlatformOptions.FrameworkOption) ||
+                parseResult.HasOption(CommonOptions.OperatingSystemOption) ||
+                parseResult.HasOption(CommonOptions.RuntimeOption)
+                )
+            {
+                throw new GracefulException(LocalizableStrings.CmdOptionCannotBeUsedWithTestModulesDescription);
+            }
+        }
+    }
+    public static bool ValidateBuildPathOptions(BuildOptions buildPathOptions, TerminalTestReporter output)
+    {
+        PathOptions pathOptions = buildPathOptions.PathOptions;
+
         if (!string.IsNullOrEmpty(pathOptions.ProjectPath))
         {
             return ValidateFilePath(pathOptions.ProjectPath, CliConstants.ProjectExtensions, LocalizableStrings.CmdInvalidProjectFileExtensionErrorDescription, output);