Adding ARM64 support to the test console #1927
Conversation
| { | ||
| get | ||
| { | ||
| #if !NETSTANDARD1_4 |
There was a problem hiding this comment.
Can't this come from the PlatformEnvironment ?
There was a problem hiding this comment.
No due to backwards compat reasons this API normally lies about the OS architecture, so this is the way to do it.
|
@dotMorten just because I'm curious, will you finish your work here? |
| { | ||
| // use the new IsWow64Process2 to detect ARM64 as well. | ||
| var processHandle = System.Diagnostics.Process.GetCurrentProcess().Handle; | ||
| if (IsWow64Process2(processHandle, out ImageFileMachine pProcessMachinem, out ImageFileMachine pNativeMachine)) |
There was a problem hiding this comment.
Is the m at the end of pProcessMachinem a typo?
There was a problem hiding this comment.
I submitted this PR almost a year and a half ago, so honestly I have no clue but probably? 🤷♂️. To be honest, I don't even remember submitting this PR.
Feel free to take what I have and work with it, but with such a delay in response and in the mean time I found other means for testing on ARM64, I don't see myself picking this PR up.
There was a problem hiding this comment.
I will most likely pick this up, when we get ARM64 hardware.
| case ImageFileMachine.IA64: | ||
| return Architecture.X64; |
There was a problem hiding this comment.
IA64 (Itanium) should not be mapped to X64.
| if (architecture == Architecture.X86 && (osArchitecture == Architecture.X64 || osArchitecture == Architecture.ARM64)) | ||
| { | ||
| return; | ||
| } | ||
| if ((architecture == Architecture.ARM || architecture == Architecture.X86) && osArchitecture == Architecture.ARM64) |
There was a problem hiding this comment.
The combination architecture == Architecture.X86 && osArchitecture == Architecture.ARM64 is checked twice. Using IsWow64GuestMachineSupported may be more appropriate here.
| </data> | ||
| <data name="InvalidPlatformType" xml:space="preserve"> | ||
| <value>Invalid platform type:{0}. Valid platform types are x86, x64 and Arm.</value> | ||
| <value>Invalid platform type:{0}. Valid platform types are x86, x64, Arm and Arm64.</value> |
There was a problem hiding this comment.
capitalize ARM and ARM64 in messages?
There was a problem hiding this comment.
Nope! This is the correct capitalization
There was a problem hiding this comment.
I would use lower case only as in C# compiler usage help. In any case the two lists below should be consistent, and they are not (@nohwnd):
vstest/src/Microsoft.TestPlatform.ObjectModel/Architecture.cs
Lines 11 to 16 in b999506
|
@nohwnd I assume you will continue with this work? I guess we still want/need the additional supported architectures? |
|
@ViktorHofer yes, but plain tests seem to work just fine on ARM64 already with the custom path to dotnet or the PR that was merged this morning #2479 . |
|
Do you know the reason why VSTest isn't AnyCPU by default? Why do we need to pass in the architecture on Core at all? |
|
the runner (vstest.console) is anyCPU, but we have architecture specific hosts (testhost), to run the processes at the correct bittness that you select via the settings, to run your tests. |
|
I see, that makes sense. Still wondering why the testhost isn't part of the (platform specific) SDK but delivered via the nuget package. |
|
@ViktorHofer Because then it can be updated independently, so you don't need to fix your tests every time you install new dotnet sdk. Also you can get updates faster in case you are stuck with a given dotnet sdk. |
|
@dotMorten Thank you for the hard work and sorry it took so long! I will close this PR as we have been merging a couple of PRs providing support for ARM64 in the past months. Feel free to test it out and let us know if we missed any case. |
7 participants
Description
I've found that I'm not able to run UWP ARM64 unit test on ARM64 devices.
Looking through the code, there's no indication ARM64 was ever supported, and the OS Architecture detection was getting the wrong values.
This is the beginnings of adding support for executing ARM64 unit tests.
I'm submitting as a draft for now, for feedback, and will also require some more testing at my end.