VS integration tests should use common dump directory

Changes the VS integration tests to use the common dump directory that
all of our other infrastructure uses. Make it easy to debug failures
when all of the dump files are in the same directory.

src/Test/Utilities/Portable/FX/DumpProcRunner.cs

-// Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the Apache License, Version 2.0.  See License.txt in the project root for license information.
-
-using System;
-using System.Diagnostics;
-using System.IO;
-
-namespace Microsoft.CodeAnalysis.Test.Utilities
-{
-    public class ProcDumpRunner
-    {
-        public const string ProcDumpPathEnvironmentVariableKey = "ProcDumpPath";
-
-        private const string ProcDumpExeFileName = "procdump.exe";
-
-        // /accepteula command line option to automatically accept the Sysinternals license agreement.
-        // -ma	Write a 'Full' dump file. Includes All the Image, Mapped and Private memory.
-        // -e	Write a dump when the process encounters an unhandled exception. Include the 1 to create dump on first chance exceptions.
-        // -t	Write a dump when the process terminates.
-        private const string ProcDumpSwitches = "/accepteula -ma -e -t";
-
-        /// <summary>
-        /// Starts procdump.exe against the process.
-        /// </summary>
-        /// <param name="procDumpPath">The path to the procdump executable</param>
-        /// <param name="processId">process id</param>
-        /// <param name="processName">process name</param>
-        /// <param name="loggingMethod">method to log diagnostics to</param>
-        /// <param name="destinationDirectory">destination directory for dumps</param>
-        public static void StartProcDump(string procDumpPath, int processId, string processName, string destinationDirectory, Action<string> loggingMethod)
-        {
-            if (!string.IsNullOrWhiteSpace(procDumpPath))
-            {
-                var procDumpFilePath = Path.Combine(procDumpPath, ProcDumpExeFileName);
-                var dumpDirectory = Path.Combine(destinationDirectory, "Dumps");
-                Directory.CreateDirectory(dumpDirectory);
-
-                var procDumpProcess = Process.Start(procDumpFilePath, $" {ProcDumpSwitches} {processId} \"{dumpDirectory}\"");
-                loggingMethod($"Launched ProcDump attached to {processName} (process Id: {processId})");
-            }
-            else
-            {
-                loggingMethod($"Environment variables do not contain {ProcDumpPathEnvironmentVariableKey} (path to procdump.exe). Will skip attaching procdump to VS instance.");
-            }
-        }
-    }
-}

src/Tools/Source/RunTests/ITestExecutor.cs

@@ -11,7 +11,7 @@ namespace RunTests
     internal struct TestExecutionOptions
     {
         internal string XunitPath { get; }
-        internal string ProcDumpPath { get; }
+        internal ProcDumpInfo? ProcDumpInfo { get; }
         internal string LogFilePath { get; }
         internal string Trait { get; }
         internal string NoTrait { get; }
@@ -19,10 +19,10 @@ internal struct TestExecutionOptions
         internal bool Test64 { get; }
         internal bool TestVsi { get; }
         
-        internal TestExecutionOptions(string xunitPath, string procDumpPath, string logFilePath, string trait, string noTrait, bool useHtml, bool test64, bool testVsi)
+        internal TestExecutionOptions(string xunitPath, ProcDumpInfo? procDumpInfo, string logFilePath, string trait, string noTrait, bool useHtml, bool test64, bool testVsi)
         {
             XunitPath = xunitPath;
-            ProcDumpPath = procDumpPath;
+            ProcDumpInfo = procDumpInfo;
             LogFilePath = logFilePath;
             Trait = trait;
             NoTrait = noTrait;

src/Tools/Source/RunTests/ProcDumpUtil.cs

+// Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the Apache License, Version 2.0.  See License.txt in the project root for license information.
+
+using System;
+using System.Collections.Generic;
+using System.Diagnostics;
+using System.IO;
+
+namespace RunTests
+{
+    internal struct ProcDumpInfo
+    {
+        private const string s_KeyProcDumpFilePath = "ProcDumpFilePath";
+        private const string s_KeyDumpDirectory = "ProcDumpOutputPath";
+
+        internal string ProcDumpFilePath { get; }
+        internal string DumpDirectory { get; }
+
+        internal ProcDumpInfo(string procDumpFilePath, string dumpDirectory)
+        {
+            Debug.Assert(Path.IsPathRooted(procDumpFilePath));
+            Debug.Assert(Path.IsPathRooted(dumpDirectory));
+            ProcDumpFilePath = procDumpFilePath;
+            DumpDirectory = dumpDirectory;
+        }
+
+        internal void WriteEnvironmentVariables(Dictionary<string, string> environment)
+        {
+            environment[s_KeyProcDumpFilePath] = ProcDumpFilePath;
+            environment[s_KeyDumpDirectory] = DumpDirectory;
+        }
+
+        internal static ProcDumpInfo? ReadFromEnvironment()
+        {
+            bool validate(string s) => !string.IsNullOrEmpty(s) && Path.IsPathRooted(s);
+
+            var procDumpFilePath = Environment.GetEnvironmentVariable(s_KeyProcDumpFilePath);
+            var dumpDirectory = Environment.GetEnvironmentVariable(s_KeyDumpDirectory);
+
+            if (!validate(procDumpFilePath) || !validate(dumpDirectory))
+            {
+                return null;
+            }
+
+            return new ProcDumpInfo(procDumpFilePath, dumpDirectory);
+        }
+    }
+
+    internal static class ProcDumpUtil
+    {
+        internal static Process AttachProcDump(ProcDumpInfo procDumpInfo, int processId, string processName)
+        {
+            return AttachProcDump(procDumpInfo.ProcDumpFilePath, processId, processName, procDumpInfo.DumpDirectory);
+        }
+
+        /// <summary>
+        /// Attaches a new procdump.exe against the specified process.
+        /// </summary>
+        /// <param name="procDumpFilePath">The path to the procdump executable</param>
+        /// <param name="processId">process id</param>
+        /// <param name="processName">process name</param>
+        /// <param name="dumpDirectory">destination directory for dumps</param>
+        internal static Process AttachProcDump(string procDumpFilePath, int processId, string processName, string dumpDirectory)
+        {
+            // /accepteula command line option to automatically accept the Sysinternals license agreement.
+            // -ma	Write a 'Full' dump file. Includes All the Image, Mapped and Private memory.
+            // -e	Write a dump when the process encounters an unhandled exception. Include the 1 to create dump on first chance exceptions.
+            // -t	Write a dump when the process terminates.
+            const string procDumpSwitches = "/accepteula -ma -e -t";
+            Directory.CreateDirectory(dumpDirectory);
+            dumpDirectory = dumpDirectory.TrimEnd('\\');
+
+            return Process.Start(procDumpFilePath, $" {procDumpSwitches} {processId} \"{dumpDirectory}\"");
+        }
+    }
+}

src/Tools/Source/RunTests/ProcessTestExecutor.cs

@@ -9,7 +9,6 @@
 using System.Text;
 using System.Threading;
 using System.Threading.Tasks;
-using Microsoft.CodeAnalysis.Test.Utilities;
 
 namespace RunTests
 {
@@ -84,12 +83,22 @@ public async Task<TestResult> RunTestAsync(AssemblyInfo assemblyInfo, Cancellati
 
                 // Define environment variables for processes started via ProcessRunner.
                 var environmentVariables = new Dictionary<string, string>();
-                environmentVariables.Add(ProcDumpRunner.ProcDumpPathEnvironmentVariableKey, _options.ProcDumpPath);
+                _options.ProcDumpInfo?.WriteEnvironmentVariables(environmentVariables);
 
                 var outputDirectory = _options.LogFilePath != null
                     ? Path.GetDirectoryName(_options.LogFilePath)
                     : Directory.GetCurrentDirectory();
 
+                // Attach procDump to processes when the are started so we can watch for 
+                // unexepected crashes.
+                void onProcessStart(Process process)
+                {
+                    if (_options.ProcDumpInfo != null)
+                    {
+                        ProcDumpUtil.AttachProcDump(_options.ProcDumpInfo.Value, process.Id, process.ProcessName);
+                    }
+                }
+
                 var start = DateTime.UtcNow;
                 var xunitPath = _options.XunitPath;
                 var processOutput = await ProcessRunner.RunProcessAsync(
@@ -100,7 +109,7 @@ public async Task<TestResult> RunTestAsync(AssemblyInfo assemblyInfo, Cancellati
                     captureOutput: true,
                     cancellationToken: cancellationToken,
                     environmentVariables: environmentVariables,
-                    onProcessStartHandler: (process) => ProcDumpRunner.StartProcDump(_options.ProcDumpPath, process.Id, process.ProcessName, outputDirectory, Logger.Log));
+                    onProcessStartHandler: onProcessStart);
                 var span = DateTime.UtcNow - start;
 
                 if (processOutput.ExitCode != 0)

src/Tools/Source/RunTests/Program.cs

@@ -190,13 +190,10 @@ async Task DumpProcess(Process targetProcess, string dumpFilePath)
             }
 
             Console.WriteLine("Roslyn Error: test timeout exceeded, dumping remaining processes");
-            if (!string.IsNullOrEmpty(options.ProcDumpPath))
+            var procDumpInfo = GetProcDumpInfo(options);
+            if (procDumpInfo != null)
             {
-                var dumpDir = options.LogFilePath != null
-                    ? Path.GetDirectoryName(options.LogFilePath)
-                    : Directory.GetCurrentDirectory();
-                Console.WriteLine($"Dump file location is {dumpDir}");
-
+                var dumpDir = procDumpInfo.Value.DumpDirectory;
                 var counter = 0;
                 foreach (var proc in ProcessUtil.GetProcessTree(Process.GetCurrentProcess()).OrderBy(x => x.ProcessName))
                 {
@@ -213,6 +210,19 @@ async Task DumpProcess(Process targetProcess, string dumpFilePath)
             WriteLogFile(options);
         }
 
+        private static ProcDumpInfo? GetProcDumpInfo(Options options)
+        {
+            if (!string.IsNullOrEmpty(options.ProcDumpPath))
+            {
+                var dumpDir = options.LogFilePath != null
+                    ? Path.GetDirectoryName(options.LogFilePath)
+                    : Directory.GetCurrentDirectory();
+                return new ProcDumpInfo(options.ProcDumpPath, dumpDir);
+            }
+
+            return null;
+        }
+
         /// <summary>
         /// Quick sanity check to look over the set of assemblies to make sure they are valid and something was
         /// specified.
@@ -314,7 +324,7 @@ private static ITestExecutor CreateTestExecutor(Options options)
         {
             var testExecutionOptions = new TestExecutionOptions(
                 xunitPath: options.XunitPath,
-                procDumpPath: options.ProcDumpPath,
+                procDumpInfo: GetProcDumpInfo(options),
                 logFilePath: options.LogFilePath,
                 trait: options.Trait,
                 noTrait: options.NoTrait,

src/Tools/Source/RunTests/RunTests.csproj

@@ -26,8 +26,5 @@
     <PackageReference Include="System.Reflection.Metadata" Version="$(SystemReflectionMetadataVersion)" />
     <PackageReference Include="System.ValueTuple" Version="$(SystemValueTupleVersion)" />
   </ItemGroup>
-  <ItemGroup>
-    <Compile Include="..\..\..\Test\Utilities\Portable\FX\DumpProcRunner.cs" />
-  </ItemGroup>
   <Import Project="..\..\..\..\build\Targets\Imports.targets" />
 </Project>
\ No newline at end of file

src/VisualStudio/IntegrationTest/TestUtilities/VisualStudioInstanceFactory.cs

@@ -16,6 +16,7 @@
 using Microsoft.VisualStudio.IntegrationTest.Utilities.Interop;
 using Microsoft.VisualStudio.Setup.Configuration;
 using Process = System.Diagnostics.Process;
+using RunTests;
 
 namespace Microsoft.VisualStudio.IntegrationTest.Utilities
 {
@@ -168,8 +169,11 @@ private void UpdateCurrentlyRunningInstance(ImmutableHashSet<string> requiredPac
 
                 hostProcess = StartNewVisualStudioProcess(installationPath);
 
-                var procDumpPath = Environment.GetEnvironmentVariable(ProcDumpRunner.ProcDumpPathEnvironmentVariableKey);
-                ProcDumpRunner.StartProcDump(procDumpPath, hostProcess.Id, hostProcess.ProcessName, Path.Combine(GetAssemblyDirectory(), "xUnitResults"), s => Debug.WriteLine(s));
+                var procDumpInfo = ProcDumpInfo.ReadFromEnvironment();
+                if (procDumpInfo != null)
+                {
+                    ProcDumpUtil.AttachProcDump(procDumpInfo.Value, hostProcess.Id, hostProcess.ProcessName);
+                }
 
                 // We wait until the DTE instance is up before we're good
                 dte = IntegrationHelper.WaitForNotNullAsync(() => IntegrationHelper.TryLocateDteForProcess(hostProcess)).Result;

src/VisualStudio/IntegrationTest/TestUtilities/VisualStudioIntegrationTestUtilities.csproj

@@ -72,9 +72,12 @@
     <ProjectReference Include="..\..\CSharp\Repl\CSharpVisualStudioRepl.csproj" />
     <ProjectReference Include="..\..\InteractiveServices\VisualStudioInteractiveServices.csproj" />
   </ItemGroup>
+  <ItemGroup>
+    <Compile Include="..\..\..\Tools\Source\RunTests\ProcDumpUtil.cs" />
+  </ItemGroup>
   <ItemGroup>
     <InternalsVisibleTo Include="Microsoft.VisualStudio.LiveUnitTesting.IntegrationTests" />
     <InternalsVisibleToTest Include="Microsoft.VisualStudio.IntegrationTest.Setup" />
   </ItemGroup>
   <Import Project="..\..\..\..\build\Targets\Imports.targets" />
-</Project>
\ No newline at end of file
+</Project>