Skip to content

Commit 308f095

Browse files
Youssef1313mariam-abdulla
Youssef1313
and
mariam-abdulla
authored
Rely on RunCommand for the new dotnet test experience for MTP (#47493)
Co-authored-by: mariam-abdulla <[email protected]>
1 parent 5f50397 commit 308f095

File tree

15 files changed

+197
-139
lines changed

15 files changed

+197
-139
lines changed

‎src/Cli/dotnet/commands/CommonRunHelpers.cs

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
using System.Diagnostics;
5+
using Microsoft.Build.Evaluation;
6+
using Microsoft.Build.Execution;
7+
using Microsoft.Build.Framework;
8+
using Microsoft.DotNet.Cli.Utils;
9+
10+
namespace Microsoft.DotNet.Cli;
11+
12+
internal static class CommonRunHelpers
13+
{
14+
/// <param name="globalProperties">
15+
/// Should have <see cref="StringComparer.OrdinalIgnoreCase"/>.
16+
/// </param>
17+
public static void AddUserPassedProperties(Dictionary<string, string> globalProperties, string[]? args)
18+
{
19+
Debug.Assert(globalProperties.Comparer == StringComparer.OrdinalIgnoreCase);
20+
21+
var fakeCommand = new System.CommandLine.CliCommand("dotnet") { CommonOptions.PropertiesOption };
22+
var propertyParsingConfiguration = new System.CommandLine.CliConfiguration(fakeCommand);
23+
var propertyParseResult = propertyParsingConfiguration.Parse(args);
24+
var propertyValues = propertyParseResult.GetValue(CommonOptions.PropertiesOption);
25+
26+
if (propertyValues is null)
27+
{
28+
return;
29+
}
30+
31+
foreach (var property in propertyValues)
32+
{
33+
foreach (var (key, value) in MSBuildPropertyParser.ParseProperties(property))
34+
{
35+
globalProperties[key] = value;
36+
}
37+
}
38+
}
39+
40+
public static Dictionary<string, string> GetGlobalPropertiesFromArgs(string[]? args)
41+
{
42+
var globalProperties = new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase)
43+
{
44+
// This property disables default item globbing to improve performance
45+
// This should be safe because we are not evaluating items, only properties
46+
{ Constants.EnableDefaultItems, "false" },
47+
{ Constants.MSBuildExtensionsPath, AppContext.BaseDirectory }
48+
};
49+
50+
AddUserPassedProperties(globalProperties, args);
51+
return globalProperties;
52+
}
53+
}

‎src/Cli/dotnet/commands/RunProperties.cs

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
using Microsoft.Build.Execution;
5+
using Microsoft.DotNet.Cli.Utils;
6+
7+
namespace Microsoft.DotNet.Cli;
8+
9+
internal record RunProperties(string? RunCommand, string? RunArguments, string? RunWorkingDirectory)
10+
{
11+
internal static RunProperties FromProjectAndApplicationArguments(ProjectInstance project, string[] applicationArgs, bool fallbackToTargetPath)
12+
{
13+
string runProgram = project.GetPropertyValue("RunCommand");
14+
if (fallbackToTargetPath &&
15+
(string.IsNullOrEmpty(runProgram) || !File.Exists(runProgram)))
16+
{
17+
// If we can't find the executable that runCommand is pointing to, we simply use TargetPath instead.
18+
// In this case, we discard everything related to "Run" (i.e, RunWorkingDirectory and RunArguments) and use only TargetPath
19+
runProgram = project.GetPropertyValue("TargetPath");
20+
return new(runProgram, null, null);
21+
}
22+
23+
string runArguments = project.GetPropertyValue("RunArguments");
24+
string runWorkingDirectory = project.GetPropertyValue("RunWorkingDirectory");
25+
26+
if (applicationArgs.Length != 0)
27+
{
28+
runArguments += " " + ArgumentEscaper.EscapeAndConcatenateArgArrayForProcessStart(applicationArgs);
29+
}
30+
31+
return new(runProgram, runArguments, runWorkingDirectory);
32+
}
33+
}

‎src/Cli/dotnet/commands/dotnet-run/RunCommand.cs

Lines changed: 6 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,6 @@ namespace Microsoft.DotNet.Tools.Run;
2323

2424
public partial class RunCommand
2525
{
26-
private record RunProperties(string? RunCommand, string? RunArguments, string? RunWorkingDirectory);
27-
2826
public bool NoBuild { get; }
2927

3028
/// <summary>
@@ -253,7 +251,7 @@ private void EnsureProjectIsBuilt(out Func<ProjectCollection, ProjectInstance>?
253251
EntryPointFileFullPath = EntryPointFileFullPath,
254252
};
255253

256-
AddUserPassedProperties(command.GlobalProperties, RestoreArgs);
254+
CommonRunHelpers.AddUserPassedProperties(command.GlobalProperties, RestoreArgs);
257255

258256
projectFactory = command.CreateProjectInstance;
259257
buildResult = command.Execute(
@@ -314,19 +312,11 @@ private ICommand GetTargetCommand(Func<ProjectCollection, ProjectInstance>? proj
314312
var command = CreateCommandFromRunProperties(project, runProperties);
315313
return command;
316314

317-
static ProjectInstance EvaluateProject(string? projectFilePath, Func<ProjectCollection, ProjectInstance>? projectFactory, string[] restoreArgs, ILogger? binaryLogger)
315+
static ProjectInstance EvaluateProject(string? projectFilePath, Func<ProjectCollection, ProjectInstance>? projectFactory, string[]? restoreArgs, ILogger? binaryLogger)
318316
{
319317
Debug.Assert(projectFilePath is not null || projectFactory is not null);
320318

321-
var globalProperties = new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase)
322-
{
323-
// This property disables default item globbing to improve performance
324-
// This should be safe because we are not evaluating items, only properties
325-
{ Constants.EnableDefaultItems, "false" },
326-
{ Constants.MSBuildExtensionsPath, AppContext.BaseDirectory }
327-
};
328-
329-
AddUserPassedProperties(globalProperties, restoreArgs);
319+
var globalProperties = CommonRunHelpers.GetGlobalPropertiesFromArgs(restoreArgs);
330320

331321
var collection = new ProjectCollection(globalProperties: globalProperties, loggers: binaryLogger is null ? null : [binaryLogger], toolsetDefinitionLocations: ToolsetDefinitionLocations.Default);
332322

@@ -349,20 +339,13 @@ static void ValidatePreconditions(ProjectInstance project)
349339

350340
static RunProperties ReadRunPropertiesFromProject(ProjectInstance project, string[] applicationArgs)
351341
{
352-
string runProgram = project.GetPropertyValue("RunCommand");
353-
if (string.IsNullOrEmpty(runProgram))
342+
var runProperties = RunProperties.FromProjectAndApplicationArguments(project, applicationArgs, fallbackToTargetPath: false);
343+
if (string.IsNullOrEmpty(runProperties.RunCommand))
354344
{
355345
ThrowUnableToRunError(project);
356346
}
357347

358-
string runArguments = project.GetPropertyValue("RunArguments");
359-
string runWorkingDirectory = project.GetPropertyValue("RunWorkingDirectory");
360-
361-
if (applicationArgs.Any())
362-
{
363-
runArguments += " " + ArgumentEscaper.EscapeAndConcatenateArgArrayForProcessStart(applicationArgs);
364-
}
365-
return new(runProgram, runArguments, runWorkingDirectory);
348+
return runProperties;
366349
}
367350

368351
static ICommand CreateCommandFromRunProperties(ProjectInstance project, RunProperties runProperties)
@@ -455,37 +438,6 @@ static FacadeLogger ConfigureDispatcher(List<BinaryLogger> binaryLoggers)
455438
}
456439
}
457440

458-
/// <param name="globalProperties">
459-
/// Should have <see cref="StringComparer.OrdinalIgnoreCase"/>.
460-
/// </param>
461-
private static void AddUserPassedProperties(Dictionary<string, string> globalProperties, string[] args)
462-
{
463-
Debug.Assert(globalProperties.Comparer == StringComparer.OrdinalIgnoreCase);
464-
465-
var fakeCommand = new System.CommandLine.CliCommand("dotnet") { CommonOptions.PropertiesOption };
466-
var propertyParsingConfiguration = new System.CommandLine.CliConfiguration(fakeCommand);
467-
var propertyParseResult = propertyParsingConfiguration.Parse(args);
468-
var propertyValues = propertyParseResult.GetValue(CommonOptions.PropertiesOption);
469-
470-
if (propertyValues != null)
471-
{
472-
foreach (var property in propertyValues)
473-
{
474-
foreach (var (key, value) in MSBuildPropertyParser.ParseProperties(property))
475-
{
476-
if (globalProperties.TryGetValue(key, out var existingValues))
477-
{
478-
globalProperties[key] = existingValues + ";" + value;
479-
}
480-
else
481-
{
482-
globalProperties[key] = value;
483-
}
484-
}
485-
}
486-
}
487-
}
488-
489441
/// <summary>
490442
/// This class acts as a wrapper around the BinaryLogger, to allow us to keep the BinaryLogger alive across multiple phases of the build.
491443
/// The methods here are stubs so that the real binarylogger sees that we support these functionalities.

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

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,6 @@ internal static class CliConstants
4747
public const string MTPTarget = "_MTPBuild";
4848

4949
public const string TestTraceLoggingEnvVar = "DOTNET_CLI_TEST_TRACEFILE";
50-
public const string NetCoreIdentifier = ".NETCoreApp";
5150
}
5251

5352
internal static class TestStates
@@ -93,10 +92,7 @@ internal static class ProjectProperties
9392
internal const string TargetPath = "TargetPath";
9493
internal const string ProjectFullPath = "MSBuildProjectFullPath";
9594
internal const string RunSettingsFilePath = "RunSettingsFilePath";
96-
internal const string IsExecutable = "_IsExecutable";
97-
internal const string UseAppHost = "UseAppHost";
98-
internal const string TargetFrameworkIdentifier = "TargetFrameworkIdentifier";
99-
internal const string TargetDir = "TargetDir";
100-
internal const string AssemblyName = "AssemblyName";
101-
internal const string NativeExecutableExtension = "_NativeExecutableExtension";
95+
internal const string RunCommand = "RunCommand";
96+
internal const string RunArguments = "RunArguments";
97+
internal const string RunWorkingDirectory = "RunWorkingDirectory";
10298
}

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,9 @@ private void LogProjectProperties(IEnumerable<TestModule> modules)
159159
logMessageBuilder.AppendLine($"{ProjectProperties.IsTestProject}: {module.IsTestProject}");
160160
logMessageBuilder.AppendLine($"{ProjectProperties.IsTestingPlatformApplication}: {module.IsTestingPlatformApplication}");
161161
logMessageBuilder.AppendLine($"{ProjectProperties.TargetFramework}: {module.TargetFramework}");
162-
logMessageBuilder.AppendLine($"{ProjectProperties.TargetPath}: {module.TargetPath}");
162+
logMessageBuilder.AppendLine($"{ProjectProperties.RunCommand}: {module.RunProperties.RunCommand}");
163+
logMessageBuilder.AppendLine($"{ProjectProperties.RunArguments}: {module.RunProperties.RunArguments}");
164+
logMessageBuilder.AppendLine($"{ProjectProperties.RunWorkingDirectory}: {module.RunProperties.RunWorkingDirectory}");
163165
logMessageBuilder.AppendLine($"{ProjectProperties.RunSettingsFilePath}: {module.RunSettingsFilePath}");
164166
logMessageBuilder.AppendLine();
165167
}

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

Lines changed: 11 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,11 @@ public static (IEnumerable<TestModule> Projects, bool IsBuiltOrRestored) GetProj
2828
Path.GetDirectoryName(solutionModel.Description) :
2929
SolutionAndProjectUtility.GetRootDirectory(solutionFilePath);
3030

31-
ConcurrentBag<TestModule> projects = GetProjectsProperties(new ProjectCollection(), solutionModel.SolutionProjects.Select(p => Path.Combine(rootDirectory, p.FilePath)), buildOptions);
31+
// TODO: We should pass a binary logger if the dotnet test invocation passed one.
32+
// We will take the same file name but append something to it, like `-dotnet-test-evaluation`
33+
// Tracked by https://github.com/dotnet/sdk/issues/47494
34+
var collection = new ProjectCollection(globalProperties: CommonRunHelpers.GetGlobalPropertiesFromArgs([.. buildOptions.MSBuildArgs]), loggers: [], toolsetDefinitionLocations: ToolsetDefinitionLocations.Default);
35+
ConcurrentBag<TestModule> projects = GetProjectsProperties(collection, solutionModel.SolutionProjects.Select(p => Path.Combine(rootDirectory, p.FilePath)), buildOptions);
3236

3337
return (projects, isBuiltOrRestored);
3438
}
@@ -42,7 +46,11 @@ public static (IEnumerable<TestModule> Projects, bool IsBuiltOrRestored) GetProj
4246
return (Array.Empty<TestModule>(), isBuiltOrRestored);
4347
}
4448

45-
IEnumerable<TestModule> projects = SolutionAndProjectUtility.GetProjectProperties(projectFilePath, GetGlobalProperties(buildOptions), new ProjectCollection());
49+
// TODO: We should pass a binary logger if the dotnet test invocation passed one.
50+
// We will take the same file name but append something to it, like `-dotnet-test-evaluation`
51+
// Tracked by https://github.com/dotnet/sdk/issues/47494
52+
var collection = new ProjectCollection(globalProperties: CommonRunHelpers.GetGlobalPropertiesFromArgs([.. buildOptions.MSBuildArgs]), loggers: [], toolsetDefinitionLocations: ToolsetDefinitionLocations.Default);
53+
IEnumerable<TestModule> projects = SolutionAndProjectUtility.GetProjectProperties(projectFilePath, collection);
4654

4755
return (projects, isBuiltOrRestored);
4856
}
@@ -68,14 +76,10 @@ public static BuildOptions GetBuildOptions(ParseResult parseResult, int degreeOf
6876
parseResult.GetValue(TestingPlatformOptions.NoBuildOption),
6977
parseResult.HasOption(CommonOptions.VerbosityOption) ? parseResult.GetValue(CommonOptions.VerbosityOption) : null,
7078
degreeOfParallelism,
71-
GetGlobalProperties([.. msbuildArgs]),
7279
unmatchedTokens,
7380
msbuildArgs);
7481
}
7582

76-
private static string[]? GetGlobalProperties(IReadOnlyList<string> args)
77-
=> new CliConfiguration(new CliCommand("dotnet") { CommonOptions.PropertiesOption }).Parse(args).GetValue(CommonOptions.PropertiesOption);
78-
7983
private static IEnumerable<string> GetBinaryLoggerTokens(IEnumerable<string> args)
8084
{
8185
return args.Where(arg =>
@@ -110,7 +114,7 @@ private static ConcurrentBag<TestModule> GetProjectsProperties(ProjectCollection
110114
new ParallelOptions { MaxDegreeOfParallelism = buildOptions.DegreeOfParallelism },
111115
(project) =>
112116
{
113-
IEnumerable<TestModule> projectsMetadata = SolutionAndProjectUtility.GetProjectProperties(project, GetGlobalProperties(buildOptions), projectCollection);
117+
IEnumerable<TestModule> projectsMetadata = SolutionAndProjectUtility.GetProjectProperties(project, projectCollection);
114118
foreach (var projectMetadata in projectsMetadata)
115119
{
116120
allProjects.Add(projectMetadata);
@@ -119,19 +123,4 @@ private static ConcurrentBag<TestModule> GetProjectsProperties(ProjectCollection
119123

120124
return allProjects;
121125
}
122-
123-
private static Dictionary<string, string> GetGlobalProperties(BuildOptions buildOptions)
124-
{
125-
var globalProperties = new Dictionary<string, string>(buildOptions.UserSpecifiedProperties.Length);
126-
127-
foreach (var property in buildOptions.UserSpecifiedProperties)
128-
{
129-
foreach (var (key, value) in MSBuildPropertyParser.ParseProperties(property))
130-
{
131-
globalProperties[key] = value;
132-
}
133-
}
134-
135-
return globalProperties;
136-
}
137126
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33

44
namespace Microsoft.DotNet.Cli;
55

6-
internal sealed record TestModule(string? TargetPath, string? ProjectFullPath, string? TargetFramework, string? RunSettingsFilePath, bool IsTestingPlatformApplication, bool IsTestProject);
6+
internal sealed record TestModule(RunProperties RunProperties, string? ProjectFullPath, string? TargetFramework, string? RunSettingsFilePath, bool IsTestingPlatformApplication, bool IsTestProject);
77

88
internal sealed record Handshake(Dictionary<byte, string>? Properties);
99

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,4 +7,4 @@ internal record TestOptions(string Configuration, string Architecture, bool HasF
77

88
internal record PathOptions(string ProjectPath, string SolutionPath, string DirectoryPath);
99

10-
internal record BuildOptions(PathOptions PathOptions, bool HasNoRestore, bool HasNoBuild, VerbosityOptions? Verbosity, int DegreeOfParallelism, string[] UserSpecifiedProperties, List<string> UnmatchedTokens, IEnumerable<string> MSBuildArgs);
10+
internal record BuildOptions(PathOptions PathOptions, bool HasNoRestore, bool HasNoBuild, VerbosityOptions? Verbosity, int DegreeOfParallelism, List<string> UnmatchedTokens, IEnumerable<string> MSBuildArgs);

0 commit comments

Comments
 (0)