From afa0e028fb7641b56072e1f57a9beb9ee3660d42 Mon Sep 17 00:00:00 2001 From: Christian Soltenborn Date: Tue, 26 Jun 2018 11:33:01 +0200 Subject: [PATCH 1/5] fixed broken error reporting --- .../Core/TestCases/TestCaseFactory.cs | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/GoogleTestAdapter/Core/TestCases/TestCaseFactory.cs b/GoogleTestAdapter/Core/TestCases/TestCaseFactory.cs index 6c4690a65..08cb9cef4 100644 --- a/GoogleTestAdapter/Core/TestCases/TestCaseFactory.cs +++ b/GoogleTestAdapter/Core/TestCases/TestCaseFactory.cs @@ -40,12 +40,11 @@ public IList CreateTestCases(Action reportTestCase = null) return NewCreateTestcases(reportTestCase); } + string workingDir = _settings.GetWorkingDirForDiscovery(_executable); + string finalParams = GetDiscoveryParams(); List standardOutput = new List(); try { - string workingDir = _settings.GetWorkingDirForDiscovery(_executable); - string finalParams = GetDiscoveryParams(); - int processExitCode = 0; ProcessLauncher launcher = null; var listTestsTask = new Task(() => @@ -68,8 +67,7 @@ public IList CreateTestCases(Action reportTestCase = null) } catch (Exception e) { - SequentialTestRunner.LogExecutionError(_logger, _executable, Path.GetFullPath(""), - GoogleTestConstants.ListTestsOption, e); + SequentialTestRunner.LogExecutionError(_logger, _executable, workingDir, finalParams, e); return new List(); } @@ -152,11 +150,10 @@ private IList NewCreateTestcases(Action reportTestCase) parser.ReportLine(s); }; + string workingDir = _settings.GetWorkingDirForDiscovery(_executable); + var finalParams = GetDiscoveryParams(); try { - string workingDir = _settings.GetWorkingDirForDiscovery(_executable); - var finalParams = GetDiscoveryParams(); - int processExitCode = ProcessExecutor.ExecutionFailed; ProcessExecutor executor = null; var listAndParseTestsTask = new Task(() => @@ -192,8 +189,7 @@ private IList NewCreateTestcases(Action reportTestCase) } catch (Exception e) { - SequentialTestRunner.LogExecutionError(_logger, _executable, Path.GetFullPath(""), - GoogleTestConstants.ListTestsOption, e); + SequentialTestRunner.LogExecutionError(_logger, _executable, workingDir, finalParams, e); return new List(); } return testCases; From 4917c7681e9c9b30bec204886c557ff5a2c53535 Mon Sep 17 00:00:00 2001 From: Christian Soltenborn Date: Tue, 26 Jun 2018 13:39:40 +0200 Subject: [PATCH 2/5] further improved error reporting --- GoogleTestAdapter/Core/Runners/SequentialTestRunner.cs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/GoogleTestAdapter/Core/Runners/SequentialTestRunner.cs b/GoogleTestAdapter/Core/Runners/SequentialTestRunner.cs index e016be09c..ec12c420e 100644 --- a/GoogleTestAdapter/Core/Runners/SequentialTestRunner.cs +++ b/GoogleTestAdapter/Core/Runners/SequentialTestRunner.cs @@ -138,7 +138,11 @@ private IEnumerable RunTests(string executable, string workingDir, b public static void LogExecutionError(ILogger logger, string executable, string workingDir, string arguments, Exception exception, string threadName = "") { logger.LogError($"{threadName}Failed to run test executable '{executable}': {exception.Message}"); - logger.DebugError($@"{threadName}Stacktrace:{Environment.NewLine}{exception.StackTrace}"); + if (exception is AggregateException aggregateException) + { + exception = aggregateException.Flatten(); + } + logger.DebugError($@"{threadName}Exception:{Environment.NewLine}{exception}"); logger.LogError( $"{threadName}{Strings.Instance.TroubleShootingLink}"); logger.LogError( From bbda25ee0ab46211bb67fbe74c74080005b41a67 Mon Sep 17 00:00:00 2001 From: Christian Soltenborn Date: Tue, 26 Jun 2018 18:52:48 +0200 Subject: [PATCH 3/5] bugfix: PdbLocator could crash if PATH contained invalid chars --- .../DiaResolver.Tests/PdbLocatorTests.cs | 39 ++++++++++++++++++- GoogleTestAdapter/DiaResolver/PdbLocator.cs | 18 +++++++-- 2 files changed, 52 insertions(+), 5 deletions(-) diff --git a/GoogleTestAdapter/DiaResolver.Tests/PdbLocatorTests.cs b/GoogleTestAdapter/DiaResolver.Tests/PdbLocatorTests.cs index 51feeedec..37dbf2d72 100644 --- a/GoogleTestAdapter/DiaResolver.Tests/PdbLocatorTests.cs +++ b/GoogleTestAdapter/DiaResolver.Tests/PdbLocatorTests.cs @@ -1,4 +1,5 @@ -using System.Diagnostics.CodeAnalysis; +using System; +using System.Diagnostics.CodeAnalysis; using System.IO; using FluentAssertions; using GoogleTestAdapter.Tests.Common; @@ -58,5 +59,41 @@ public void FindPdbFile_ExeWithoutPdb_AttemptsToFindPdbAreLogged() .Should() .Contain(msg => msg.Contains("Attempts to find pdb:")); } + + [TestMethod] + [TestCategory(Unit)] + [SuppressMessage("ReSharper", "AssignNullToNotNullAttribute")] + public void FindPdbFile_PATHcontainsInvalidChars_ErrorIsLogged() + { + TestResources.LoadTests_ReleaseX86.AsFileInfo().Should().Exist(); + string pdb = Path.ChangeExtension(TestResources.LoadTests_ReleaseX86, ".pdb"); + pdb.AsFileInfo().Should().Exist(); + string renamedPdb = $"{pdb}.bak"; + renamedPdb.AsFileInfo().Should().NotExist(); + + string pdbFound; + var fakeLogger = new FakeLogger(() => true); + var currentPath = Environment.GetEnvironmentVariable("PATH"); + try + { + File.Move(pdb, renamedPdb); + pdb.AsFileInfo().Should().NotExist(); + + Environment.SetEnvironmentVariable("PATH", $"MyPath;{currentPath}"); + pdbFound = PdbLocator.FindPdbFile(TestResources.LoadTests_ReleaseX86, "", fakeLogger); + } + finally + { + Environment.SetEnvironmentVariable("PATH", currentPath); + File.Move(renamedPdb, pdb); + pdb.AsFileInfo().Should().Exist(); + } + + pdbFound.Should().BeNull(); + fakeLogger.Errors + .Should() + .Contain(msg => msg.Contains("invalid path")); + } + } } \ No newline at end of file diff --git a/GoogleTestAdapter/DiaResolver/PdbLocator.cs b/GoogleTestAdapter/DiaResolver/PdbLocator.cs index 86c178a42..7b068eeda 100644 --- a/GoogleTestAdapter/DiaResolver/PdbLocator.cs +++ b/GoogleTestAdapter/DiaResolver/PdbLocator.cs @@ -33,10 +33,20 @@ public static string FindPdbFile(string binary, string pathExtension, ILogger lo { foreach (string pathElement in pathElements) { - string file = Path.Combine(pathElement, pdb); - if (File.Exists(file)) - return file; - attempts.Add($"\"{file}\""); + try + { + string file = Path.Combine(pathElement, pdb); + if (File.Exists(file)) + return file; + attempts.Add($"\"{file}\""); + } + catch (Exception e) + { + string message = $"Exception while searching for the PDB file of binary '{binary}'. "; + message += "Do you have some invalid path on your PATH environment variable? "; + message += $"The according path is '{pathElement}' and will be ignored. Exception:{Environment.NewLine}{e}"; + logger.LogError(message); + } } } From 00324772746b1033d4af3497dec20ffbbebf8feb Mon Sep 17 00:00:00 2001 From: Christian Soltenborn Date: Tue, 26 Jun 2018 21:45:52 +0200 Subject: [PATCH 4/5] log warning instead of error for PATH problem --- GoogleTestAdapter/DiaResolver/PdbLocator.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/GoogleTestAdapter/DiaResolver/PdbLocator.cs b/GoogleTestAdapter/DiaResolver/PdbLocator.cs index 7b068eeda..d1d8d8383 100644 --- a/GoogleTestAdapter/DiaResolver/PdbLocator.cs +++ b/GoogleTestAdapter/DiaResolver/PdbLocator.cs @@ -43,9 +43,10 @@ public static string FindPdbFile(string binary, string pathExtension, ILogger lo catch (Exception e) { string message = $"Exception while searching for the PDB file of binary '{binary}'. "; - message += "Do you have some invalid path on your PATH environment variable? "; - message += $"The according path is '{pathElement}' and will be ignored. Exception:{Environment.NewLine}{e}"; - logger.LogError(message); + message += "Do you have some invalid path on your system's PATH environment variable? "; + message += $"The according path is '{pathElement}' and will be ignored."; + logger.LogWarning(message); + logger.DebugWarning($"Exception:{Environment.NewLine}{e}"); } } } From 60b30c15ea59f6bae5c6d0f93d8c79021497cb6b Mon Sep 17 00:00:00 2001 From: Christian Soltenborn Date: Tue, 26 Jun 2018 21:47:35 +0200 Subject: [PATCH 5/5] fixed test --- GoogleTestAdapter/DiaResolver.Tests/PdbLocatorTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/GoogleTestAdapter/DiaResolver.Tests/PdbLocatorTests.cs b/GoogleTestAdapter/DiaResolver.Tests/PdbLocatorTests.cs index 37dbf2d72..cfdf347f9 100644 --- a/GoogleTestAdapter/DiaResolver.Tests/PdbLocatorTests.cs +++ b/GoogleTestAdapter/DiaResolver.Tests/PdbLocatorTests.cs @@ -90,7 +90,7 @@ public void FindPdbFile_PATHcontainsInvalidChars_ErrorIsLogged() } pdbFound.Should().BeNull(); - fakeLogger.Errors + fakeLogger.Warnings .Should() .Contain(msg => msg.Contains("invalid path")); }