Add TargetArch (or similar) as a xunit.netcore.extensions

[sdmaclea]

It would be nice to have something similar to TestPlatforms for target arch x64,x86,arm,arm64

https://github.com/dotnet/buildtools/blob/2f0dec8eba53942a758910ea7b4cc79e4a7d2c36/src/xunit.netcore.extensions/TestPlatforms.cs#L10-L19

This would be intended to be used in corefx to extend the [ActiveIssue(int issue, TestPlatforms platforms)]test filtering to allow disabling for a specific architecture.

Current usage includes cases like this [ActiveIssue(2743, TestPlatforms.AnyUnix & ~TestPlatforms.OSX)]

This would allow something like [ActiveIssue(2743, TestPlatforms.AnyUnix, TestArch.ARM)]

Arm has a list of failing test https://github.com/dotnet/coreclr/issues/16001

It is currently disabling whole test suites.
https://github.com/dotnet/coreclr/blob/235a34673994560deb6011be08aff18b4399658b/tests/scripts/run-corefx-tests.bat#L20-L24

This would provide a mechanism for fime grain control.

x86 and arm64 will have similar issues. See https://github.com/dotnet/coreclr/issues/17020 for instance.

@BruceForstall @jkotas

[jkotas]

@pgavlin tried to make this happen before he left: #1659

I am wondering what is the right level of metadata granularity to include inline with the corefx tests vs. in side-file.

The OS (Windows, Unix, ...) make sense to include inline with the corefx tests because of the OS differences are general concern for corefx repo.

The arch differences (arm, arm64, x86, ...) make less sense. And the specialized runtime configurations like GC stress, R2R, ... make even less sense to include inline with corefx tests because of those are not a concern for corefx repo. I think we may need a way to disable xunit tests via exclude file to make this work reasonably well, in addition to the existing capability to disable tests inline.

[sdmaclea]

Maybe

-notrait category=failing_arm

Is sufficient

[sdmaclea]

If the xunit.netconsole.exe could list the test class w/ method and consume a filtered list. We could add infrastructure to do this.

The only way I see to get a list of tests is to run them.

[sdmaclea]

There are a lot of tests disable because of open issues. Per the developer guide they look like transient issues which are supposed to eventually be resolved. Seemed like this was how corefx was keeping CI green.

The exclusion list @BruceForstall used for arm is a bit heavy.

[sdmaclea]

I'd love to enable corefx testing for arm64, but if it has to be green first... It may never happen. The exclusion lists are a reasonable start.

[sdmaclea]

I looked briefly at #1659 It looks about exactly what I was describing.

The patch looks like it is in good shape, any reason it can not be merged?

[sdmaclea]

Granularity of Metadata.

On linux-arm64, the Sytem.IO.Pipes test has 505 checks. One is failing when run in unpriviledged. I would rather not set up my CI with sudo, because it make my machines vulnerable. I would like to disable the single check, rather then the whole suite.

One option is to post process the testResults.xml files to mark the expected failures as if they were skipped..
That works as long as there isn't a hang or crash.

[jkotas]

One is failing when run in unpriviledged. I would rather not set up my CI with sudo, because it make my machines vulnerable. I would like to disable the single check, rather then the whole suite.

This test should be disabled, fixed or moved to outter loop category on all Unixes, not just for arm64. We want the default corefx tests to pass in any environment: without admin rights, on non-English locales, in containers, etc.

Here are a few examples of similar fixes:

https://github.com/dotnet/corefx/pull/28391 https://github.com/dotnet/corefx/pull/28317

[BruceForstall]

@sdmaclea I agree that the whole-assembly disabling mechanism I introduced for Windows ARM is too heavyweight (it was an expedient, really).

https://github.com/dotnet/buildtools/pull/1659 seems reasonable to me. Note, however, that Pat is gone, so it either needs to go in as-is, or someone needs to pick it up and update/PR/merge it.

In conversations with @jkotas, we've talked about a desire to have some kind of test corefx exclusion mechanism that would live as a checked-in file in the coreclr repro, to facilitate breaking changes, as well as our requirements around testing many JIT stress modes. I haven't thought much about what that should look like, though, because I have very little experience working with the corefx tests.

Maybe @weshaggard has some comments here.

[sdmaclea]

One approach here might be to insert "ActiveIssue" when CoreCLR builds it.

A bash script like this could be stored in coreclr for each platform and would be simple enough.

#/bin/bash

# System.Diagnostics.Tests.ProcessTests.TestMainModule
sed -i \
  '/^ *public void TestMainModule() *$/i \
  [ActiveIssue("https://github.com/dotnet/coreclr/issues/17174")]' \
  src/System.Diagnostics.Process/tests/ProcessTests.cs

# System.Diagnostics.Tests.ProcessTests.TestId
sed -i \
  '/^ *public void TestId() *$/i \
  [ActiveIssue("https://github.com/dotnet/coreclr/issues/17174")]' \
  src/System.Diagnostics.Process/tests/ProcessTests.cs

[sdmaclea]

Maybe a new category would be better [FailingCoreCLR] then the tests could selected by -notrait and -trait options

[jkotas]

I think it is important for the CoreCLR-specific test exclusion mechanism to not require modifications of the test sources.

[BruceForstall]

Note that with https://github.com/dotnet/coreclr/pull/18365 we're getting closer to fine-grained exclusions. We still need to expand this support to all platforms and all stress modes.

[ViktorHofer]

// Auto-generated message

Going forward, the .NET team is using https://github.com/dotnet/arcade to develop the code and issues formerly in this repository. Feel free to reopen/move this issue if it still applies to servicing branches in this repository, or source code which now resides in the Arcade repository.