Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix InvokeTestingPlatformTask to handle running with msbuild.exe and 'Test' target #4840

Merged
merged 10 commits into from
Feb 4, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<Project Sdk="Microsoft.NET.Sdk">
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFrameworks>$(MicrosoftTestingTargetFrameworks);netstandard2.0</TargetFrameworks>
<DefineConstants>PLATFORM_MSBUILD</DefineConstants>
Expand Down Expand Up @@ -47,6 +47,7 @@ This package provides MSBuild integration of the platform, its extensions and co
<ItemGroup>
<TfmSpecificPackageFile Include="@(ReferenceCopyLocalPaths->WithMetadataValue('ReferenceSourceTarget', 'ProjectReference')->WithMetadataValue('PrivateAssets', 'All'))" PackagePath="lib\$(TargetFramework)\%(ReferenceCopyLocalPaths.DestinationSubDirectory)" />
<BuildOutputInPackage Include="@(ReferenceCopyLocalPaths->WithMetadataValue('ReferenceSourceTarget', 'ProjectReference')->WithMetadataValue('CopyToBuildOutput', 'true'))" TargetPath="%(ReferenceCopyLocalPaths.DestinationSubDirectory)" />
<BuildOutputInPackage Include="@(RuntimeTargetsCopyLocalItems->HasMetadata('NuGetPackageId'))" TargetPath="%(RuntimeCopyLocalPaths.DestinationSubPath)" />
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hopefully this gets us green.

It adds three extra dlls to the package:

image

@rainersigwald Is this the right way to package?

</ItemGroup>
</Target>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,19 @@ public bool TryGetDotnetPathByArchitecture(

_resolutionLog("DotnetHostHelper.TryGetDotnetPathByArchitecture: Muxer was not found using DOTNET_ROOT* env variables.");

// Search in path
muxerPath = GetMuxerFromPath();
if (muxerPath is not null)
{
if (IsValidArchitectureMuxer(targetArchitecture, muxerPath))
{
_resolutionLog($"DotnetHostHelper.TryGetDotnetPathByArchitecture: Muxer compatible with '{targetArchitecture}' resolved from PATH: '{muxerPath}'");
return true;
}

_resolutionLog($"DotnetHostHelper.TryGetDotnetPathByArchitecture: Muxer resolved using PATH is not compatible with the target architecture: '{muxerPath}'");
}

// Try to search for global registration
muxerPath = isWinOs ? GetMuxerFromGlobalRegistrationWin(targetArchitecture) : GetMuxerFromGlobalRegistrationOnUnix(targetArchitecture);

Expand Down Expand Up @@ -205,6 +218,22 @@ public bool TryGetDotnetPathByArchitecture(
return true;
}

private string? GetMuxerFromPath()
{
string values = Environment.GetEnvironmentVariable("PATH")!;
foreach (string? p in values.Split(Path.PathSeparator))
{
string fullPath = Path.Combine(p, _muxerName);
if (File.Exists(fullPath))
{
_resolutionLog($"DotnetMuxerLocator.GetMuxerFromPath: Found {_muxerName} using PATH environment variable: '{fullPath}'");
return fullPath;
}
}

return null;
}

private string? GetMuxerFromGlobalRegistrationWin(PlatformArchitecture targetArchitecture)
{
if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,14 @@ namespace Microsoft.Testing.Platform.MSBuild;
public class InvokeTestingPlatformTask : Build.Utilities.ToolTask, IDisposable
{
private const string MonoRunnerName = "mono";
private const string DotnetRunnerName = "dotnet";
private static readonly string DotnetRunnerName = RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? "dotnet.exe" : "dotnet";

private readonly IFileSystem _fileSystem;
private readonly PipeNameDescription _pipeNameDescription;
private readonly CancellationTokenSource _waitForConnections = new();
private readonly List<NamedPipeServer> _connections = new();
private readonly StringBuilder _output = new();
private readonly Lock _initLock = new();
private readonly Process _currentProcess = Process.GetCurrentProcess();
private readonly Architecture _currentProcessArchitecture = RuntimeInformation.ProcessArchitecture;

private Task? _connectionLoopTask;
Expand Down Expand Up @@ -90,7 +89,7 @@ protected override string ToolName
if (TargetPath.ItemSpec.EndsWith(".dll", StringComparison.InvariantCultureIgnoreCase))
{
Log.LogMessage(MessageImportance.Low, $"Target path is a dll '{TargetPath.ItemSpec}'");
return DotnetRunnerName + (RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? ".exe" : string.Empty);
return DotnetRunnerName;
}

// If the target is an exe and we're not on Windows we try with the mono runner.
Expand Down Expand Up @@ -121,39 +120,27 @@ protected override string ToolName
// We look for dotnet muxer only if we're not running with mono.
if (dotnetRunnerName != MonoRunnerName)
{
if (!IsCurrentProcessArchitectureCompatible())
if (DotnetHostPath is not null && File.Exists(DotnetHostPath.ItemSpec) && IsCurrentProcessArchitectureCompatible())
{
Log.LogMessage(MessageImportance.Low, $"Current process architecture '{_currentProcessArchitecture}' is not compatible with '{TestArchitecture.ItemSpec}'");
PlatformArchitecture targetArchitecture = EnumPolyfill.Parse<PlatformArchitecture>(TestArchitecture.ItemSpec, ignoreCase: true);
StringBuilder resolutionLog = new();
DotnetMuxerLocator dotnetMuxerLocator = new(log => resolutionLog.AppendLine(log));
if (dotnetMuxerLocator.TryGetDotnetPathByArchitecture(targetArchitecture, out string? dotnetPath))
{
Log.LogMessage(MessageImportance.Low, resolutionLog.ToString());
Log.LogMessage(MessageImportance.Low, $"dotnet muxer tool path found using architecture: '{TestArchitecture.ItemSpec}' '{dotnetPath}'");
return dotnetPath;
}
else
{
Log.LogMessage(MessageImportance.Low, resolutionLog.ToString());
Log.LogError(string.Format(CultureInfo.InvariantCulture, Resources.MSBuildResources.IncompatibleArchitecture, dotnetRunnerName, TestArchitecture.ItemSpec));
return null;
}
Log.LogMessage(MessageImportance.Low, $"dotnet muxer tool path found using DOTNET_HOST_PATH environment variable: '{DotnetHostPath.ItemSpec}'");
return DotnetHostPath.ItemSpec;
}

Log.LogMessage(MessageImportance.Low, $"Current process architecture '{_currentProcessArchitecture}'. Requested test architecture '{TestArchitecture.ItemSpec}'");
PlatformArchitecture targetArchitecture = EnumPolyfill.Parse<PlatformArchitecture>(TestArchitecture.ItemSpec, ignoreCase: true);
StringBuilder resolutionLog = new();
DotnetMuxerLocator dotnetMuxerLocator = new(log => resolutionLog.AppendLine(log));
if (dotnetMuxerLocator.TryGetDotnetPathByArchitecture(targetArchitecture, out string? dotnetPath))
{
Log.LogMessage(MessageImportance.Low, resolutionLog.ToString());
Log.LogMessage(MessageImportance.Low, $"dotnet muxer tool path found using architecture: '{TestArchitecture.ItemSpec}' '{dotnetPath}'");
return dotnetPath;
}
else
{
if (DotnetHostPath is not null && File.Exists(DotnetHostPath.ItemSpec))
{
Log.LogMessage(MessageImportance.Low, $"dotnet muxer tool path found using DOTNET_HOST_PATH environment variable: '{DotnetHostPath.ItemSpec}'");
return DotnetHostPath.ItemSpec;
}

ProcessModule? mainModule = _currentProcess.MainModule;
if (mainModule != null && Path.GetFileName(mainModule.FileName)!.Equals(dotnetRunnerName, StringComparison.OrdinalIgnoreCase))
{
Log.LogMessage(MessageImportance.Low, $"dotnet muxer tool path found using current process: '{mainModule.FileName}' architecture: '{_currentProcessArchitecture}'");
return mainModule.FileName;
}
Log.LogMessage(MessageImportance.Low, resolutionLog.ToString());
Log.LogError(string.Format(CultureInfo.InvariantCulture, Resources.MSBuildResources.IncompatibleArchitecture, dotnetRunnerName, TestArchitecture.ItemSpec));
return null;
}
}

Expand All @@ -180,24 +167,25 @@ protected override string GenerateCommandLineCommands()
{
Build.Utilities.CommandLineBuilder builder = new();

if (IsNetCoreApp)
if (ToolName == DotnetRunnerName && IsNetCoreApp)
{
string dotnetRunnerName = ToolName;
if (dotnetRunnerName != MonoRunnerName && Path.GetFileName(_currentProcess.MainModule!.FileName!).Equals(dotnetRunnerName, StringComparison.OrdinalIgnoreCase))
{
builder.AppendSwitch("exec");
builder.AppendFileNameIfNotNull(TargetPath.ItemSpec);
}
// In most cases, if ToolName is "dotnet.exe", that means we are given a "dll" file.
// In turn, that means we are not .NET Framework (because we will use Exe otherwise).
// In case ToolName ended up being "dotnet.exe" and we are
// .NET Framework, that means it's the user's assembly that is named "dotnet".
// In that case, we want to execute the tool (user's executable) directly.
// So, we only only "exec" if we are .NETCoreApp
builder.AppendSwitch("exec");
builder.AppendFileNameIfNotNull(TargetPath.ItemSpec);
}
else
else if (ToolName == MonoRunnerName)
{
// If the target is an exe and we're not on Windows we try with the mono runner and so we pass the test module to the mono runner.
if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows) && TargetPath.ItemSpec.EndsWith(".exe", StringComparison.InvariantCultureIgnoreCase))
{
builder.AppendFileNameIfNotNull(TargetPath.ItemSpec);
}
// If ToolName is "mono", that means TargetPath is an "exe" file and we are not running on Windows.
// In this case, we use the given exe file as an argument to mono.
builder.AppendFileNameIfNotNull(TargetPath.ItemSpec);
}

// If we are not "dotnet.exe" and not "mono", then we are given an executable from user and we are running on Windows.
builder.AppendSwitchIfNotNull($"--{MSBuildConstants.MSBuildNodeOptionKey} ", _pipeNameDescription.Name);

if (!string.IsNullOrEmpty(TestingPlatformCommandLineArguments?.ItemSpec))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,31 @@ public static void ClassCleanup()
}
}

// https://github.com/NuGet/NuGet.Client/blob/c5934bdcbc578eec1e2921f49e6a5d53481c5099/test/NuGet.Core.FuncTests/Msbuild.Integration.Test/MsbuildIntegrationTestFixture.cs#L65-L94
private protected static async Task<string> FindMsbuildWithVsWhereAsync()
{
string vswherePath = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.ProgramFilesX86), "Microsoft Visual Studio", "Installer", "vswhere.exe");
var commandLine = new TestInfrastructure.CommandLine();
await commandLine.RunAsync($"\"{vswherePath}\" -latest -prerelease -requires Microsoft.Component.MSBuild -find MSBuild\\**\\Bin\\MSBuild.exe");

string? path = null;
using (var stringReader = new StringReader(commandLine.StandardOutput))
{
string? line;
while ((line = await stringReader.ReadLineAsync()) != null)
{
if (path != null)
{
throw new Exception("vswhere returned more than 1 line");
}

path = line;
}
}

return path!;
}

private static string ExtractVersionFromPackage(string rootFolder, string packagePrefixName)
{
string[] matches = Directory.GetFiles(rootFolder, packagePrefixName + "*" + NuGetPackageExtensionName, SearchOption.TopDirectoryOnly);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,29 @@ private async Task InvokeTestingPlatform_Target_Should_Build_Without_Warnings_An
}
}

[TestMethod]
[OSCondition(OperatingSystems.Windows)]
public async Task RunUsingTestTargetWithNetfxMSBuild()
{
using TestAsset testAsset = await TestAsset.GenerateAssetAsync(
AssetName,
SourceCode
.PatchCodeWithReplace("$PlatformTarget$", string.Empty)
.PatchCodeWithReplace("$TargetFrameworks$", $"<TargetFramework>{TargetFrameworks.NetCurrent}</TargetFramework>")
.PatchCodeWithReplace("$AssertValue$", bool.TrueString.ToLowerInvariant())
.PatchCodeWithReplace("$MicrosoftTestingPlatformVersion$", MicrosoftTestingPlatformVersion));

string msbuildExe = await FindMsbuildWithVsWhereAsync();
var commandLine = new TestInfrastructure.CommandLine();
string binlogFile = Path.Combine(testAsset.TargetAssetPath, Guid.NewGuid().ToString("N"), "msbuild.binlog");
await commandLine.RunAsync($"\"{msbuildExe}\" {testAsset.TargetAssetPath} /t:Restore");
await commandLine.RunAsync($"\"{msbuildExe}\" {testAsset.TargetAssetPath} /t:\"Build;Test\" /bl:\"{binlogFile}\"", environmentVariables: new Dictionary<string, string?>()
{
["DOTNET_ROOT"] = string.Empty,
});
StringAssert.Contains(commandLine.StandardOutput, "Tests succeeded");
}

[TestMethod]
public async Task Invoke_DotnetTest_With_Arch_Switch_x86_Should_Work()
{
Expand Down
17 changes: 14 additions & 3 deletions test/Utilities/Microsoft.Testing.TestInfrastructure/CommandLine.cs
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,19 @@ public async Task RunAsync(
}
}

private static (string Command, string Arguments) GetCommandAndArguments(string commandLine)
{
// Hacky way to split command and arguments that works with the limited cases we use in our tests.
if (!commandLine.StartsWith('"'))
{
string[] tokens = commandLine.Split(' ');
return (tokens[0], string.Join(" ", tokens.Skip(1)));
}

int endQuote = commandLine.IndexOf('"', 1);
return (commandLine.Substring(1, endQuote - 1), commandLine.Substring(endQuote + 2));
}

public async Task<int> RunAsyncAndReturnExitCodeAsync(
string commandLine,
IDictionary<string, string?>? environmentVariables = null,
Expand All @@ -65,9 +78,7 @@ public async Task<int> RunAsyncAndReturnExitCodeAsync(
try
{
Interlocked.Increment(ref s_totalProcessesAttempt);
string[] tokens = commandLine.Split(' ');
string command = tokens[0];
string arguments = string.Join(" ", tokens.Skip(1));
(string command, string arguments) = GetCommandAndArguments(commandLine);
_errorOutputLines.Clear();
_standardOutputLines.Clear();
ProcessConfiguration startInfo = new(command)
Expand Down