From 869211f004a4be27910faaf3a128c7c10e820f7c Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Fri, 4 Nov 2022 15:59:06 -0700 Subject: [PATCH] Improve coreclr test infra These fixes were built for PR #74886; however, as that PR is so utterly large an unreviewable, I've pulled out the test infra changes for separate review Changes - Increase the number of trampolines in the llvm aot compilation process to 20,000 from 10,000 (This avoids running out of them in some of the hardware intrinsics tests - Add a concept of striping tests when running under GC stress - To use this new feature, specify N within the merged test assembly. If this value is set, then the tests within that merged test assembly will be run across N different work items instead of 1 when running under any form of GC stress based scenario. At this moment the largest supported value of N is 99 - Emit the testresults.xml file as a file which is exported from the tests. This is useful for debugging testresult.xml parsing failures - Fix the testresults summary generator to never emit an empty CDATA string. If one is present the parser may fail the parse. - In the XUnitWrapperGenerator fix the implementation of the Outerloop and ActiveIssue when used with a conditional member. - Add PlatformDetection.IsMonoLLVMAOT, PlatformDetection.IsMonoLLVMFULLAOT, and PlatformDetection.IsMonoInterpreter boolean properties to the PlatformDetection type for use with the ActiveIssue attribute --- .../CoreCLRTestLibrary/PlatformDetection.cs | 6 + .../XUnitWrapperGenerator.cs | 75 +++++++-- .../Common/XUnitWrapperLibrary/TestFilter.cs | 59 +++++++- .../Common/XUnitWrapperLibrary/TestSummary.cs | 22 ++- src/tests/Common/helixpublishwitharcade.proj | 60 +++++++- src/tests/Common/mergedrunner.targets | 2 +- src/tests/Common/testenvironment.proj | 4 + src/tests/Directory.Build.targets | 142 ++++++++++++++++-- src/tests/build.proj | 2 +- 9 files changed, 333 insertions(+), 39 deletions(-) diff --git a/src/tests/Common/CoreCLRTestLibrary/PlatformDetection.cs b/src/tests/Common/CoreCLRTestLibrary/PlatformDetection.cs index 6916c7b6e6881..ababd3c898759 100644 --- a/src/tests/Common/CoreCLRTestLibrary/PlatformDetection.cs +++ b/src/tests/Common/CoreCLRTestLibrary/PlatformDetection.cs @@ -20,5 +20,11 @@ public static class PlatformDetection && (AppContext.TryGetSwitch("System.Runtime.InteropServices.BuiltInComInterop.IsSupported", out bool isEnabled) ? isEnabled : true); + + static string _variant = Environment.GetEnvironmentVariable("DOTNET_RUNTIME_VARIANT"); + + public static bool IsMonoLLVMAOT => _variant == "llvmaot"; + public static bool IsMonoLLVMFULLAOT => _variant == "llvmfullaot"; + public static bool IsMonoInterpreter => _variant == "monointerpreter"; } } diff --git a/src/tests/Common/XUnitWrapperGenerator/XUnitWrapperGenerator.cs b/src/tests/Common/XUnitWrapperGenerator/XUnitWrapperGenerator.cs index b71b554f37ca1..adcfa65fabe09 100644 --- a/src/tests/Common/XUnitWrapperGenerator/XUnitWrapperGenerator.cs +++ b/src/tests/Common/XUnitWrapperGenerator/XUnitWrapperGenerator.cs @@ -154,7 +154,7 @@ private static string GenerateFullTestRunner(ImmutableArray testInfos builder.AppendLine(string.Join("\n", aliasMap.Values.Where(alias => alias != "global").Select(alias => $"extern alias {alias};"))); builder.AppendLine("System.Collections.Generic.HashSet testExclusionList = XUnitWrapperLibrary.TestFilter.LoadTestExclusionList();"); - builder.AppendLine("XUnitWrapperLibrary.TestFilter filter = new (args.Length != 0 ? args[0] : null, testExclusionList);"); + builder.AppendLine("XUnitWrapperLibrary.TestFilter filter = new (args, testExclusionList);"); builder.AppendLine("XUnitWrapperLibrary.TestSummary summary = new();"); builder.AppendLine("System.Diagnostics.Stopwatch stopwatch = System.Diagnostics.Stopwatch.StartNew();"); builder.AppendLine("XUnitWrapperLibrary.TestOutputRecorder outputRecorder = new(System.Console.Out);"); @@ -162,14 +162,43 @@ private static string GenerateFullTestRunner(ImmutableArray testInfos ITestReporterWrapper reporter = new WrapperLibraryTestSummaryReporting("summary", "filter", "outputRecorder"); - foreach (ITestInfo test in testInfos) + StringBuilder testExecutorBuilder = new(); + int testsLeftInCurrentTestExecutor = 0; + int currentTestExecutor = 0; + int totalTestsEmitted = 0; + + if (testInfos.Length > 0) { - builder.AppendLine(test.GenerateTestExecution(reporter)); + // Break tests into groups of 50 so that we don't create an unreasonably large main method + // Excessively large methods are known to take a long time to compile, and use excessive stack + // leading to test failures. + foreach (ITestInfo test in testInfos) + { + if (testsLeftInCurrentTestExecutor == 0) + { + if (currentTestExecutor != 0) + testExecutorBuilder.AppendLine("}"); + currentTestExecutor++; + testExecutorBuilder.AppendLine($"void TestExecutor{currentTestExecutor}(){{"); + builder.AppendLine($"TestExecutor{currentTestExecutor}();"); + testsLeftInCurrentTestExecutor = 50; // Break test executors into groups of 50, which empircally seems to work well + } + testExecutorBuilder.AppendLine(test.GenerateTestExecution(reporter)); + totalTestsEmitted++; + testsLeftInCurrentTestExecutor--; + } + testExecutorBuilder.AppendLine("}"); } - builder.AppendLine($@"System.IO.File.WriteAllText(""{assemblyName}.testResults.xml"", summary.GetTestResultOutput(""{assemblyName}""));"); + builder.AppendLine($@"string testResults = summary.GetTestResultOutput(""{assemblyName}"");"); + builder.AppendLine($@"string workitemUploadRoot = System.Environment.GetEnvironmentVariable(""HELIX_WORKITEM_UPLOAD_ROOT"");"); + builder.AppendLine($@"if (workitemUploadRoot != null) System.IO.File.WriteAllText(System.IO.Path.Combine(workitemUploadRoot, ""{assemblyName}.testResults.xml.txt""), testResults);"); + builder.AppendLine($@"System.IO.File.WriteAllText(""{assemblyName}.testResults.xml"", testResults);"); builder.AppendLine("return 100;"); + builder.Append(testExecutorBuilder); + + builder.AppendLine("public static class TestCount { public const int Count = " + totalTestsEmitted.ToString() + "; }"); return builder.ToString(); } @@ -193,14 +222,37 @@ private static string GenerateXHarnessTestRunner(ImmutableArray testI ITestReporterWrapper reporter = new WrapperLibraryTestSummaryReporting("summary", "filter", "outputRecorder"); - foreach (ITestInfo test in testInfos) + StringBuilder testExecutorBuilder = new(); + int testsLeftInCurrentTestExecutor = 0; + int currentTestExecutor = 0; + + if (testInfos.Length > 0) { - builder.AppendLine(test.GenerateTestExecution(reporter)); + // Break tests into groups of 50 so that we don't create an unreasonably large main method + // Excessively large methods are known to take a long time to compile, and use excessive stack + // leading to test failures. + foreach (ITestInfo test in testInfos) + { + if (testsLeftInCurrentTestExecutor == 0) + { + if (currentTestExecutor != 0) + testExecutorBuilder.AppendLine("}"); + currentTestExecutor++; + testExecutorBuilder.AppendLine($"static void TestExecutor{currentTestExecutor}(XUnitWrapperLibrary.TestSummary summary, XUnitWrapperLibrary.TestFilter filter, XUnitWrapperLibrary.TestOutputRecorder outputRecorder, System.Diagnostics.Stopwatch stopwatch){{"); + builder.AppendLine($"TestExecutor{currentTestExecutor}(summary, filter, outputRecorder, stopwatch);"); + testsLeftInCurrentTestExecutor = 50; // Break test executors into groups of 50, which empirically seems to work well + } + testExecutorBuilder.AppendLine(test.GenerateTestExecution(reporter)); + testsLeftInCurrentTestExecutor--; + } + testExecutorBuilder.AppendLine("}"); } builder.AppendLine("return summary;"); builder.AppendLine("}"); + builder.Append(testExecutorBuilder); + return builder.ToString(); } @@ -354,7 +406,8 @@ private static IEnumerable GetTestMethodInfosForMethod(IMethodSymbol testInfos, conditionType, conditionMembers, - aliasMap[conditionType.ContainingAssembly.MetadataName]); + aliasMap[conditionType.ContainingAssembly.MetadataName], + false /* do not negate the condition, as this attribute indicates that a test will be run */); break; } case "Xunit.OuterloopAttribute": @@ -373,7 +426,8 @@ private static IEnumerable GetTestMethodInfosForMethod(IMethodSymbol testInfos, conditionType, filterAttribute.ConstructorArguments[2].Values, - aliasMap[conditionType.ContainingAssembly.MetadataName]); + aliasMap[conditionType.ContainingAssembly.MetadataName], + true /* negate the condition, as this attribute indicates that a test will NOT be run */); break; } else if (filterAttribute.AttributeConstructor.Parameters.Length == 4) @@ -667,9 +721,12 @@ private static ImmutableArray DecorateWithUserDefinedCondition( ImmutableArray testInfos, ITypeSymbol conditionType, ImmutableArray values, - string externAlias) + string externAlias, + bool negate) { string condition = string.Join("&&", values.Select(v => $"{externAlias}::{conditionType.ToDisplayString(FullyQualifiedWithoutGlobalNamespace)}.{v.Value}")); + if (negate) + condition = $"!({condition})"; return ImmutableArray.CreateRange(testInfos.Select(m => new ConditionalTest(m, condition))); } diff --git a/src/tests/Common/XUnitWrapperLibrary/TestFilter.cs b/src/tests/Common/XUnitWrapperLibrary/TestFilter.cs index 11523964ba491..4e37d64a368b2 100644 --- a/src/tests/Common/XUnitWrapperLibrary/TestFilter.cs +++ b/src/tests/Common/XUnitWrapperLibrary/TestFilter.cs @@ -118,14 +118,50 @@ public override string ToString() // issues.targets file as a split model would be very confusing for developers // and test monitors. private readonly HashSet? _testExclusionList; + private readonly int _stripe = 0; + private readonly int _stripeCount = 1; + private int _shouldRunQuery = -1; - public TestFilter(string? filterString, HashSet? testExclusionList) + public TestFilter(string? filterString, HashSet? testExclusionList) : + this(filterString == null ? Array.Empty() : new string[]{filterString}, testExclusionList) { + } + + public TestFilter(string[] filterArgs, HashSet? testExclusionList) + { + string? filterString = null; + + for (int i = 0; i < filterArgs.Length; i++) + { + if (filterArgs[i].StartsWith("-stripe")) + { + _stripe = Int32.Parse(filterArgs[++i]); + _stripeCount = Int32.Parse(filterArgs[++i]); + } + else + { + if (filterString == null) + filterString = filterArgs[0]; + } + } + + var stripeEnvironment = Environment.GetEnvironmentVariable("TEST_HARNESS_STRIPE_TO_EXECUTE"); + if (!String.IsNullOrEmpty(stripeEnvironment) && stripeEnvironment != ".0.1") + { + var stripes = stripeEnvironment.Split('.'); + if (stripes.Length == 3) + { + Console.WriteLine($"Test striping enabled via TEST_HARNESS_STRIPE_TO_EXECUTE environment variable set to '{stripeEnvironment}'"); + _stripe = Int32.Parse(stripes[1]); + _stripeCount = Int32.Parse(stripes[2]); + } + } + if (filterString is not null) { if (filterString.IndexOfAny(new[] { '!', '(', ')', '~', '=' }) != -1) { - throw new ArgumentException("Complex test filter expressions are not supported today. The only filters supported today are the simple form supported in 'dotnet test --filter' (substrings of the test's fully qualified name). If further filtering options are desired, file an issue on dotnet/runtime for support.", nameof(filterString)); + throw new ArgumentException("Complex test filter expressions ar e not supported today. The only filters supported today are the simple form supported in 'dotnet test --filter' (substrings of the test's fully qualified name). If further filtering options are desired, file an issue on dotnet/runtime for support.", nameof(filterString)); } _filter = new NameClause(TermKind.FullyQualifiedName, filterString, substring: true); } @@ -140,15 +176,26 @@ public TestFilter(ISearchClause? filter, HashSet? testExclusionList) public bool ShouldRunTest(string fullyQualifiedName, string displayName, string[]? traits = null) { + bool shouldRun = false; if (_testExclusionList is not null && _testExclusionList.Contains(displayName.Replace("\\", "/"))) { - return false; + shouldRun = false; } - if (_filter is null) + else if (_filter is null) + { + shouldRun = true; + } + else + { + shouldRun = _filter.IsMatch(fullyQualifiedName, displayName, traits ?? Array.Empty()); + } + + if (shouldRun) { - return true; + // Test stripe, if true, then report success + return ((System.Threading.Interlocked.Increment(ref _shouldRunQuery)) % _stripeCount) == _stripe; } - return _filter.IsMatch(fullyQualifiedName, displayName, traits ?? Array.Empty()); + return false; } public static HashSet LoadTestExclusionList() diff --git a/src/tests/Common/XUnitWrapperLibrary/TestSummary.cs b/src/tests/Common/XUnitWrapperLibrary/TestSummary.cs index 0de2af1ec838b..b230fa485a055 100644 --- a/src/tests/Common/XUnitWrapperLibrary/TestSummary.cs +++ b/src/tests/Common/XUnitWrapperLibrary/TestSummary.cs @@ -73,11 +73,29 @@ public string GetTestResultOutput(string assemblyName) string outputElement = !string.IsNullOrWhiteSpace(test.Output) ? $"" : string.Empty; if (test.Exception is not null) { - resultsFile.AppendLine($@"result=""Fail"">{outputElement}"); + string exceptionMessage = test.Exception.Message; + if (test.Exception is System.Reflection.TargetInvocationException tie) + { + if (tie.InnerException != null) + { + exceptionMessage = $"{exceptionMessage} \n INNER EXCEPTION--\n {tie.InnerException.GetType()}--\n{tie.InnerException.Message}--\n{tie.InnerException.StackTrace}"; + } + } + if (string.IsNullOrWhiteSpace(exceptionMessage)) + { + exceptionMessage = "NoExceptionMessage"; + } + + string? stackTrace = test.Exception.StackTrace; + if (string.IsNullOrWhiteSpace(stackTrace)) + { + stackTrace = "NoStackTrace"; + } + resultsFile.AppendLine($@"result=""Fail"">{outputElement}"); } else if (test.SkipReason is not null) { - resultsFile.AppendLine($@"result=""Skip"">"); + resultsFile.AppendLine($@"result=""Skip"">"); } else { diff --git a/src/tests/Common/helixpublishwitharcade.proj b/src/tests/Common/helixpublishwitharcade.proj index 885d346434756..57d12bab0134e 100644 --- a/src/tests/Common/helixpublishwitharcade.proj +++ b/src/tests/Common/helixpublishwitharcade.proj @@ -391,6 +391,8 @@ + + @@ -652,6 +654,8 @@ + + @@ -738,6 +742,55 @@ + + + true + + false + + + + + <_MergedStressWrapperMarker Condition="'$(StripeMergedStressWorkItems)'=='true'" Include="$(TestBinDir)**\*.MergedTestAssemblyForStress"/> + + + + + + %(_MergedStressWrapperMarker.Filename) + $([System.IO.Path]::GetExtension('$(WithoutFinalExt)')) + $([System.IO.Path]::GetFilenameWithoutExtension('$(WithoutFinalExt)')) + $([System.IO.Path]::GetExtension('$(WithoutFinalExt2)')) + $([System.IO.Path]::GetFilenameWithoutExtension('$(WithoutFinalExt2)')) + $([System.IO.Path]::GetFilenameWithoutExtension('$(WithoutFinalExt3)')) + $(MergedWrapperPayloadGroup)$(ActiveStripe)$(TotalStripes) + %(_MergedStressWrapperMarker.Identity) + + + + %(MergedPayloads.PayloadDirectory) + $([System.String]::Copy('%(MergedPayloads.MergedTestHelixCommand)').Replace('TEST_HARNESS_STRIPE_TO_EXECUTE=.0.1','TEST_HARNESS_STRIPE_TO_EXECUTE=$(ActiveStripe)$(TotalStripes)')) + $([System.TimeSpan]::FromMinutes($(TimeoutPerTestCollectionInMinutes))) + coreclr_tests.run.$(TargetOS).$(TargetArchitecture).$(Configuration).mch;coreclr_tests.run.$(TargetOS).$(TargetArchitecture).$(Configuration).log + + + + + + + + %(MergedPayloads.PayloadDirectory) + %(MergedPayloads.MergedTestHelixCommand) + $([System.TimeSpan]::FromMinutes($(TimeoutPerTestCollectionInMinutes))) + coreclr_tests.run.$(TargetOS).$(TargetArchitecture).$(Configuration).mch;coreclr_tests.run.$(TargetOS).$(TargetArchitecture).$(Configuration).log + + + + %(PayloadDirectory) @@ -747,13 +800,6 @@ coreclr_tests.run.$(TargetOS).$(TargetArchitecture).$(Configuration).mch;coreclr_tests.run.$(TargetOS).$(TargetArchitecture).$(Configuration).log - - %(PayloadDirectory) - %(MergedTestHelixCommand) - $([System.TimeSpan]::FromMinutes($(TimeoutPerTestCollectionInMinutes))) - coreclr_tests.run.$(TargetOS).$(TargetArchitecture).$(Configuration).mch;coreclr_tests.run.$(TargetOS).$(TargetArchitecture).$(Configuration).log - - $([System.TimeSpan]::FromMinutes($(TimeoutPerTestCollectionInMinutes))) dotnet $(XUnitRunnerDll) %(XUnitWrapperDlls) $(XUnitRunnerArgs) diff --git a/src/tests/Common/mergedrunner.targets b/src/tests/Common/mergedrunner.targets index 78ccb1dc5dc42..81a9def0117f1 100644 --- a/src/tests/Common/mergedrunner.targets +++ b/src/tests/Common/mergedrunner.targets @@ -15,7 +15,7 @@ Produce an error if any project references were removed due to conflict resolution. If a ProjectReference is removed due to conflict resolution, then we're likely losing test coverage as it's probably a test that has the same assembly name and version as another test. --> - + diff --git a/src/tests/Common/testenvironment.proj b/src/tests/Common/testenvironment.proj index 27e14c6c3194c..43a05f487f09c 100644 --- a/src/tests/Common/testenvironment.proj +++ b/src/tests/Common/testenvironment.proj @@ -272,6 +272,8 @@ <_TestEnvFileLine Condition="'$(RuntimeVariant)' == 'monointerpreter'" Include="set MONO_ENV_OPTIONS=--interpreter" /> + <_TestEnvFileLine Condition="'$(RuntimeVariant)' != ''" Include="set DOTNET_RUNTIME_VARIANT=$(RuntimeVariant)" /> + <_TestEnvFileLine Condition="'$(Scenario)' == 'clrinterpreter'" Include="set DOTNET_Interpret=%2A" /> <_TestEnvFileLine Condition="'$(Scenario)' == 'clrinterpreter'" Include="set DOTNET_InterpreterHWIntrinsicsIsSupportedFalse=1" /> @@ -294,6 +296,8 @@ <_TestEnvFileLine Condition="'$(RuntimeVariant)' == 'llvmfullaot'" Include="export MONO_ENV_OPTIONS=--full-aot" /> + <_TestEnvFileLine Condition="'$(RuntimeVariant)' != ''" Include="export DOTNET_RUNTIME_VARIANT=$(RuntimeVariant)" /> + <_TestEnvFileLine Condition="'$(Scenario)' == 'clrinterpreter'" Include="export DOTNET_Interpret=%2A" /> <_TestEnvFileLine Condition="'$(Scenario)' == 'clrinterpreter'" Include="export DOTNET_InterpreterHWIntrinsicsIsSupportedFalse=1" /> diff --git a/src/tests/Directory.Build.targets b/src/tests/Directory.Build.targets index 369abf945a373..815d023fe212b 100644 --- a/src/tests/Directory.Build.targets +++ b/src/tests/Directory.Build.targets @@ -271,11 +271,135 @@ + + + $([System.String]::new(';',10)) + + + + + 1 + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + MergedStressTestAssembly + + + + - - + + MergedTestAssembly + + + NoMonoAot + @@ -284,20 +408,12 @@ Condition="Exists('$(AssemblyName).reflect.xml')"/> - - - - +