Allow dumps to be captured for SDK integration tests (dotnet#27094)

pranavkm · web-flow · commit d5db6dac98c9 · 2020-10-21T14:45:11.000-07:00
* Allow dumps to be captured for SDK integration tests

SDK tests are currently not running on Helix. There is ongoing work to allow these to be run on Helix.
In the meanwhile, we have a bunch of tests that are quarantined, succeeding for 30 days, but cannot be turned
on because it's unclear why they hang.

This PR changes the tests to use `dotnet test` for their runs and uses the cool new features to enforce test hangs
and capture dumps.

* Unquarantine passing tests
diff --git a/eng/QuarantinedTests.BeforeArcade.props b/eng/QuarantinedTests.BeforeArcade.props
@@ -1,9 +1,14 @@
 <Project>
-  <PropertyGroup>
+  <PropertyGroup Condition="'$(UseVSTestRunner)' != 'true'">
     <_QuarantinedTestRunAdditionalArgs>-trait "Quarantined=true"</_QuarantinedTestRunAdditionalArgs>
     <_NonQuarantinedTestRunAdditionalArgs>-notrait "Quarantined=true"</_NonQuarantinedTestRunAdditionalArgs>
   </PropertyGroup>
 
+  <PropertyGroup Condition="'$(UseVSTestRunner)' == 'true'">
+    <_QuarantinedTestRunAdditionalArgs>--filter Quarantined=true</_QuarantinedTestRunAdditionalArgs>
+    <_NonQuarantinedTestRunAdditionalArgs>--filter Quarantined!=true</_NonQuarantinedTestRunAdditionalArgs>
+  </PropertyGroup>
+
   <PropertyGroup>
     <TestRunnerAdditionalArguments Condition="'$(RunQuarantinedTests)' == ''">$(_NonQuarantinedTestRunAdditionalArgs) $(TestRunnerAdditionalArguments)</TestRunnerAdditionalArguments>
     <TestRunnerAdditionalArguments Condition="'$(RunQuarantinedTests)' == 'true'">$(_QuarantinedTestRunAdditionalArgs) $(TestRunnerAdditionalArguments)</TestRunnerAdditionalArguments>
diff --git a/eng/Workarounds.props b/eng/Workarounds.props
@@ -13,7 +13,7 @@
     <PublishWindowsPdb>false</PublishWindowsPdb>
   </PropertyGroup>
 
-  <PropertyGroup>
+  <PropertyGroup Condition="'$(UseVSTestRunner)' != 'true'">
     <!-- Xunit output is piped into a file anyways, so color codes aren't useful. -->
     <TestRunnerAdditionalArguments>$(TestRunnerAdditionalArguments) -nocolor</TestRunnerAdditionalArguments>
   </PropertyGroup>
diff --git a/src/Components/WebAssembly/Sdk/integrationtests/Directory.Build.props b/src/Components/WebAssembly/Sdk/integrationtests/Directory.Build.props
@@ -0,0 +1,9 @@
+<Project>
+  <!-- UseVSTestRunner must be configured before $(RepoRoot)\Directory.Build.props is included. -->
+  <PropertyGroup>
+    <UseVSTestRunner>true</UseVSTestRunner>
+    <TestRunnerAdditionalArguments>$(TestRunnerAdditionalArguments) --blame "CollectHangDump;TestTimeout=6m"</TestRunnerAdditionalArguments>
+  </PropertyGroup>
+
+  <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory)..\, Directory.Build.props))\Directory.Build.props" />
+</Project>
diff --git a/src/Components/WebAssembly/Sdk/integrationtests/WasmBuildIntegrationTest.cs b/src/Components/WebAssembly/Sdk/integrationtests/WasmBuildIntegrationTest.cs
@@ -287,7 +287,6 @@ public async Task Build_Hosted_Works()
         }
 
         [Fact]
-        [QuarantinedTest]
         public async Task Build_SatelliteAssembliesAreCopiedToBuildOutput()
         {
             // Arrange
diff --git a/src/Components/WebAssembly/Sdk/integrationtests/WasmBuildLazyLoadTest.cs b/src/Components/WebAssembly/Sdk/integrationtests/WasmBuildLazyLoadTest.cs
@@ -89,7 +89,6 @@ public async Task Build_LazyLoadExplicitAssembly_Release_Works()
         }
 
         [Fact]
-        [QuarantinedTest("https://github.com/dotnet/aspnetcore/issues/23756")]
         public async Task Publish_LazyLoadExplicitAssembly_Debug_Works()
         {
             // Arrange
diff --git a/src/Components/WebAssembly/Sdk/integrationtests/WasmCompressionTests.cs b/src/Components/WebAssembly/Sdk/integrationtests/WasmCompressionTests.cs
@@ -11,7 +11,6 @@ namespace Microsoft.AspNetCore.Razor.Design.IntegrationTests
     public class WasmCompressionTests
     {
         [Fact]
-        [QuarantinedTest("https://github.com/dotnet/aspnetcore/issues/23397")]
         public async Task Publish_UpdatesFilesWhenSourcesChange()
         {
             // Arrange
@@ -51,7 +50,6 @@ public async Task Publish_UpdatesFilesWhenSourcesChange()
         }
 
         [Fact]
-        [QuarantinedTest("https://github.com/dotnet/aspnetcore/issues/23397")]
         public async Task Publish_WithoutLinkerAndCompression_UpdatesFilesWhenSourcesChange()
         {
             // Arrange
@@ -82,7 +80,6 @@ public async Task Publish_WithoutLinkerAndCompression_UpdatesFilesWhenSourcesCha
         }
 
         [Fact]
-        [QuarantinedTest("https://github.com/dotnet/aspnetcore/issues/23756")]
         public async Task Publish_WithLinkerAndCompression_IsIncremental()
         {
             // Arrange
diff --git a/src/Components/WebAssembly/Sdk/integrationtests/WasmPublishIntegrationTest.cs b/src/Components/WebAssembly/Sdk/integrationtests/WasmPublishIntegrationTest.cs
@@ -146,7 +146,6 @@ public async Task Publish_WithScopedCss_Works()
         }
 
         [Fact]
-        [QuarantinedTest("https://github.com/dotnet/aspnetcore/issues/23756")]
         public async Task Publish_InRelease_Works()
         {
             // Arrange
@@ -314,7 +313,6 @@ public async Task Publish_WithTrimmingdDisabled_Works()
         }
 
         [Fact]
-        [QuarantinedTest("https://github.com/dotnet/aspnetcore/issues/23756")]
         public async Task Publish_SatelliteAssemblies_AreCopiedToBuildOutput()
         {
             // Arrange
@@ -348,7 +346,6 @@ public async Task Publish_SatelliteAssemblies_AreCopiedToBuildOutput()
         }
 
         [Fact]
-        [QuarantinedTest]
         public async Task Publish_HostedApp_DefaultSettings_Works()
         {
             // Arrange
@@ -410,7 +407,6 @@ public async Task Publish_HostedApp_DefaultSettings_Works()
         }
 
         [Fact]
-        [QuarantinedTest("https://github.com/dotnet/aspnetcore/issues/23756")]
         public async Task Publish_HostedApp_ProducesBootJsonDataWithExpectedContent()
         {
             // Arrange
diff --git a/src/Razor/Microsoft.NET.Sdk.Razor/integrationtests/Directory.Build.props b/src/Razor/Microsoft.NET.Sdk.Razor/integrationtests/Directory.Build.props
@@ -0,0 +1,9 @@
+<Project>
+  <!-- UseVSTestRunner must be configured before the $(RepoRoot)\Directory.Build.props is included. -->
+  <PropertyGroup>
+    <UseVSTestRunner>true</UseVSTestRunner>
+    <TestRunnerAdditionalArguments>$(TestRunnerAdditionalArguments) --blame "CollectHangDump;TestTimeout=6m"</TestRunnerAdditionalArguments>
+  </PropertyGroup>
+
+  <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory)..\, Directory.Build.props))\Directory.Build.props" />
+</Project>
diff --git a/src/Razor/Microsoft.NET.Sdk.Razor/integrationtests/Microsoft.NET.Sdk.Razor.IntegrationTests.csproj b/src/Razor/Microsoft.NET.Sdk.Razor/integrationtests/Microsoft.NET.Sdk.Razor.IntegrationTests.csproj
@@ -1,4 +1,4 @@
-<Project Sdk="Microsoft.NET.Sdk">
+﻿<Project Sdk="Microsoft.NET.Sdk">
 
   <PropertyGroup>
     <!--
diff --git a/src/Razor/Microsoft.NET.Sdk.Razor/integrationtests/MvcBuildIntegrationTest21.cs b/src/Razor/Microsoft.NET.Sdk.Razor/integrationtests/MvcBuildIntegrationTest21.cs
@@ -3,7 +3,6 @@
 
 using System.IO;
 using System.Threading.Tasks;
-using Microsoft.AspNetCore.Testing;
 using Xunit;
 
 namespace Microsoft.AspNetCore.Razor.Design.IntegrationTests
@@ -19,7 +18,6 @@ public MvcBuildIntegrationTest21(LegacyBuildServerTestFixture buildServer)
         public override string TargetFramework => "netcoreapp2.1";
 
         [Fact]
-        [QuarantinedTest("https://github.com/dotnet/aspnetcore/issues/25042")]
         public async Task Building_WorksWhenMultipleRazorConfigurationsArePresent()
         {
             using (var project = CreateTestProject())
diff --git a/src/Razor/Microsoft.NET.Sdk.Razor/integrationtests/MvcBuildIntegrationTestLegacy.cs b/src/Razor/Microsoft.NET.Sdk.Razor/integrationtests/MvcBuildIntegrationTestLegacy.cs
@@ -1,4 +1,4 @@
-﻿// Copyright (c) .NET Foundation. All rights reserved.
+// Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
@@ -31,7 +31,6 @@ protected IDisposable CreateTestProject()
         }
 
         [Fact]
-        [QuarantinedTest]
         public virtual async Task Building_Project()
         {
             using (CreateTestProject())
@@ -56,7 +55,6 @@ public virtual async Task Building_Project()
         }
 
         [Fact]
-        [QuarantinedTest("https://github.com/dotnet/aspnetcore/issues/23635")]
         public virtual async Task BuildingProject_CopyToOutputDirectoryFiles()
         {
             using (CreateTestProject())
@@ -74,7 +72,6 @@ public virtual async Task BuildingProject_CopyToOutputDirectoryFiles()
         }
 
         [Fact]
-        [QuarantinedTest("https://github.com/dotnet/aspnetcore/issues/25931")]
         public virtual async Task Publish_Project()
         {
             using (CreateTestProject())
@@ -126,7 +123,6 @@ public async Task PublishingProject_CopyToPublishDirectoryItems()
         }
 
         [Fact]
-        [QuarantinedTest("https://github.com/dotnet/aspnetcore/issues/23756")]
         public virtual async Task Publish_IncludesRefAssemblies_WhenCopyRefAssembliesToPublishDirectoryIsSet()
         {
             using (CreateTestProject())
diff --git a/src/Razor/Microsoft.NET.Sdk.Razor/integrationtests/StaticWebAssetsIntegrationTest.cs b/src/Razor/Microsoft.NET.Sdk.Razor/integrationtests/StaticWebAssetsIntegrationTest.cs
@@ -57,7 +57,6 @@ public async Task Build_GeneratesStaticWebAssetsManifest_Success_CreatesManifest
         }
 
         [Fact]
-        [QuarantinedTest("https://github.com/dotnet/aspnetcore/issues/22049")]
         [InitializeTestProject("AppWithPackageAndP2PReference", additionalProjects: new[] { "ClassLibrary", "ClassLibrary2" })]
         public async Task Publish_CopiesStaticWebAssetsToDestinationFolder()
         {
@@ -84,7 +83,6 @@ public async Task Publish_CopiesStaticWebAssetsToDestinationFolder()
         }
 
         [ConditionalFact]
-        [QuarantinedTest("https://github.com/dotnet/aspnetcore/issues/18543")]
         [OSSkipCondition(OperatingSystems.Linux | OperatingSystems.MacOSX)]
         [InitializeTestProject("AppWithPackageAndP2PReferenceAndRID", additionalProjects: new[] { "ClassLibrary", "ClassLibrary2" })]
         public async Task Publish_CopiesStaticWebAssetsToDestinationFolder_PublishSingleFile()
@@ -112,7 +110,6 @@ public async Task Publish_CopiesStaticWebAssetsToDestinationFolder_PublishSingle
         }
 
         [Fact]
-        [QuarantinedTest]
         [InitializeTestProject("AppWithPackageAndP2PReference", additionalProjects: new[] { "ClassLibrary", "ClassLibrary2" })]
         public async Task Publish_WithBuildReferencesDisabled_CopiesStaticWebAssetsToDestinationFolder()
         {
@@ -136,7 +133,6 @@ public async Task Publish_WithBuildReferencesDisabled_CopiesStaticWebAssetsToDes
         }
 
         [Fact]
-        [QuarantinedTest("https://github.com/dotnet/aspnetcore/issues/22049")]
         [InitializeTestProject("AppWithPackageAndP2PReference", additionalProjects: new[] { "ClassLibrary", "ClassLibrary2" })]
         public async Task Publish_NoBuild_CopiesStaticWebAssetsToDestinationFolder()
         {
@@ -188,7 +184,6 @@ public async Task Build_Fails_WhenConflictingAssetsFoundBetweenAStaticWebAssetAn
         }
 
         [Fact]
-        [QuarantinedTest("https://github.com/dotnet/aspnetcore/issues/23756")]
         [InitializeTestProject("AppWithPackageAndP2PReference", language: "C#", additionalProjects: new[] { "ClassLibrary", "ClassLibrary2" })]
         public async Task Clean_Success_RemovesManifestAndCache()
         {
diff --git a/src/Shared/MSBuild.Testing/MSBuildProcessManager.cs b/src/Shared/MSBuild.Testing/MSBuildProcessManager.cs
@@ -212,109 +212,12 @@ async Task<ProcessResult> GetTimeoutForProcess(Process process, TimeSpan? timeou
                 }
                 if (!process.HasExited)
                 {
-                    await CollectDumps(process, timeout, diagnostics);
-
                     // This is a timeout.
                     process.Kill();
                 }
 
                 throw new TimeoutException($"command '${process.StartInfo.FileName} {process.StartInfo.Arguments}' timed out after {timeout}. Output: {output.ToString()}");
             }
-
-            static async Task CollectDumps(Process process, TimeSpan? timeout, StringBuilder diagnostics)
-            {
-                var procDumpProcess = await CaptureDump(process, timeout, diagnostics);
-                var allDotNetProcesses = Process.GetProcessesByName("dotnet");
-
-                var allDotNetChildProcessCandidates = allDotNetProcesses
-                    .Where(p => p.StartTime >= process.StartTime && p.Id != process.Id);
-
-                if (!allDotNetChildProcessCandidates.Any())
-                {
-                    diagnostics.AppendLine("Couldn't find any candidate child process.");
-                    foreach (var dotnetProcess in allDotNetProcesses)
-                    {
-                        diagnostics.AppendLine($"Found dotnet process with PID {dotnetProcess.Id} and start time {dotnetProcess.StartTime}.");
-                    }
-                }
-
-                foreach (var childProcess in allDotNetChildProcessCandidates)
-                {
-                    diagnostics.AppendLine($"Found child process candidate '{childProcess.Id}'.");
-                }
-
-                var childrenProcessDumpProcesses = await Task.WhenAll(allDotNetChildProcessCandidates.Select(d => CaptureDump(d, timeout, diagnostics)));
-                foreach (var childProcess in childrenProcessDumpProcesses)
-                {
-                    if (childProcess != null && childProcess.HasExited)
-                    {
-                        diagnostics.AppendLine($"ProcDump failed to run for child dotnet process candidate '{process.Id}'.");
-                        childProcess.Kill();
-                    }
-                }
-
-                if (procDumpProcess != null && procDumpProcess.HasExited)
-                {
-                    diagnostics.AppendLine($"ProcDump failed to run for '{process.Id}'.");
-                    procDumpProcess.Kill();
-                }
-            }
-
-            static async Task<Process> CaptureDump(Process process, TimeSpan? timeout, StringBuilder diagnostics)
-            {
-                var metadataAttributes = Assembly.GetExecutingAssembly()
-                    .GetCustomAttributes<AssemblyMetadataAttribute>();
-
-                var procDumpPath = metadataAttributes
-                    .SingleOrDefault(ama => ama.Key == "ProcDumpToolPath")?.Value;
-
-                if (string.IsNullOrEmpty(procDumpPath))
-                {
-                    diagnostics.AppendLine("ProcDumpPath not defined.");
-                    return null;
-                }
-                var procDumpExePath = Path.Combine(procDumpPath, "procdump.exe");
-                if (!File.Exists(procDumpExePath))
-                {
-                    diagnostics.AppendLine($"Can't find procdump.exe in '{procDumpPath}'.");
-                    return null;
-                }
-
-                var dumpDirectory = metadataAttributes
-                    .SingleOrDefault(ama => ama.Key == "ArtifactsLogDir")?.Value;
-
-                if (string.IsNullOrEmpty(dumpDirectory))
-                {
-                    diagnostics.AppendLine("ArtifactsLogDir not defined.");
-                    return null;
-                }
-
-                if (!Directory.Exists(dumpDirectory))
-                {
-                    diagnostics.AppendLine($"'{dumpDirectory}' does not exist.");
-                    return null;
-                }
-
-                var procDumpPattern = Path.Combine(dumpDirectory, "HangingProcess_PROCESSNAME_PID_YYMMDD_HHMMSS.dmp");
-                var procDumpStartInfo = new ProcessStartInfo(
-                    procDumpExePath,
-                    $"-accepteula -ma {process.Id} {procDumpPattern}")
-                {
-                    CreateNoWindow = true,
-                    UseShellExecute = false,
-                    RedirectStandardOutput = true,
-                    RedirectStandardError = true
-                };
-
-                var procDumpProcess = Process.Start(procDumpStartInfo);
-                var tcs = new TaskCompletionSource<object>();
-
-                procDumpProcess.Exited += (s, a) => tcs.TrySetResult(null);
-                procDumpProcess.EnableRaisingEvents = true;
-
-                await Task.WhenAny(tcs.Task, Task.Delay(timeout ?? TimeSpan.FromSeconds(30)));
-                return procDumpProcess;
-            }
         }
 
         internal class ProcessResult

Original file line number	Diff line number	Diff line change
`@@ -287,7 +287,6 @@ public async Task Build_Hosted_Works()`
`287`	`287`	`}`
`288`	`288`
`289`	`289`	`[Fact]`
`290`		`- [QuarantinedTest]`
`291`	`290`	`public async Task Build_SatelliteAssembliesAreCopiedToBuildOutput()`
`292`	`291`	`{`
`293`	`292`	`// Arrange`
Original file line number	Diff line number	Diff line change
`@@ -89,7 +89,6 @@ public async Task Build_LazyLoadExplicitAssembly_Release_Works()`
`89`	`89`	`}`
`90`	`90`
`91`	`91`	`[Fact]`
`92`		`- [QuarantinedTest("https://github.com/dotnet/aspnetcore/issues/23756")]`
`93`	`92`	`public async Task Publish_LazyLoadExplicitAssembly_Debug_Works()`
`94`	`93`	`{`
`95`	`94`	`// Arrange`
Original file line number	Diff line number	Diff line change
`@@ -11,7 +11,6 @@ namespace Microsoft.AspNetCore.Razor.Design.IntegrationTests`
`11`	`11`	`public class WasmCompressionTests`
`12`	`12`	`{`
`13`	`13`	`[Fact]`
`14`		`- [QuarantinedTest("https://github.com/dotnet/aspnetcore/issues/23397")]`
`15`	`14`	`public async Task Publish_UpdatesFilesWhenSourcesChange()`
`16`	`15`	`{`
`17`	`16`	`// Arrange`
`@@ -51,7 +50,6 @@ public async Task Publish_UpdatesFilesWhenSourcesChange()`
`51`	`50`	`}`
`52`	`51`
`53`	`52`	`[Fact]`
`54`		`- [QuarantinedTest("https://github.com/dotnet/aspnetcore/issues/23397")]`
`55`	`53`	`public async Task Publish_WithoutLinkerAndCompression_UpdatesFilesWhenSourcesChange()`
`56`	`54`	`{`
`57`	`55`	`// Arrange`
`@@ -82,7 +80,6 @@ public async Task Publish_WithoutLinkerAndCompression_UpdatesFilesWhenSourcesCha`
`82`	`80`	`}`
`83`	`81`
`84`	`82`	`[Fact]`
`85`		`- [QuarantinedTest("https://github.com/dotnet/aspnetcore/issues/23756")]`
`86`	`83`	`public async Task Publish_WithLinkerAndCompression_IsIncremental()`
`87`	`84`	`{`
`88`	`85`	`// Arrange`
Original file line number	Diff line number	Diff line change
`@@ -146,7 +146,6 @@ public async Task Publish_WithScopedCss_Works()`
`146`	`146`	`}`
`147`	`147`
`148`	`148`	`[Fact]`
`149`		`- [QuarantinedTest("https://github.com/dotnet/aspnetcore/issues/23756")]`
`150`	`149`	`public async Task Publish_InRelease_Works()`
`151`	`150`	`{`
`152`	`151`	`// Arrange`
`@@ -314,7 +313,6 @@ public async Task Publish_WithTrimmingdDisabled_Works()`
`314`	`313`	`}`
`315`	`314`
`316`	`315`	`[Fact]`
`317`		`- [QuarantinedTest("https://github.com/dotnet/aspnetcore/issues/23756")]`
`318`	`316`	`public async Task Publish_SatelliteAssemblies_AreCopiedToBuildOutput()`
`319`	`317`	`{`
`320`	`318`	`// Arrange`
`@@ -348,7 +346,6 @@ public async Task Publish_SatelliteAssemblies_AreCopiedToBuildOutput()`
`348`	`346`	`}`
`349`	`347`
`350`	`348`	`[Fact]`
`351`		`- [QuarantinedTest]`
`352`	`349`	`public async Task Publish_HostedApp_DefaultSettings_Works()`
`353`	`350`	`{`
`354`	`351`	`// Arrange`
`@@ -410,7 +407,6 @@ public async Task Publish_HostedApp_DefaultSettings_Works()`
`410`	`407`	`}`
`411`	`408`
`412`	`409`	`[Fact]`
`413`		`- [QuarantinedTest("https://github.com/dotnet/aspnetcore/issues/23756")]`
`414`	`410`	`public async Task Publish_HostedApp_ProducesBootJsonDataWithExpectedContent()`
`415`	`411`	`{`
`416`	`412`	`// Arrange`
Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,4 @@`
`1`		`-<Project Sdk="Microsoft.NET.Sdk">`
	`1`	`+<Project Sdk="Microsoft.NET.Sdk">`
`2`	`2`
`3`	`3`	`<PropertyGroup>`
`4`	`4`	`<!--`