Merge pull request #8943 from jaredpar/fix-cache

Fix an issue in the test caching layer

src/Tools/Source/RunTests/Cache/CachingTestExecutor.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.Text;
 using System.Threading;
@@ -20,6 +21,11 @@ internal CachingTestExecutor(Options options, ITestExecutor testExecutor, IDataS
             _contentUtil = new ContentUtil(options);
         }
 
+        public string GetCommandLine(string assemblyPath)
+        {
+            return _testExecutor.GetCommandLine(assemblyPath);
+        }
+
         public async Task<TestResult> RunTestAsync(string assemblyPath, CancellationToken cancellationToken)
         {
             var contentFile = _contentUtil.GetTestResultContentFile(assemblyPath);
@@ -31,16 +37,17 @@ public async Task<TestResult> RunTestAsync(string assemblyPath, CancellationToke
             Logger.Log(builder.ToString());
 
             TestResult testResult;
-            if (!_dataStorage.TryGetTestResult(contentFile.Checksum, out testResult))
+            CachedTestResult cachedTestResult;
+            if (!_dataStorage.TryGetCachedTestResult(contentFile.Checksum, out cachedTestResult))
             {
                 Logger.Log($"{Path.GetFileName(assemblyPath)} - running");
                 testResult = await _testExecutor.RunTestAsync(assemblyPath, cancellationToken);
                 Logger.Log($"{Path.GetFileName(assemblyPath)} - caching");
-                _dataStorage.AddTestResult(contentFile, testResult);
+                CacheTestResult(contentFile, testResult);
             }
             else
             {
-                testResult = Migrate(testResult);
+                testResult = Migrate(assemblyPath, cachedTestResult);
                 Logger.Log($"{Path.GetFileName(assemblyPath)} - cache hit");
             }
 
@@ -48,32 +55,45 @@ public async Task<TestResult> RunTestAsync(string assemblyPath, CancellationToke
         }
 
         /// <summary>
-        /// The results file is specified in terms of the cache storage.  Need to make it local
-        /// to the current output folder
+        /// Recreate the on disk artifacts for the cached data and return the correct <see cref="TestResult"/>
+        /// value.
         /// </summary>
-        /// <param name="testResult"></param>
-        /// <returns></returns>
-        private static TestResult Migrate(TestResult testResult)
-        {
-            if (string.IsNullOrEmpty(testResult.ResultsFilePath))
+        private TestResult Migrate(string assemblyPath, CachedTestResult cachedTestResult)
         {
-                return testResult;
-            }
-
-            var resultsDir = Path.Combine(Path.GetDirectoryName(testResult.AssemblyPath), Constants.ResultsDirectoryName);
+            var resultsDir = Path.Combine(Path.GetDirectoryName(assemblyPath), Constants.ResultsDirectoryName);
             FileUtil.EnsureDirectory(resultsDir);
-            var resultsFilePath = Path.Combine(resultsDir, Path.GetFileName(testResult.ResultsFilePath));
-            File.Copy(testResult.ResultsFilePath, resultsFilePath, overwrite: true);
+            var resultsFilePath = Path.Combine(resultsDir, cachedTestResult.ResultsFileName);
+            File.WriteAllText(resultsFilePath, cachedTestResult.ResultsFileContent);
+            var commandLine = _testExecutor.GetCommandLine(assemblyPath);
 
             return new TestResult(
-                exitCode: testResult.ExitCode,
-                assemblyPath: testResult.AssemblyName,
+                exitCode: cachedTestResult.ExitCode,
+                assemblyPath: assemblyPath,
                 resultDir: resultsDir,
                 resultsFilePath: resultsFilePath,
-                commandLine: testResult.CommandLine,
-                elapsed: testResult.Elapsed,
+                commandLine: commandLine,
+                elapsed: TimeSpan.FromMilliseconds(0),
+                standardOutput: cachedTestResult.StandardOutput,
+                errorOutput: cachedTestResult.ErrorOutput);
+        }
+
+        private void CacheTestResult(ContentFile contentFile, TestResult testResult)
+        {
+            try
+            {
+                var resultFileContent = File.ReadAllText(testResult.ResultsFilePath);
+                var cachedTestResult = new CachedTestResult(
+                    exitCode: testResult.ExitCode,
                     standardOutput: testResult.StandardOutput,
-                errorOutput: testResult.ErrorOutput);
+                    errorOutput: testResult.ErrorOutput,
+                    resultsFileName: Path.GetFileName(testResult.ResultsFilePath),
+                    resultsFileContent: resultFileContent);
+                _dataStorage.AddCachedTestResult(contentFile, cachedTestResult);
+            }
+            catch (Exception ex)
+            {
+                Logger.Log($"Failed to create cached {ex}");
+            }
         }
     }
 }

src/Tools/Source/RunTests/Cache/IDataStorage.cs

@@ -10,8 +10,32 @@ namespace RunTests.Cache
 {
     internal interface IDataStorage
     {
-        bool TryGetTestResult(string checksum, out TestResult testResult);
+        bool TryGetCachedTestResult(string checksum, out CachedTestResult testResult);
 
-        void AddTestResult(ContentFile conentFile, TestResult testResult);
+        void AddCachedTestResult(ContentFile conentFile, CachedTestResult testResult);
+    }
+
+    internal struct CachedTestResult
+    {
+        internal int ExitCode { get; }
+        internal string StandardOutput { get; }
+        internal string ErrorOutput { get; }
+        internal string ResultsFileName { get; }
+        internal string ResultsFileContent { get; }
+
+        internal CachedTestResult(
+            int exitCode,
+            string standardOutput,
+            string errorOutput,
+            string resultsFileName,
+            string resultsFileContent)
+        {
+            ExitCode = exitCode;
+            StandardOutput = standardOutput;
+            ErrorOutput = errorOutput;
+            ResultsFileName = resultsFileName;
+            ResultsFileContent = resultsFileContent;
+        }
     }
 }
+

src/Tools/Source/RunTests/Cache/LocalDataStorage.cs

@@ -17,12 +17,11 @@ internal sealed class LocalDataStorage : IDataStorage
     {
         private enum StorageKind
         {
-            AssemblyPath,
             ExitCode,
-            CommandLine,
             StandardOutput,
             ErrorOutput,
-            ResultsFile,
+            ResultsFileContent,
+            ResultsFileName,
             Content
         }
 
@@ -36,9 +35,9 @@ internal LocalDataStorage(string storagePath = null)
             _storagePath = storagePath ?? Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.LocalApplicationData), DirectoryName);
         }
 
-        public bool TryGetTestResult(string checksum, out TestResult testResult)
+        public bool TryGetCachedTestResult(string checksum, out CachedTestResult testResult)
         {
-            testResult = default(TestResult);
+            testResult = default(CachedTestResult);
 
             var storageFolder = GetStorageFolder(checksum);
             if (!Directory.Exists(storageFolder))
@@ -49,27 +48,17 @@ public bool TryGetTestResult(string checksum, out TestResult testResult)
             try
             {
                 var exitCode = Read(checksum, StorageKind.ExitCode);
-                var commandLine = Read(checksum, StorageKind.CommandLine);
-                var assemblyPath = Read(checksum, StorageKind.AssemblyPath);
                 var standardOutput = Read(checksum, StorageKind.StandardOutput);
                 var errorOutput = Read(checksum, StorageKind.ErrorOutput);
+                var resultsFileName = Read(checksum, StorageKind.ResultsFileName);
+                var resultsFileContent = Read(checksum, StorageKind.ResultsFileContent);
 
-                var resultsFilePath = GetStoragePath(checksum, StorageKind.ResultsFile);
-                var resultDir = Path.GetDirectoryName(resultsFilePath);
-                if (!File.Exists(resultsFilePath))
-                {
-                    resultsFilePath = null;
-                }
-
-                testResult = new TestResult(
+                testResult = new CachedTestResult(
                     exitCode: int.Parse(exitCode),
-                    assemblyPath: assemblyPath,
-                    resultDir: resultDir,
-                    resultsFilePath: resultsFilePath,
-                    commandLine: commandLine,
-                    elapsed: TimeSpan.FromSeconds(0),
                     standardOutput: standardOutput,
-                    errorOutput: errorOutput);
+                    errorOutput: errorOutput,
+                    resultsFileName: resultsFileName,
+                    resultsFileContent: resultsFileContent);
                 return true;
             }
             catch (Exception e)
@@ -81,7 +70,7 @@ public bool TryGetTestResult(string checksum, out TestResult testResult)
             return false;
         }
 
-        public void AddTestResult(ContentFile contentFile, TestResult testResult)
+        public void AddCachedTestResult(ContentFile contentFile, CachedTestResult testResult)
         {
             var checksum = contentFile.Checksum;
             var storagePath = Path.Combine(_storagePath, checksum);
@@ -93,16 +82,11 @@ public void AddTestResult(ContentFile contentFile, TestResult testResult)
                 }
 
                 Write(checksum, StorageKind.ExitCode, testResult.ExitCode.ToString());
-                Write(checksum, StorageKind.AssemblyPath, testResult.AssemblyPath);
                 Write(checksum, StorageKind.StandardOutput, testResult.StandardOutput);
                 Write(checksum, StorageKind.ErrorOutput, testResult.ErrorOutput);
-                Write(checksum, StorageKind.CommandLine, testResult.CommandLine);
+                Write(checksum, StorageKind.ResultsFileName, testResult.ResultsFileName);
+                Write(checksum, StorageKind.ResultsFileContent, testResult.ResultsFileContent);
                 Write(checksum, StorageKind.Content, contentFile.Content);
-
-                if (!string.IsNullOrEmpty(testResult.ResultsFilePath))
-                {
-                    File.Copy(testResult.ResultsFilePath, GetStoragePath(checksum, StorageKind.ResultsFile));
-                }
             }
             catch (Exception e)
             {

src/Tools/Source/RunTests/ITestExecutor.cs

@@ -41,6 +41,8 @@ internal TestResult(int exitCode, string assemblyPath, string resultDir, string
 
     internal interface ITestExecutor
     {
+        string GetCommandLine(string assemblyPath);
+
         Task<TestResult> RunTestAsync(string assemblyPath, CancellationToken cancellationToken);
     }
 }

src/Tools/Source/RunTests/ProcessTestExecutor.cs

@@ -20,20 +20,15 @@ internal ProcessTestExecutor(Options options)
             _options = options;
         }
 
-        public async Task<TestResult> RunTestAsync(string assemblyPath, CancellationToken cancellationToken)
+        public string GetCommandLine(string assemblyPath)
         {
-            try
+            return $"{_options.XunitPath} {GetCommandLineArguments(assemblyPath)}";
+        }
+
+        public string GetCommandLineArguments(string assemblyPath)
         {
             var assemblyName = Path.GetFileName(assemblyPath);
-                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(resultsFilePath).Close();
+            var resultsFilePath = GetResultsFilePath(assemblyPath);
 
             var builder = new StringBuilder();
             builder.AppendFormat(@"""{0}""", assemblyPath);
@@ -58,12 +53,36 @@ public async Task<TestResult> RunTestAsync(string assemblyPath, CancellationToke
                 }
             }
 
-                var start = DateTime.UtcNow;
+            return builder.ToString();
+        }
+
+        private string GetResultsFilePath(string assemblyPath)
+        {
+            var assemblyName = Path.GetFileName(assemblyPath);
+            var resultsDir = Path.Combine(Path.GetDirectoryName(assemblyPath), Constants.ResultsDirectoryName);
+            return Path.Combine(resultsDir, $"{assemblyName}.{(_options.UseHtml ? "html" : "xml")}");
+        }
+
+        public async Task<TestResult> RunTestAsync(string assemblyPath, CancellationToken cancellationToken)
+        {
+            try
+            {
+                var commandLineArguments = GetCommandLineArguments(assemblyPath);
+                var resultsFilePath = GetResultsFilePath(assemblyPath);
+                var resultsDir = Path.GetDirectoryName(resultsFilePath);
 
+                // 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(resultsFilePath).Close();
+
+                var start = DateTime.UtcNow;
                 var xunitPath = _options.XunitPath;
                 var processOutput = await ProcessRunner.RunProcessAsync(
                     xunitPath,
-                    builder.ToString(),
+                    commandLineArguments,
                     lowPriority: false,
                     displayWindow: false,
                     captureOutput: true,
@@ -94,7 +113,7 @@ public async Task<TestResult> RunTestAsync(string assemblyPath, CancellationToke
                     }
                 }
 
-                var commandLine = $"{xunitPath} {builder.ToString()}";
+                var commandLine = GetCommandLine(assemblyPath);
                 var standardOutput = string.Join(Environment.NewLine, processOutput.OutputLines);
                 var errorOutput = string.Join(Environment.NewLine, processOutput.ErrorLines);