arcade icon indicating copy to clipboard operation
arcade copied to clipboard

Revise how `TestArchitectures` is used by test infrastructure

Open Youssef1313 opened this issue 5 months ago • 9 comments

Arcade testing infrastructure allows projects to set TestArchitectures property. This property can have multiple values semicolon-separated, and it will cause RunTests target to run for each architecture.

  • VSTest.targets
    • Always used by NUnit.targets
    • Used by MSTest.targets only if not MTP
    • Used by XUnit.targets and XUnitV3.targets when UseVSTestRunner is true.
    • It does the following:
      • <_TestRunnerCommand>&quot;$(DotNetTool)&quot; test $(_TestAssembly) --logger:"console%3Bverbosity=normal" --logger:"trx%3BLogFileName=$(_TestResultTrxFileName)" --logger:"html%3BLogFileName=$(_TestResultHtmlFileName)" "--ResultsDirectory:$(_TestResultDirectory)" "--Framework:%(TestToRun.TargetFrameworkIdentifier),Version=%(TestToRun.TargetFrameworkVersion)"</_TestRunnerCommand>
    • It looks like the architecture is completely unused. So a test project specifying <TestArchitectures>x64;x86</TestArchitecture> is likely to end up running the tests twice for x64 and not for x86 at all.
  • XUnit.Runner.targets
    • Used by XUnit.targets (xunit 2) when UseVSTestRunner isn't true.
    • On netfx, it runs either xunit.console.x86.exe (if x86) or the x64 xunit.console.exe otherwise, which is good.
    • On Core, we load xunit.console.dll in dotnet.exe.
      • If architecture is x86, we try to run with dotnet.exe that is x86 (we fallback to $(DotNetTool) if x86 dotnet.exe isn't found! We should error instead!)
      • Otherwise, we don't care about the specified architecture and always use $(DotNetTool) which might not be correct.
  • XUnitV3.Runner.targets
    • Used by XUnitV3.targets when UseVSTestRunner isn't true.
    • We simply have an executable to run. And we use RunCommand to find the exe and we pass RunArguments and that is it. This cannot work correctly with architectures and the value is completely ignored.
    • Should we somehow detect when multiple architectures are specified, or even when a single architecture isn't matching the produced apphost architecture, and then fallback to using the right dotnet.exe + dll? Or is there a way to produce multiple apphosts during build for each of the specified architectures?
  • Microsoft.Testing.Platform.targets
    • Used either directly by specifying <TestRunnerName>Microsoft.Testing.Platform</TestRunnerName> (most repos don't need that), or it's also used by MSTest.targets when MTP is enabled.
    • It has the exact same flow as XUnitV3.Runner.targets
    • In future we will want to even refactor it to share as much between MTP.targets and XUnitV3.Runner.targets but that's a different discussion unrelated to the architecture-related issues we have today.

Moreover, this usage of PlatformTarget is suspicious:

https://github.com/dotnet/arcade/blob/e2fed65f9c524d12c64876194ae4ce177b935bb3/src/Microsoft.DotNet.Arcade.Sdk/tools/Tests.targets#L30-L33

  • On .NET Framework, the value is unusable during evaluation.
    • https://github.com/dotnet/project-system/issues/5901#issuecomment-1101862260
  • On .NET Core, this property should never be read.

@agocke @jaredpar @ViktorHofer What do you think here please?

FYI @nohwnd @Evangelink

Youssef1313 avatar Jul 28 '25 04:07 Youssef1313

What's an alternative to use instead of PlatformTarget?

ViktorHofer avatar Jul 28 '25 08:07 ViktorHofer

On .NET Framework, I don't know what will be the best thing to use given that PlatformTarget is unusable during evaluation it seems. Or, maybe we can delay our read of PlatformTarget and have the RunTests target depend on AdjustDefaultPlatformTargetForNetFrameworkExeWithNoNativeCopyLocalItems, but it might not be trivial to depend on AdjustDefaultPlatformTargetForNetFrameworkExeWithNoNativeCopyLocalItems as it probably relies on other targets that are restore/build related, and we want to allow building and testing as separate steps. @agocke / @jaredpar will know better here though.

On .NET Core, there are two paths:

  • If we are building an executable that we want to run directly, we need to build it with the proper RuntimeIdentifier. That means the architecture should be known on build, before we run the tests, and that kills the idea of testing multiple architecture, unless there is some trick that I can't see.
  • Alternatively, we could ignore the "exe" (apphost), and instead run the dll with the right dotnet.

Youssef1313 avatar Jul 28 '25 08:07 Youssef1313

Internal chat thread on that: https://teams.microsoft.com/l/message/19:[email protected]/1753472122810?tenantId=72f988bf-86f1-41af-91ab-2d7cd011db47&groupId=4ba7372f-2799-4677-89f0-7a1aaea3706c&parentMessageId=1753472122810&teamName=.NET%20Developer%20Experience&channelName=MSBuild&createdTime=1753472122810

Evangelink avatar Jul 28 '25 11:07 Evangelink

@dibarbet

jaredpar avatar Jul 28 '25 13:07 jaredpar

Ping @dibarbet

We need to have some clarity on how to exactly approach the above issues so we can implement it.

Youssef1313 avatar Jul 31 '25 12:07 Youssef1313

On .NET Framework, I don't know what will be the best thing to use given that PlatformTarget is unusable during evaluation it seems. Or, maybe we can delay our read of PlatformTarget and have the RunTests target depend on AdjustDefaultPlatformTargetForNetFrameworkExeWithNoNativeCopyLocalItems, but it might not be trivial to depend on AdjustDefaultPlatformTargetForNetFrameworkExeWithNoNativeCopyLocalItems as it probably relies on other targets that are restore/build related, and we want to allow building and testing as separate steps. @agocke / @jaredpar will know better here though.

Think I'm probably not the right person to answer questions on how this works - @jjonescz may be

dibarbet avatar Jul 31 '25 17:07 dibarbet

I'm not sure what's the problem with PlatformTarget. That's set in the project file in .net framework projects or empty for default behavior (AnyCpu). In case it's AnyCpu, you want to use current architecture? Perhaps you can use $(PROCESSOR_ARCHITECTURE) or something like that?

jjonescz avatar Aug 01 '25 15:08 jjonescz

@jjonescz PlatformTarget is only relevant on .NET Framework, but Arcade uses it everywhere it seems. Even on .NET Framework, its value isn't correct during evaluation. It will be x86, but then it's reassigned in AdjustDefaultPlatformTargetForNetFrameworkExeWithNoNativeCopyLocalItems

Youssef1313 avatar Aug 01 '25 15:08 Youssef1313

Moreover, there are the issues in places where we execute exe directly and we cannot do anything with TestArchitectures, which might be misleading if this property isn't supported under every test runner. In that case, I don't know if the best solution there is to fallback to dotnet exec or if we have a better way of doing it.

Youssef1313 avatar Aug 01 '25 15:08 Youssef1313