Skip to content

Commit 50b4f3e

Browse files
Youssef1313Evangelink
Youssef1313
and
Evangelink
authored
Add validation for command-line arguments related to new dotnet test (#47616)
Co-authored-by: Amaury Levé <[email protected]>
1 parent 9f73261 commit 50b4f3e

20 files changed

+185
-77
lines changed

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -455,7 +455,10 @@ See https://aka.ms/dotnet-test/mtp for more information.</value>
455455
<value>The provided project file has an invalid extension: {0}.</value>
456456
</data>
457457
<data name="CmdMultipleBuildPathOptionsErrorDescription" xml:space="preserve">
458-
<value>Specify either the project, solution or directory option.</value>
458+
<value>Specify either the project, solution, directory, or test modules option.</value>
459+
</data>
460+
<data name="CmdOptionCannotBeUsedWithTestModulesDescription" xml:space="preserve">
461+
<value>The options architecture, configuration, framework, operating system, and runtime cannot be used with '--test-modules' option.</value>
459462
</data>
460463
<data name="CmdNoAnsiDescription" xml:space="preserve">
461464
<value>Disable ANSI output.</value>

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

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -12,45 +12,45 @@ namespace Microsoft.DotNet.Cli;
1212

1313
internal sealed class MSBuildHandler : IDisposable
1414
{
15-
private readonly List<string> _args;
15+
private readonly BuildOptions _buildOptions;
1616
private readonly TestApplicationActionQueue _actionQueue;
1717
private readonly TerminalTestReporter _output;
1818

1919
private readonly ConcurrentBag<TestApplication> _testApplications = new();
2020
private bool _areTestingPlatformApplications = true;
2121

22-
public MSBuildHandler(List<string> args, TestApplicationActionQueue actionQueue, TerminalTestReporter output)
22+
public MSBuildHandler(BuildOptions buildOptions, TestApplicationActionQueue actionQueue, TerminalTestReporter output)
2323
{
24-
_args = args;
24+
_buildOptions = buildOptions;
2525
_actionQueue = actionQueue;
2626
_output = output;
2727
}
2828

29-
public bool RunMSBuild(BuildOptions buildOptions)
29+
public bool RunMSBuild()
3030
{
31-
if (!ValidationUtility.ValidateBuildPathOptions(buildOptions, _output))
31+
if (!ValidationUtility.ValidateBuildPathOptions(_buildOptions, _output))
3232
{
3333
return false;
3434
}
3535

3636
int msBuildExitCode;
3737
string path;
38-
PathOptions pathOptions = buildOptions.PathOptions;
38+
PathOptions pathOptions = _buildOptions.PathOptions;
3939

4040
if (!string.IsNullOrEmpty(pathOptions.ProjectPath))
4141
{
4242
path = PathUtility.GetFullPath(pathOptions.ProjectPath);
43-
msBuildExitCode = RunBuild(path, isSolution: false, buildOptions);
43+
msBuildExitCode = RunBuild(path, isSolution: false);
4444
}
4545
else if (!string.IsNullOrEmpty(pathOptions.SolutionPath))
4646
{
4747
path = PathUtility.GetFullPath(pathOptions.SolutionPath);
48-
msBuildExitCode = RunBuild(path, isSolution: true, buildOptions);
48+
msBuildExitCode = RunBuild(path, isSolution: true);
4949
}
5050
else
5151
{
5252
path = PathUtility.GetFullPath(pathOptions.DirectoryPath ?? Directory.GetCurrentDirectory());
53-
msBuildExitCode = RunBuild(path, buildOptions);
53+
msBuildExitCode = RunBuild(path);
5454
}
5555

5656
if (msBuildExitCode != ExitCode.Success)
@@ -62,7 +62,7 @@ public bool RunMSBuild(BuildOptions buildOptions)
6262
return true;
6363
}
6464

65-
private int RunBuild(string directoryPath, BuildOptions buildOptions)
65+
private int RunBuild(string directoryPath)
6666
{
6767
(bool solutionOrProjectFileFound, string message) = SolutionAndProjectUtility.TryGetProjectOrSolutionFilePath(directoryPath, out string projectOrSolutionFilePath, out bool isSolution);
6868

@@ -72,16 +72,16 @@ private int RunBuild(string directoryPath, BuildOptions buildOptions)
7272
return ExitCode.GenericFailure;
7373
}
7474

75-
(IEnumerable<TestModule> projects, bool restored) = GetProjectsProperties(projectOrSolutionFilePath, isSolution, buildOptions);
75+
(IEnumerable<TestModule> projects, bool restored) = GetProjectsProperties(projectOrSolutionFilePath, isSolution);
7676

7777
InitializeTestApplications(projects);
7878

7979
return restored ? ExitCode.Success : ExitCode.GenericFailure;
8080
}
8181

82-
private int RunBuild(string filePath, bool isSolution, BuildOptions buildOptions)
82+
private int RunBuild(string filePath, bool isSolution)
8383
{
84-
(IEnumerable<TestModule> projects, bool restored) = GetProjectsProperties(filePath, isSolution, buildOptions);
84+
(IEnumerable<TestModule> projects, bool restored) = GetProjectsProperties(filePath, isSolution);
8585

8686
InitializeTestApplications(projects);
8787

@@ -114,7 +114,7 @@ private void InitializeTestApplications(IEnumerable<TestModule> modules)
114114
throw new UnreachableException($"This program location is thought to be unreachable. Class='{nameof(MSBuildHandler)}' Method='{nameof(InitializeTestApplications)}'");
115115
}
116116

117-
var testApp = new TestApplication(module, _args);
117+
var testApp = new TestApplication(module, _buildOptions);
118118
_testApplications.Add(testApp);
119119
}
120120
}
@@ -133,11 +133,11 @@ public bool EnqueueTestApplications()
133133
return true;
134134
}
135135

136-
private (IEnumerable<TestModule> Projects, bool Restored) GetProjectsProperties(string solutionOrProjectFilePath, bool isSolution, BuildOptions buildOptions)
136+
private (IEnumerable<TestModule> Projects, bool Restored) GetProjectsProperties(string solutionOrProjectFilePath, bool isSolution)
137137
{
138138
(IEnumerable<TestModule> projects, bool isBuiltOrRestored) = isSolution ?
139-
MSBuildUtility.GetProjectsFromSolution(solutionOrProjectFilePath, buildOptions) :
140-
MSBuildUtility.GetProjectsFromProject(solutionOrProjectFilePath, buildOptions);
139+
MSBuildUtility.GetProjectsFromSolution(solutionOrProjectFilePath, _buildOptions) :
140+
MSBuildUtility.GetProjectsFromProject(solutionOrProjectFilePath, _buildOptions);
141141

142142
LogProjectProperties(projects);
143143

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

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ namespace Microsoft.DotNet.Cli;
1212
internal sealed class TestApplication : IDisposable
1313
{
1414
private readonly TestModule _module;
15-
private readonly List<string> _args;
15+
private readonly BuildOptions _buildOptions;
1616

1717
private readonly List<string> _outputData = [];
1818
private readonly List<string> _errorData = [];
@@ -35,10 +35,10 @@ internal sealed class TestApplication : IDisposable
3535

3636
public TestModule Module => _module;
3737

38-
public TestApplication(TestModule module, List<string> args)
38+
public TestApplication(TestModule module, BuildOptions buildOptions)
3939
{
4040
_module = module;
41-
_args = args;
41+
_buildOptions = buildOptions;
4242
}
4343

4444
public void AddExecutionId(string executionId)
@@ -85,22 +85,17 @@ private ProcessStartInfo CreateProcessStartInfo(bool isDll, TestOptions testOpti
8585
}
8686

8787
private string GetFileName(TestOptions testOptions, bool isDll)
88-
{
89-
if (testOptions.HasFilterMode || !IsArchitectureSpecified(testOptions))
90-
{
91-
return isDll ? Environment.ProcessPath : _module.RunProperties.RunCommand;
92-
}
93-
94-
return Environment.ProcessPath;
95-
}
88+
=> isDll ? Environment.ProcessPath : _module.RunProperties.RunCommand;
9689

9790
private string GetArguments(TestOptions testOptions, bool isDll)
9891
{
99-
if (testOptions.HasFilterMode || !IsArchitectureSpecified(testOptions))
92+
if (testOptions.HasFilterMode || !isDll || !IsArchitectureSpecified(testOptions))
10093
{
10194
return BuildArgs(testOptions, isDll);
10295
}
10396

97+
// We fallback to dotnet run only when we have a dll and an architecture is specified.
98+
// TODO: Is this a valid case?
10499
return BuildArgsWithDotnetRun(testOptions);
105100
}
106101

@@ -312,6 +307,11 @@ private string BuildArgsWithDotnetRun(TestOptions testOptions)
312307
builder.Append($" {CommonOptions.NoRestoreOption.Name}");
313308
builder.Append($" {TestingPlatformOptions.NoBuildOption.Name}");
314309

310+
// TODO: Instead of passing Architecture and Configuration this way, pass _buildOptions.MSBuildArgs
311+
// _buildOptions.MSBuildArgs will include all needed global properties.
312+
// TODO: Care to be taken when dealing with -bl.
313+
// We will want to adjust the file name here.
314+
315315
if (!string.IsNullOrEmpty(testOptions.Architecture))
316316
{
317317
builder.Append($" {CommonOptions.ArchitectureOption.Name} {testOptions.Architecture}");
@@ -341,8 +341,9 @@ private void AppendCommonArgs(StringBuilder builder, TestOptions testOptions)
341341
builder.Append($" {TestingPlatformOptions.HelpOption.Name} ");
342342
}
343343

344-
builder.Append(_args.Count != 0
345-
? _args.Aggregate((a, b) => $"{a} {b}")
344+
var args = _buildOptions.UnmatchedTokens;
345+
builder.Append(args.Count != 0
346+
? args.Aggregate((a, b) => $"{a} {b}")
346347
: string.Empty);
347348

348349
builder.Append($" {CliConstants.ServerOptionKey} {CliConstants.ServerOptionValue} {CliConstants.DotNetTestPipeOptionKey} {_pipeNameDescription.Name} {_module.RunProperties.RunArguments}");

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

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,18 +11,16 @@ namespace Microsoft.DotNet.Cli;
1111

1212
internal sealed class TestModulesFilterHandler
1313
{
14-
private readonly List<string> _args;
1514
private readonly TestApplicationActionQueue _actionQueue;
1615
private readonly TerminalTestReporter _output;
1716

18-
public TestModulesFilterHandler(List<string> args, TestApplicationActionQueue actionQueue, TerminalTestReporter output)
17+
public TestModulesFilterHandler(TestApplicationActionQueue actionQueue, TerminalTestReporter output)
1918
{
20-
_args = args;
2119
_actionQueue = actionQueue;
2220
_output = output;
2321
}
2422

25-
public bool RunWithTestModulesFilter(ParseResult parseResult)
23+
public bool RunWithTestModulesFilter(ParseResult parseResult, BuildOptions buildOptions)
2624
{
2725
// If the module path pattern(s) was provided, we will use that to filter the test modules
2826
string testModules = parseResult.GetValue(TestingPlatformOptions.TestModulesFilterOption);
@@ -55,7 +53,7 @@ public bool RunWithTestModulesFilter(ParseResult parseResult)
5553

5654
foreach (string testModule in testModulePaths)
5755
{
58-
var testApp = new TestApplication(new TestModule(new RunProperties(testModule, null, null), null, null, null, true, true), _args);
56+
var testApp = new TestApplication(new TestModule(new RunProperties(testModule, null, null), null, null, null, true, true), buildOptions);
5957
// Write the test application to the channel
6058
_actionQueue.Enqueue(testApp);
6159
}

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

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,28 +30,30 @@ public int Run(ParseResult parseResult)
3030
bool hasFailed = false;
3131
try
3232
{
33+
ValidationUtility.ValidateMutuallyExclusiveOptions(parseResult);
34+
3335
PrepareEnvironment(parseResult, out TestOptions testOptions, out int degreeOfParallelism);
3436

3537
InitializeOutput(degreeOfParallelism, parseResult, testOptions.IsHelp);
3638

3739
InitializeActionQueue(degreeOfParallelism, testOptions, testOptions.IsHelp);
3840

3941
BuildOptions buildOptions = MSBuildUtility.GetBuildOptions(parseResult, degreeOfParallelism);
40-
_msBuildHandler = new(buildOptions.UnmatchedTokens, _actionQueue, _output);
41-
TestModulesFilterHandler testModulesFilterHandler = new(buildOptions.UnmatchedTokens, _actionQueue, _output);
42+
_msBuildHandler = new(buildOptions, _actionQueue, _output);
43+
TestModulesFilterHandler testModulesFilterHandler = new(_actionQueue, _output);
4244

4345
_eventHandlers = new TestApplicationsEventHandlers(_executions, _output);
4446

4547
if (testOptions.HasFilterMode)
4648
{
47-
if (!testModulesFilterHandler.RunWithTestModulesFilter(parseResult))
49+
if (!testModulesFilterHandler.RunWithTestModulesFilter(parseResult, buildOptions))
4850
{
4951
return ExitCode.GenericFailure;
5052
}
5153
}
5254
else
5355
{
54-
if (!_msBuildHandler.RunMSBuild(buildOptions))
56+
if (!_msBuildHandler.RunMSBuild())
5557
{
5658
return ExitCode.GenericFailure;
5759
}

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

Lines changed: 47 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,63 @@
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-
using Microsoft.DotNet.Tools.Test;
4+
using System.CommandLine;
5+
using Microsoft.DotNet.Cli.Extensions;
6+
using Microsoft.DotNet.Cli.Utils;
57
using Microsoft.Testing.Platform.OutputDevice.Terminal;
68

9+
using LocalizableStrings = Microsoft.DotNet.Tools.Test.LocalizableStrings;
10+
711
namespace Microsoft.DotNet.Cli;
812

913
internal static class ValidationUtility
1014
{
11-
public static bool ValidateBuildPathOptions(BuildOptions buildPathOptions, TerminalTestReporter output)
15+
public static void ValidateMutuallyExclusiveOptions(ParseResult parseResult)
1216
{
13-
PathOptions pathOptions = buildPathOptions.PathOptions;
14-
if ((!string.IsNullOrEmpty(pathOptions.ProjectPath) && !string.IsNullOrEmpty(pathOptions.SolutionPath)) ||
15-
(!string.IsNullOrEmpty(pathOptions.ProjectPath) && !string.IsNullOrEmpty(pathOptions.DirectoryPath)) ||
16-
(!string.IsNullOrEmpty(pathOptions.SolutionPath) && !string.IsNullOrEmpty(pathOptions.DirectoryPath)))
17+
ValidatePathOptions(parseResult);
18+
ValidateOptionsIrrelevantToModulesFilter(parseResult);
19+
20+
static void ValidatePathOptions(ParseResult parseResult)
1721
{
18-
output.WriteMessage(LocalizableStrings.CmdMultipleBuildPathOptionsErrorDescription);
19-
return false;
22+
var count = 0;
23+
if (parseResult.HasOption(TestingPlatformOptions.TestModulesFilterOption))
24+
count++;
25+
26+
if (parseResult.HasOption(TestingPlatformOptions.DirectoryOption))
27+
count++;
28+
29+
if (parseResult.HasOption(TestingPlatformOptions.SolutionOption))
30+
count++;
31+
32+
if (parseResult.HasOption(TestingPlatformOptions.ProjectOption))
33+
count++;
34+
35+
if (count > 1)
36+
throw new GracefulException(LocalizableStrings.CmdMultipleBuildPathOptionsErrorDescription);
2037
}
2138

39+
static void ValidateOptionsIrrelevantToModulesFilter(ParseResult parseResult)
40+
{
41+
if (!parseResult.HasOption(TestingPlatformOptions.TestModulesFilterOption))
42+
{
43+
return;
44+
}
45+
46+
if (parseResult.HasOption(CommonOptions.ArchitectureOption) ||
47+
parseResult.HasOption(TestingPlatformOptions.ConfigurationOption) ||
48+
parseResult.HasOption(TestingPlatformOptions.FrameworkOption) ||
49+
parseResult.HasOption(CommonOptions.OperatingSystemOption) ||
50+
parseResult.HasOption(CommonOptions.RuntimeOption)
51+
)
52+
{
53+
throw new GracefulException(LocalizableStrings.CmdOptionCannotBeUsedWithTestModulesDescription);
54+
}
55+
}
56+
}
57+
public static bool ValidateBuildPathOptions(BuildOptions buildPathOptions, TerminalTestReporter output)
58+
{
59+
PathOptions pathOptions = buildPathOptions.PathOptions;
60+
2261
if (!string.IsNullOrEmpty(pathOptions.ProjectPath))
2362
{
2463
return ValidateFilePath(pathOptions.ProjectPath, CliConstants.ProjectExtensions, LocalizableStrings.CmdInvalidProjectFileExtensionErrorDescription, output);

‎src/Cli/dotnet/commands/dotnet-test/xlf/LocalizableStrings.cs.xlf

Lines changed: 7 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎src/Cli/dotnet/commands/dotnet-test/xlf/LocalizableStrings.de.xlf

Lines changed: 7 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)