Separated out responsibilities a bit

src/Tools/Source/RunTests/Constants.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.Linq;
+using System.Text;
+using System.Threading.Tasks;
+
+namespace RunTests
+{
+    internal static class Constants
+    {
+        internal const string ResultsDirectoryName = "xUnitResults";
+    }
+}

src/Tools/Source/RunTests/ITestExecutor.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.IO;
 using System.Threading;
 using System.Threading.Tasks;
 
@@ -8,16 +9,29 @@ namespace RunTests
 {
     internal struct TestResult
     {
-        internal readonly bool Succeeded;
-        internal readonly string AssemblyName;
-        internal readonly TimeSpan Elapsed;
-        internal readonly string ErrorOutput;
+        internal int ExitCode { get; }
+        internal string AssemblyPath { get; }
+        internal string AssemblyName { get; }
+        internal string CommandLine { get; }
+        internal TimeSpan Elapsed { get; }
+        internal string StandardOutput { get; }
+        internal string ErrorOutput { get; }
 
-        internal TestResult(bool succeeded, string assemblyName, TimeSpan elapsed, string errorOutput)
+        /// <summary>
+        /// Path to the results file.  Can be null in the case xunit error'd and did not create one. 
+        /// </summary>
+        internal string ResultsFilePath { get; }
+        internal bool Succeeded => ExitCode == 0;
+
+        internal TestResult(int exitCode, string assemblyPath, string resultsFilePath, string commandLine, TimeSpan elapsed, string standardOutput, string errorOutput)
         {
-            Succeeded = succeeded;
-            AssemblyName = assemblyName;
+            ExitCode = exitCode;
+            AssemblyName = Path.GetFileName(assemblyPath);
+            AssemblyPath = assemblyPath;
+            CommandLine = commandLine;
+            ResultsFilePath = resultsFilePath;
             Elapsed = elapsed;
+            StandardOutput = standardOutput;
             ErrorOutput = errorOutput;
         }
     }

src/Tools/Source/RunTests/LocalDataStorage.cs

@@ -38,7 +38,14 @@ public bool TryGetTestResult(string cacheKey, out TestResult testResult)
                 var text = File.ReadAllText(filePath);
                 if (text.Length > 0)
                 {
-                    testResult = new TestResult(succeeded: true, assemblyName: text, elapsed: TimeSpan.FromSeconds(0), errorOutput: string.Empty);
+                    testResult = new TestResult(
+                        exitCode: 0,
+                        assemblyPath: text,
+                        elapsed: TimeSpan.FromSeconds(0),
+                        commandLine: string.Empty,
+                        resultsFilePath: null,
+                        standardOutput: string.Empty,
+                        errorOutput: string.Empty);
                     return true;
                 }
             }

src/Tools/Source/RunTests/ProcessTestExecutor.cs

@@ -25,20 +25,19 @@ public async Task<TestResult> RunTest(string assemblyPath, CancellationToken can
             try
             { 
                 var assemblyName = Path.GetFileName(assemblyPath);
-                var resultsFile = Path.Combine(Path.GetDirectoryName(assemblyPath), "xUnitResults", $"{assemblyName}.{(_options.UseHtml ? "html" : "xml")}");
-                var resultsDir = Path.GetDirectoryName(resultsFile);
-                var outputLogPath = Path.Combine(resultsDir, $"{assemblyName}.out.log");
+                var resultsDir = Path.Combine(Path.GetDirectoryName(assemblyPath), Constants.ResultsDirectoryName);
+                var resultsFilePath = Path.Combine(resultsDir, $"{assemblyName}.{(_options.UseHtml ? "html" : "xml")}");
 
                 // NOTE: xUnit doesn't always create the log directory
                 Directory.CreateDirectory(resultsDir);
 
                 // NOTE: xUnit seems to have an occasional issue creating logs create
                 // an empty log just in case, so our runner will still fail.
-                File.Create(resultsFile).Close();
+                File.Create(resultsFilePath).Close();
 
                 var builder = new StringBuilder();
                 builder.AppendFormat(@"""{0}""", assemblyPath);
-                builder.AppendFormat(@" -{0} ""{1}""", _options.UseHtml ? "html" : "xml", resultsFile);
+                builder.AppendFormat(@" -{0} ""{1}""", _options.UseHtml ? "html" : "xml", resultsFilePath);
                 builder.Append(" -noshadow -verbose");
 
                 if (!string.IsNullOrWhiteSpace(_options.Trait))
@@ -59,7 +58,6 @@ public async Task<TestResult> RunTest(string assemblyPath, CancellationToken can
                     }
                 }
 
-                var errorOutput = new StringBuilder();
                 var start = DateTime.UtcNow;
 
                 var xunitPath = _options.XunitPath;
@@ -74,8 +72,6 @@ public async Task<TestResult> RunTest(string assemblyPath, CancellationToken can
 
                 if (processOutput.ExitCode != 0)
                 {
-                    File.WriteAllLines(outputLogPath, processOutput.OutputLines);
-
                     // On occasion we get a non-0 output but no actual data in the result file.  The could happen
                     // if xunit manages to crash when running a unit test (a stack overflow could cause this, for instance).
                     // To avoid losing information, write the process output to the console.  In addition, delete the results
@@ -83,7 +79,7 @@ public async Task<TestResult> RunTest(string assemblyPath, CancellationToken can
                     var resultData = string.Empty;
                     try
                     {
-                        resultData = File.ReadAllText(resultsFile).Trim();
+                        resultData = File.ReadAllText(resultsFilePath).Trim();
                     }
                     catch
                     {
@@ -93,33 +89,23 @@ public async Task<TestResult> RunTest(string assemblyPath, CancellationToken can
                     if (resultData.Length == 0)
                     {
                         // Delete the output file.
-                        File.Delete(resultsFile);
-                    }
-
-                    errorOutput.AppendLine($"Command: {_options.XunitPath} {builder}");
-                    errorOutput.AppendLine($"xUnit output: {outputLogPath}");
-
-                    if (processOutput.ErrorLines.Any())
-                    {
-                        foreach (var line in processOutput.ErrorLines)
-                        {
-                            errorOutput.AppendLine(line);
-                        }
-                    }
-                    else
-                    {
-                        errorOutput.AppendLine($"xunit produced no error output but had exit code {processOutput.ExitCode}");
-                    }
-
-                    // If the results are html, use Process.Start to open in the browser.
-
-                    if (_options.UseHtml && resultData.Length > 0)
-                    {
-                        Process.Start(resultsFile);
+                        File.Delete(resultsFilePath);
+                        resultsFilePath = null;
                     }
                 }
 
-                return new TestResult(processOutput.ExitCode == 0, assemblyName, span, errorOutput.ToString());
+                var commandLine = $"{xunitPath} {builder.ToString()}";
+                var standardOutput = string.Join(Environment.NewLine, processOutput.OutputLines);
+                var errorOutput = string.Join(Environment.NewLine, processOutput.ErrorLines);
+
+                return new TestResult(
+                    exitCode: processOutput.ExitCode,
+                    assemblyPath: assemblyPath,
+                    resultsFilePath: resultsFilePath,
+                    commandLine: commandLine,
+                    elapsed: span,
+                    standardOutput: standardOutput,
+                    errorOutput: errorOutput);
             }
             catch (Exception ex)
             {

src/Tools/Source/RunTests/Program.cs

@@ -32,7 +32,7 @@ internal static int Main(string[] args)
             // TODO: Should caching test execution be enabled / disabled by an option?
             ITestExecutor testExecutor = new ProcessTestExecutor(options);
             testExecutor = new CachingTestExecutor(options, testExecutor, new LocalDataStorage());
-            var testRunner = new TestRunner(testExecutor);
+            var testRunner = new TestRunner(options, testExecutor);
             var start = DateTime.Now;
 
             Console.WriteLine("Running {0} test assemblies", options.Assemblies.Count());

src/Tools/Source/RunTests/RunTests.csproj

@@ -25,6 +25,7 @@
   <ItemGroup>
     <Compile Include="CacheUtil.cs" />
     <Compile Include="CachingTestExecutor.cs" />
+    <Compile Include="Constants.cs" />
     <Compile Include="FileUtil.cs" />
     <Compile Include="IDataStorage.cs" />
     <Compile Include="ITestExecutor.cs" />

src/Tools/Source/RunTests/TestRunner.cs

@@ -15,10 +15,12 @@ namespace RunTests
     internal sealed class TestRunner
     {
         private readonly ITestExecutor _testExecutor;
+        private readonly Options _options;
 
-        internal TestRunner(ITestExecutor testExecutor)
+        internal TestRunner(Options options, ITestExecutor testExecutor)
         {
             _testExecutor = testExecutor;
+            _options = options;
         }
 
         internal async Task<bool> RunAllAsync(IEnumerable<string> assemblyList, CancellationToken cancellationToken)
@@ -84,8 +86,7 @@ private void Print(List<TestResult> testResults)
 
             foreach (var testResult in testResults.Where(x => !x.Succeeded))
             {
-                Console.WriteLine("Errors {0}: ", testResult.AssemblyName);
-                Console.WriteLine(testResult.ErrorOutput);
+                PrintFailedTestResult(testResult);
             }
 
             Console.WriteLine("================");
@@ -96,130 +97,35 @@ private void Print(List<TestResult> testResults)
             }
             Console.WriteLine("================");
         }
-<<<<<<< 4e4ca09e79f46b74191a17e957f66832fcf98bab
 
-        private async Task<TestResult> RunTest(string assemblyPath, CancellationToken cancellationToken)
+        private void PrintFailedTestResult(TestResult testResult)
         {
-            try
-            {
-                var assemblyName = Path.GetFileName(assemblyPath);
-                var resultsFile = Path.Combine(Path.GetDirectoryName(assemblyPath), "xUnitResults", $"{assemblyName}.{(_options.UseHtml ? "html" : "xml")}");
-                var resultsDir = Path.GetDirectoryName(resultsFile);
-                var outputLogPath = Path.Combine(resultsDir, $"{assemblyName}.out.log");
-
-                // NOTE: xUnit doesn't always create the log directory
-                Directory.CreateDirectory(resultsDir);
-
-                // NOTE: xUnit seems to have an occasional issue creating logs create
-                // an empty log just in case, so our runner will still fail.
-                File.Create(resultsFile).Close();
-
-                var builder = new StringBuilder();
-                builder.AppendFormat(@"""{0}""", assemblyPath);
-                builder.AppendFormat(@" -{0} ""{1}""", _options.UseHtml ? "html" : "xml", resultsFile);
-                builder.Append(" -noshadow -verbose");
-
-                if (!string.IsNullOrWhiteSpace(_options.Trait))
-                {
-                    var traits = _options.Trait.Split(new char[] { ';' }, StringSplitOptions.RemoveEmptyEntries);
-                    foreach (var trait in traits)
-                    {
-                        builder.AppendFormat(" -trait {0}", trait);
-                    }
-                }
-
-                if (!string.IsNullOrWhiteSpace(_options.NoTrait))
-                {
-                    var traits = _options.NoTrait.Split(new char[] { ';' }, StringSplitOptions.RemoveEmptyEntries);
-                    foreach (var trait in traits)
-                    {
-                        builder.AppendFormat(" -notrait {0}", trait);
-                    }
-                }
+            // Save out the error output for easy artifact inspecting
+            var resultsDir = Path.GetDirectoryName(testResult.ResultsFilePath);
+            var outputLogPath = Path.Combine(resultsDir, $"{testResult.AssemblyName}.out.log");
+            File.WriteAllText(outputLogPath, testResult.StandardOutput);
 
-                var errorOutput = new StringBuilder();
-                var start = DateTime.UtcNow;
-
-                var xunitPath = _options.XunitPath;
-                var processOutput = await ProcessRunner.RunProcessAsync(
-                    xunitPath,
-                    builder.ToString(),
-                    lowPriority: false,
-                    displayWindow: false,
-                    captureOutput: true,
-                    cancellationToken: cancellationToken).ConfigureAwait(false);
-                var span = DateTime.UtcNow - start;
-
-                if (processOutput.ExitCode != 0)
-                {
-                    File.WriteAllLines(outputLogPath, processOutput.OutputLines);
-
-                    // On occasion we get a non-0 output but no actual data in the result file.  The could happen
-                    // if xunit manages to crash when running a unit test (a stack overflow could cause this, for instance).
-                    // To avoid losing information, write the process output to the console.  In addition, delete the results
-                    // file to avoid issues with any tool attempting to interpret the (potentially malformed) text.
-                    var resultData = string.Empty;
-                    try
-                    {
-                        resultData = File.ReadAllText(resultsFile).Trim();
-                    }
-                    catch
-                    {
-                        // Happens if xunit didn't produce a log file
-                    }
+            Console.WriteLine("Errors {0}: ", testResult.AssemblyName);
+            Console.WriteLine(testResult.ErrorOutput);
 
-                    if (resultData.Length == 0)
-                    {
-                        // Delete the output file.
-                        File.Delete(resultsFile);
-                    }
+            Console.WriteLine($"Command: {testResult.CommandLine}");
+            Console.WriteLine($"xUnit output: {outputLogPath}");
 
-                    errorOutput.AppendLine($"Command: {_options.XunitPath} {builder}");
-                    errorOutput.AppendLine($"xUnit output: {outputLogPath}");
-
-                    if (processOutput.ErrorLines.Any())
-                    {
-                        foreach (var line in processOutput.ErrorLines)
-                        {
-                            errorOutput.AppendLine(line);
-                        }
-                    }
-                    else
-                    {
-                        errorOutput.AppendLine($"xunit produced no error output but had exit code {processOutput.ExitCode}");
-                    }
-
-                    // If the results are html, use Process.Start to open in the browser.
-
-                    if (_options.UseHtml && resultData.Length > 0)
-                    {
-                        Process.Start(resultsFile);
-                    }
-                }
-
-                return new TestResult(processOutput.ExitCode == 0, assemblyName, span, errorOutput.ToString());
-            }
-            catch (Exception ex)
+            if (!string.IsNullOrEmpty(testResult.ErrorOutput))
             {
-                throw new Exception($"Unable to run {assemblyPath} with {_options.XunitPath}. {ex}");
+                Console.WriteLine(testResult.ErrorOutput);
             }
-        }
-
-        private static void DeleteFile(string filePath)
-        {
-            try
-            {
-                if (File.Exists(filePath))
-                {
-                    File.Delete(filePath);
-                }
+            else
+            { 
+                Console.WriteLine($"xunit produced no error output but had exit code {testResult.ExitCode}");
             }
-            catch
+
+            // If the results are html, use Process.Start to open in the browser.
+            if (_options.UseHtml && !string.IsNullOrEmpty(testResult.ResultsFilePath))
             {
-                // Ignore
+                Process.Start(testResult.ResultsFilePath);
             }
         }
-=======
->>>>>>> Abstract out the executor
+>>>>>>> Separated out responsibilities a bit
     }
 }