From 0c657bb972ba1d3ca9d87ece78045b28ce7c295b Mon Sep 17 00:00:00 2001 From: Jared Parsons Date: Fri, 19 Feb 2016 11:12:53 -0800 Subject: [PATCH] Fix an issue in the test caching layer The test caching layer had two bugs around re-hydrating the test resutls from cache: - Made a bad assumption about the name of the file. - Assumed tests were always rehydrated into the original directory. closes #8860 --- .../RunTests/Cache/CachingTestExecutor.cs | 64 +++++++++------ .../Source/RunTests/Cache/IDataStorage.cs | 28 ++++++- .../Source/RunTests/Cache/LocalDataStorage.cs | 42 ++++------ src/Tools/Source/RunTests/ITestExecutor.cs | 2 + .../Source/RunTests/ProcessTestExecutor.cs | 77 ++++++++++++------- 5 files changed, 131 insertions(+), 82 deletions(-) diff --git a/src/Tools/Source/RunTests/Cache/CachingTestExecutor.cs b/src/Tools/Source/RunTests/Cache/CachingTestExecutor.cs index b4e8e1a51f0..66b00a7ed00 100644 --- a/src/Tools/Source/RunTests/Cache/CachingTestExecutor.cs +++ b/src/Tools/Source/RunTests/Cache/CachingTestExecutor.cs @@ -1,5 +1,6 @@ // 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 RunTestAsync(string assemblyPath, CancellationToken cancellationToken) { var contentFile = _contentUtil.GetTestResultContentFile(assemblyPath); @@ -31,16 +37,17 @@ public async Task 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 RunTestAsync(string assemblyPath, CancellationToke } /// - /// 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 + /// value. /// - /// - /// - private static TestResult Migrate(TestResult testResult) + private TestResult Migrate(string assemblyPath, CachedTestResult cachedTestResult) { - if (string.IsNullOrEmpty(testResult.ResultsFilePath)) - { - 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, - standardOutput: testResult.StandardOutput, - errorOutput: testResult.ErrorOutput); + 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, + resultsFileName: Path.GetFileName(testResult.ResultsFilePath), + resultsFileContent: resultFileContent); + _dataStorage.AddCachedTestResult(contentFile, cachedTestResult); + } + catch (Exception ex) + { + Logger.Log($"Failed to create cached {ex}"); + } } } } diff --git a/src/Tools/Source/RunTests/Cache/IDataStorage.cs b/src/Tools/Source/RunTests/Cache/IDataStorage.cs index b046586f7de..b93c3ea753a 100644 --- a/src/Tools/Source/RunTests/Cache/IDataStorage.cs +++ b/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; + } } } + diff --git a/src/Tools/Source/RunTests/Cache/LocalDataStorage.cs b/src/Tools/Source/RunTests/Cache/LocalDataStorage.cs index afc662bc679..76f420ee971 100644 --- a/src/Tools/Source/RunTests/Cache/LocalDataStorage.cs +++ b/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) { diff --git a/src/Tools/Source/RunTests/ITestExecutor.cs b/src/Tools/Source/RunTests/ITestExecutor.cs index c064b1e52fd..28713ab395d 100644 --- a/src/Tools/Source/RunTests/ITestExecutor.cs +++ b/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 RunTestAsync(string assemblyPath, CancellationToken cancellationToken); } } diff --git a/src/Tools/Source/RunTests/ProcessTestExecutor.cs b/src/Tools/Source/RunTests/ProcessTestExecutor.cs index 1086eb0c872..d8a76c1699a 100644 --- a/src/Tools/Source/RunTests/ProcessTestExecutor.cs +++ b/src/Tools/Source/RunTests/ProcessTestExecutor.cs @@ -20,13 +20,56 @@ internal ProcessTestExecutor(Options options) _options = options; } + public string GetCommandLine(string assemblyPath) + { + return $"{_options.XunitPath} {GetCommandLineArguments(assemblyPath)}"; + } + + public string GetCommandLineArguments(string assemblyPath) + { + var assemblyName = Path.GetFileName(assemblyPath); + var resultsFilePath = GetResultsFilePath(assemblyPath); + + var builder = new StringBuilder(); + builder.AppendFormat(@"""{0}""", assemblyPath); + builder.AppendFormat(@" -{0} ""{1}""", _options.UseHtml ? "html" : "xml", resultsFilePath); + 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); + } + } + + 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 RunTestAsync(string assemblyPath, CancellationToken cancellationToken) { try { - var assemblyName = Path.GetFileName(assemblyPath); - var resultsDir = Path.Combine(Path.GetDirectoryName(assemblyPath), Constants.ResultsDirectoryName); - var resultsFilePath = Path.Combine(resultsDir, $"{assemblyName}.{(_options.UseHtml ? "html" : "xml")}"); + 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); @@ -35,35 +78,11 @@ public async Task RunTestAsync(string assemblyPath, CancellationToke // an empty log just in case, so our runner will still fail. File.Create(resultsFilePath).Close(); - var builder = new StringBuilder(); - builder.AppendFormat(@"""{0}""", assemblyPath); - builder.AppendFormat(@" -{0} ""{1}""", _options.UseHtml ? "html" : "xml", resultsFilePath); - 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); - } - } - 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 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); -- GitLab