Honor --arch switch for arm64 on Windows and Mac
#3100
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nohwnd
reviewed
Oct 13, 2021
src/Microsoft.TestPlatform.CoreUtilities/Helpers/DotnetHostHelper.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.TestPlatform.CoreUtilities/Helpers/DotnetHostHelper.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.TestPlatform.CoreUtilities/Helpers/DotnetHostHelper.cs
Outdated
Show resolved
Hide resolved
…oid testhost.dll override for default msbuild vars.
…host, validate platform argumet, fix some units
|
Manual testing on Installed sdk(in this order): Global install locations /etc/dotnet/install_location-install_location_{arch} Sample project(using local TP build) <Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFrameworks>net6.0;net5.0</TargetFrameworks>
<IsPackable>false</IsPackable>
</PropertyGroup>
<ItemGroup>
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.1.0-dev" />
<PackageReference Include="MSTest.TestAdapter" Version="2.2.7" />
<PackageReference Include="MSTest.TestFramework" Version="2.2.7" />
<PackageReference Include="coverlet.collector" Version="3.0.2" />
</ItemGroup>
</Project>Source using Microsoft.VisualStudio.TestTools.UnitTesting;
namespace TestProjectNetcore
{
[TestClass]
public class UnitTest1
{
[TestMethod]
public void TestMethod1()
{
System.Console.WriteLine("OSArchitecture " + System.Runtime.InteropServices.RuntimeInformation.OSArchitecture.ToString());
System.Console.WriteLine("ProcessArchitecture " + System.Runtime.InteropServices.RuntimeInformation.ProcessArchitecture.ToString());
Assert.IsTrue(false);
}
}
}We did some update to the sdk for the Scenario 1: use global arm64 sdk installationCommand line dotnet test -p:VSTestConsolePath=".../TP/Debug/netcoreapp2.1/vstest.console.dll" TestProjectMacResults Run tests with dotnet test -p:VSTestConsolePath=".../TP/Debug/netcoreapp2.1/vstest.console.dll" TestProjectMac --arch x64Result Scenario 2: private x64 installationCommand line ./dotnetX64/dotnet test -p:VSTestConsolePath=".../TP/Debug/netcoreapp2.1/vstest.console.dll" TestProjectMacResult net5.0 is failing because we're using private net6.0/x64 installation Run tests with ./dotnetX64/dotnet test -p:VSTestConsolePath=".../TP/Debug/netcoreapp2.1/vstest.console.dll" TestProjectMac --arch arm64Result Scenario 3: private arm64 installationCommand line ./dotnet/dotnet test -p:VSTestConsolePath=".../TP/Debug/netcoreapp2.1/vstest.console.dll" TestProjectMacResult Run tests with ./dotnet/dotnet test -p:VSTestConsolePath=".../TP/Debug/netcoreapp2.1/vstest.console.dll" TestProjectMac --arch x64Result Scenario 4: only private install using env varsPrivate arm64 installation ./dotnet/dotnet test -p:VSTestConsolePath=".../TP/Debug/netcoreapp2.1/vstest.console.dll" TestProjectMacResult Forwarding for net5.0 fails because no x64 found. After Like the above net5.0 fails because we're redirecting to x64 and it's present but doesn't contain net5.0 runtime ./dotnet/dotnet test -p:VSTestConsolePath=".../TP/Debug/netcoreapp2.1/vstest.console.dll" TestProjectMac --arch x64Result The net5.0 fails because we use private net6.0 x64 and muxer is not able to find the runtime, like the above Result The net5.0 fails because x64 private is net6.0 only ./dotnetX64/dotnet test -p:VSTestConsolePath=".../TP/Debug/netcoreapp2.1/vstest.console.dll" TestProjectMacResult The net5.0 fails because x64 private is net6.0 only Now set the arch env Result Unset arch specific env var and use DOTNET_ROOT Result Scenario 5: global x64 installationIn case of global x64 installation muxer won't be available on PATH and so we need to refer to it directly /usr/local/share/dotnet/x64/dotnet test -p:VSTestConsolePath=".../TP/Debug/netcoreapp2.1/vstest.console.dll" TestProjectMacResult Run tests with Now set arm64 on env vars Result The net5.0 fails because it's not supported on arm64 Result The net5.0 fails because it's not supported on arm64 --END TESTS-- @richlander these are the manual test on M1, pls can you double check if I've covered all scenario? cc: @vitek-karas |
Ideally, @MarcoRossignoli and I talked offline. The main thing we need to validate in those private scenarios is that the The behavior initially felt like multi-level-lookup. I can see on further thought that there are two aspects to this:
There should be a way to disable global lookup for runtimes. Is that MLL? Here's a good MLL test (all Arm64):
With MLL disabled, I would expect |
|
Another scenario is using the global x64 SDK. It would be nice to see a working .NET 5 x64 runtime (just move it to the right place) so that we see more success cases. It looks like the right things are happening, however, success is still better. |
I've removed custom error message, now will fail like in
I've moved net5.0 in the correct folder, now works as expected(scenario outcome updated)
I've sent to @vitek-karas some unexpected behavior for I'll complete missing scenarios asap(MLL, global x64 SDK). |
Added global x64 Scenario 5 |
|
The error message:
The first part is OK, but the suggestion should use product terminology, so something like "Please install .NET x64 on the machine" or something along those lines. Also once we have stable download links, we should provide a link to download. For now we can at least point people to the main download page. Alternatively we could generate similar URL to what apphost does (which includes the desired architecture and version). For the tests - just to be 100% sure, can you please modify the test to print out the location of the runtime?
I agree with Rich that this should have a similar behavior to
There's no such thing as multi level lookup (MLL) on macOS. Setting the Which runtime will be used to actually run the test then follow the exact same rules as |
vitek-karas
suggested changes
Oct 25, 2021
src/Microsoft.TestPlatform.CoreUtilities/Helpers/DotnetHostHelper.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.TestPlatform.CoreUtilities/Helpers/DotnetHostHelper.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.TestPlatform.CoreUtilities/Helpers/DotnetHostHelper.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.TestPlatform.CoreUtilities/Helpers/DotnetHostHelper.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.TestPlatform.CoreUtilities/Helpers/DotnetHostHelper.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.TestPlatform.CoreUtilities/Helpers/DotnetHostHelper.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.TestPlatform.CoreUtilities/Helpers/DotnetHostHelper.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.TestPlatform.CoreUtilities/Helpers/DotnetHostHelper.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.TestPlatform.CoreUtilities/Helpers/DotnetHostHelper.cs
Outdated
Show resolved
Hide resolved
vitek-karas
approved these changes
Oct 25, 2021
…ue to Path.Combine in assertions
nohwnd
requested changes
Oct 27, 2021
There was a problem hiding this comment.
Looks good, blocked on the case sensitive check for architecture parameter, and hiding existing public api. Those are breaking changes.
There are many changes in the logging, I suggest batching the changes up before comitting using the add suggestion to batch buttons.
src/Microsoft.TestPlatform.CoreUtilities/Helpers/DotnetHostHelper.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.TestPlatform.CoreUtilities/Helpers/DotnetHostHelper.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.TestPlatform.CoreUtilities/Helpers/DotnetHostHelper.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.TestPlatform.CoreUtilities/Helpers/DotnetHostHelper.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.TestPlatform.CoreUtilities/Helpers/DotnetHostHelper.cs
Outdated
Show resolved
Hide resolved
src/package/nuspec/Microsoft.TestPlatform.TestHost.NetCore.props
Outdated
Show resolved
Hide resolved
nohwnd
approved these changes
Oct 27, 2021
This was referenced Jan 13, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
#1415129
Fix dotnet/sdk#21391
Fix #2576
Fix #2566
Fix #1296