Proposal revise CoreCLR test build/run design to reduce complexity

[sdmaclea]

Background on complexity

#36253 is adding the ability to build all the coreclr managed tests on any platform.

Working on that PR reminded me how complex our build and test run code is.

Sampling of issues:

• Too many languages YAML, Powershell, Bash, Batch\Cmd.EXE, python, MSBUILD, C#
• Too many layers. For instance to run in normal config from CI ....
  • YAML ->
  • dotnet MSBuild helixpublishwitharcade.proj
  • run-test.sh
  • run-test.py
  • dotnet MSBuild runtest.proj,
  • <test>.XUnitWrapper.dll
  • bash
  • <test.sh>
  • corerun <assembly>
• Lots of code in non-productive languages
  • MSBuild
  • Batch
• Too many process boundaries
• Test bloat from excessive scripts and XUnitWrapper.dlls
• Test meta-data buried in scripts
• Scenario flow buried in scripts
• Multiple ways to disable a test
• Incomplete and imprecise ways to disable tests

Constraints/Goal

• Allow Target specific disabling of tests by scenario at run time
• Minimize CI moving parts for efficiency and reliability
• Prepare to move to dotnet test for running tests
• Allow running on newer/incomplete platforms
  + w/o a released runtime
  + w/o a stable runtime
• Simplify add more scenarios
• Accommodate need for advanced tests
  • COM
  • AppHost
  • SingleFile
  • ILLinker

Proposed design direction

Test metadata

• Currently test metadata is stored in csproj/ilproj files.

  • Keep it there for now
  • Design will make it feasible to relocate where we want in the future

• Refine metadata tags for ability to disable running tests globally and per scenario

  • Use tags as they exist in Build all managed coreclr tests on OSX in CI #36253 except....
  • Remove Condition
  • Move conditions into values
  • Allow a finite set of values for disable test on target features descriptor tags for disabling (case insensitive)
    • true - all targets
    • arm, arm64, x64 ...
    • Windows, Unix, ...
    • ; -> or with clear precedence semantics
    • - -> and with grammatical connection
    • Possible
      • 32bit, 64bit
      • crossgen2, varargs, COM
      • longrunning
      • expect fail, expect
  • Verify tags in test runner assembly below

• Create an <test>.xunit.cs for each test (with MSBuild during build-test.*)

Metadata as C# attributes for xUnit.net

• Keep per test metadata compact
• For each test populate a ClrTestCaseAttribute
• Use named arguments to populate test case attributes
• Add minimal boiler plate. Minimum would be to attach the attribute to the assembly.

// Sample minimal test file
[assembly:ClrTestCase(Test="Interop.COM.IUnkown", TargetUnsupported="Unix")]

// Sample Attri
[AttributeUsage(AttributeTargets.Assembly, AllowMultiple=true)]
public class ClrTestCaseAttribute : Attribute
{
    public string? Test                      {get; set;}
    public string? TargetUnsupported         {get; set;}
    public string? GCStressIncompatible      {get; set;}
    public string? HeapVerifyIncompatible    {get; set;}
    public string? JitOptimizationSensitive  {get; set;}
    public string? UnloadabilityIncompatible {get; set;}
}

// Sample MSBuild generate C# xUnit test case initial minimal approach 
public partial class ClrTests
{
  [ClrTestCase(Test:"Interop.COM.ComWrappers.GlobalInstance.GlobalInstanceMarshallingTests",
                        TargetUnsupported:"Unix")]
  static void Test_GlobalInstanceMarshallingTests_hash12345678() 
  {
      Run("Interop.COM.ComWrappers.GlobalInstance.GlobalInstanceMarshallingTests");
  }
}

• This approach makes it more difficult to fully support xUnit.net features.
  • The simple consumer would simply treat the test cases as [Theory] data. But wouldn't enable filtering on test features (see below)
  • A full featured approach would require implementing ITestDiscovery and ITraitDiscovery

// Simple assembly ClrTestCaseAttribute consumer
public partial class ClrTests
{
    [Theory]
    [MemberData(nameof(TestCases))]
    public void RunTestCase(ClrTestCaseAttribute testCase)
    {
        // TBD
    }

    public static IEnumerable<ClrTestCaseAttribute> TestCases => GetTestCases();

    private static IEnumerable<ClrTestCaseAttribute> GetTestCases()
    {
        foreach (Attribute a in Attribute.GetCustomAttributes(typeof(ClrTests).Assembly, typeof(ClrTestCaseAttribute)))
        {
            yield return (ClrTestCaseAttribute) a;
        }
    }
}

ClrTests.dll Xunit runner dll

• Move to a singe dll to run all tests.

• Build late in the build managed test phase

• Simple project, depends on all built runnable test projects

• Includes it own source

• Includes <test_artifacts\**\*.xunit.cs described above

• Will run as dotnet test ClrTests.dll

  • Takes configuration from environment variables

• Also will run as dotnet ClrTests.dll <arguments>

  • Takes configuration form command line

• Supports running tests with or without bash/batch scripts

• Can generate Bash/Batch scripts for tests on demand

• Support an option configuration to run using scripts

Stop generating bash/batch scripts using MSBuild, use ClrTests.dll instead

• C# is easier to maintain.
• Reduce burden on MSBuild which is already slow.

Stop using scripts to run on platforms which don't need them

• Save lots of process forks and bash/batch script parsing time.

• Saves compressing and decompressing 10K*2 highly similar scripts

• If we are worried about bit rot, we could test the scripts periodically. Weekly might be sufficient.

Phases and integration

• This effort would be incremental.
• Developed and tested on the dev/infrastructure branch
• New system would be fully tested on CI and developer builds before tearing down the obsolete infrastructure

Discussion

I would like feedback around the proposal.

• Does it have fundamental flaws?
• Will it cause new issues?

/cc @janvorli @jkotas @echesakovMSFT @dotnet/runtime-infrastructure @vitek-karas @AaronRobinsonMSFT

[AaronRobinsonMSFT]

@sdmaclea Thanks for starting this issue. I think there are a lot of good ideas above, but I don't know if they could be used for some areas.

1. Would all tests be run in the same test process? This isn't possible for many interop scenarios because we pollute the environment which means previous tests could have poisoned the runtime and future tests would fail. The gist here is we would need to run each interop test in a separate process. The JIT probably has similar test constraints - @dotnet/jit-contrib.

2. What does the inner loop look like? Right now, I can build the runtime and libraries once and then build and run a single test multiple times without issue. It is also very easy to debug by launching that single test under a debugger or an EXE project in Visual Studio. I can debug as native/mixed/managed and know exactly where to set a break point without having to provide a long list of arguments to the runner to satisfy hidden attributes checks - see below about XUnit.

3. How are environment variables that control the runtime employed? The COMPlus_GCStress and COMPlus_JITStress features are incredible expensive. How do other variables respond to this workflow.

4. I dislike XUnit personally. I think it is overly complex and slow for what is needed in most scenario based tests like what we have in CoreCLR. It works great for the libraries unit testing style but making it work nicely with CoreCLR tests seems difficult - especially debugging the native side of the runtime. It is already partially included in CoreCLR, but my inner dev loop doesn't have any XUnit parts and and I would prefer that. Also, rumor has it we are looking into using dotnet test for CoreCLR would that affect this proposal?

[jkotas]

For each test populate a ClrTestCaseAttribute

It may be nice to use the same traits used by libraries where possible. For example:

[ActiveIssue("https://github.com/dotnet/runtime/issues/26798")]
[PlatformSpecific(TestPlatforms.Windows)]

The simple consumer would simply treat the test cases as [Theory] data

I would expect each test to be one [Fact] method. What's the reason for it to be theory data?

Would all tests be run in the same test process?

It would be nice to plan for having an option to run tests that can run in a single process (majority of tests) in a single process.

[AaronRobinsonMSFT]

Would all tests be run in the same test process?

It would be nice to plan for having an option to run tests that can run in a single process (majority of tests) in a single process.

I agree where possible. This does make debugging the test more complicated and that is something we should consider. I don't think we pay enough attention to the inner dev loop. I would like to see a scenario where we can run everything in a single process in the CI, but locally I should be to able to run a single test outside of VS without a sequence of command line arguments that is longer than my arm.

[jashook]

/cc @dotnet/jit-contrib

[jashook]

Would all tests be run in the same test process? This isn't possible for many interop scenarios because we pollute the environment which means previous tests could have poisoned the runtime and future tests would fail. The gist here is we would need to run each interop test in a separate process. The JIT probably has similar test constraints - @dotnet/jit-contrib.

To my knowledge this proposal still maintains each test running in its own process. @sdmaclea can confirm or not.

[jashook]

It would be nice to plan for having an option to run tests that can run in a single process (majority of tests) in a single process.

I would suggest this as future work, it is costly as almost all tests inherently assume somehow that they are running as a single application.

[jashook]

Also, rumor has it we are looking into using dotnet test for CoreCLR would that affect this proposal?

This proposal would most likely make transitioning to dotnet test easier.

[janvorli]

Running tests in single process would be difficult for the cases when we run with GC stress on. That would mean we would have to run the xunit with GC stress enabled too.
As for inner dev loop, I find the cmd / sh scripts and one process per test ideal. When debugging runtime issues, it is much easier to debug when xunit doesn't get into my way. Xunit runs so much code before getting to the test itself. And it adds a lot of stuff to managed heap / stack so finding stuff there gets complicated.
It is also important to keep in mind that when debugging an issue in coreclr tests, in majority of the cases we are debugging issues in the native runtime, so WinDbg is the only reasonable tool on Windows. So I am not quite convinced that enabling running those tests in VS (which I guess is the main motivation for the dotnet test) has much of a value.

[jkotas]

Running tests in single process would be difficult for the cases when we run with GC stress on. That would mean we would have to run the xunit with GC stress enabled too.

Not necessarily. We can use https://github.com/dotnet/runtime/tree/master/src/libraries/Common/tests/StaticTestGenerator or something alike. (FWIW, .NET Native used scheme like this for codegen and low-level runtime tests.)

XUnit is really two parts:

• Standard for authoring tests and test metadata. I believe that it is worth sticking to this standard where possible.
• Runners that actually run the tests. The default dynamic xunit runners (including vstest) are problematic for runtime testing for reasons you have mentioned. I believe that we should stay away from those.

[sdmaclea]

Would all tests be run in the same test process?

It would be nice to plan for having an option to run tests that can run in a single process

I would suggest this as future work,

I hadn't considered running the tests in the same process.

I am happy to keep it as a background future process thought

[sdmaclea]

It may be nice to use the same traits used by libraries where possible

I'll have to look more carefully at those and see if they fully meets our needs. IT is likely we will need a superset.

Since I was thinking the MSBuild would construct the attributes, I was trying to keep my life simple.

But perhaps the simple thing would be to write the csproj XML like

<CLRTestAttributes><![CDATA[
    [ActiveIssue("https://github.com/dotnet/runtime/issues/26798")]
    [PlatformSpecific(TestPlatforms.Windows)]
]]><CLRTestAttributes/>

And get the best of both worlds

[sdmaclea]

What's the reason for it to be theory data?

I wanted a simple list of tests with the attributes for each test.
[Assembly:Attribute()] with reflection was a cute way to do it.

I am thinking I need a tool which is somewhat the opposite of StaticTestGenerator. Extract the test metadata from the .*proj files directly or indirectly.

• directly - with an XML parser
• indirectly - MSBuild generate intermediate json snippets.
  Then write the script files and/or the XUnit.net metadata files.

[naricc]

I'd like to add that, since we are now drawing on the same set of test for multiple runtimes, we should preserve the ability to disable tests based on RuntimeFlavor.

[sdmaclea]

One of the other questions is whether to

• centralize the test properties in a single file(s) like issues.targets
• distribute the test properties in each test in *.csproj files
• or a mixture like we have now.

The mixture makes adding new Targets and RuntimePlatfoms easier, but it comes with added complexity cost.