From 93c95f5d1df3880c00cf81ab8e514cf8c27c828d Mon Sep 17 00:00:00 2001 From: Russell Johnston Date: Mon, 12 Jun 2017 16:27:27 -0700 Subject: [PATCH 1/4] Store test signatures in a Dictionary rather than a List This reduces the runtime for test discovery from quadratic to linear --- .../Core/TestCases/TestCaseFactory.cs | 22 +++++------- .../Core/TestCases/TestCaseResolver.cs | 36 +++++++++++++------ 2 files changed, 33 insertions(+), 25 deletions(-) diff --git a/GoogleTestAdapter/Core/TestCases/TestCaseFactory.cs b/GoogleTestAdapter/Core/TestCases/TestCaseFactory.cs index 90d19296f..f2d360993 100644 --- a/GoogleTestAdapter/Core/TestCases/TestCaseFactory.cs +++ b/GoogleTestAdapter/Core/TestCases/TestCaseFactory.cs @@ -58,8 +58,11 @@ public IList CreateTestCases(Action reportTestCase = null) IList testCaseDescriptors = new ListTestsParser(_settings.TestNameSeparator).ParseListTestsOutput(standardOutput); if (_settings.ParseSymbolInformation) { - List testCaseLocations = GetTestCaseLocations(testCaseDescriptors, _settings.GetPathExtension(_executable)); - return testCaseDescriptors.Select(descriptor => CreateTestCase(descriptor, testCaseLocations)).ToList(); + var testMethods = testCaseDescriptors + .SelectMany(descriptor => _signatureCreator.GetTestMethodSignatures(descriptor).Select(s => Tuple.Create(s, descriptor))) + .ToDictionary(testCase => testCase.Item1, testCase => testCase.Item2); + var testCaseLocations = GetTestCaseLocations(testMethods.Keys, _settings.GetPathExtension(_executable)); + return testMethods.Select(testMethod => CreateTestCase(testMethod.Value, testCaseLocations[testMethod.Key])).ToList(); } return testCaseDescriptors.Select(CreateTestCase).ToList(); @@ -85,7 +88,7 @@ private IList NewCreateTestcases(Action reportTestCase, List TestCaseLocation testCaseLocation = resolver.FindTestCaseLocation( _signatureCreator.GetTestMethodSignatures(args.TestCaseDescriptor).ToList()); - testCase = CreateTestCase(args.TestCaseDescriptor, testCaseLocation.Yield().ToList()); + testCase = CreateTestCase(args.TestCaseDescriptor, testCaseLocation); } else { @@ -142,14 +145,8 @@ private bool CheckProcessExitCode(int processExitCode, ICollection stand return true; } - private List GetTestCaseLocations(IList testCaseDescriptors, string pathExtension) + private Dictionary GetTestCaseLocations(IEnumerable testMethodSignatures, string pathExtension) { - var testMethodSignatures = new List(); - foreach (TestCaseDescriptor descriptor in testCaseDescriptors) - { - testMethodSignatures.AddRange(_signatureCreator.GetTestMethodSignatures(descriptor)); - } - string filterString = "*" + GoogleTestConstants.TestBodySignature; var resolver = new TestCaseResolver(_diaResolverFactory, _logger); return resolver.ResolveAllTestCases(_executable, testMethodSignatures, filterString, pathExtension); @@ -163,11 +160,8 @@ private TestCase CreateTestCase(TestCaseDescriptor descriptor) return testCase; } - private TestCase CreateTestCase(TestCaseDescriptor descriptor, List testCaseLocations) + private TestCase CreateTestCase(TestCaseDescriptor descriptor, TestCaseLocation location) { - TestCaseLocation location = testCaseLocations.FirstOrDefault( - l => l != null && _signatureCreator.GetTestMethodSignatures(descriptor).Any(s => Regex.IsMatch(l.Symbol, s))); - if (location != null) { var testCase = new TestCase( diff --git a/GoogleTestAdapter/Core/TestCases/TestCaseResolver.cs b/GoogleTestAdapter/Core/TestCases/TestCaseResolver.cs index cbd721211..0d74a5465 100644 --- a/GoogleTestAdapter/Core/TestCases/TestCaseResolver.cs +++ b/GoogleTestAdapter/Core/TestCases/TestCaseResolver.cs @@ -24,10 +24,9 @@ internal TestCaseResolver(IDiaResolverFactory diaResolverFactory, ILogger logger _logger = logger; } - internal List ResolveAllTestCases(string executable, List testMethodSignatures, string symbolFilterString, string pathExtension) + internal Dictionary ResolveAllTestCases(string executable, IEnumerable testMethodSignatures, string symbolFilterString, string pathExtension) { - List testCaseLocationsFound = - FindTestCaseLocationsInBinary(executable, testMethodSignatures, symbolFilterString, pathExtension).ToList(); + var testCaseLocationsFound = FindTestCaseLocationsInBinary(executable, testMethodSignatures, symbolFilterString, pathExtension); if (testCaseLocationsFound.Count == 0) { @@ -41,7 +40,13 @@ internal List ResolveAllTestCases(string executable, List ResolveAllTestCases(string executable, List FindTestCaseLocationsInBinary( - string binary, List testMethodSignatures, string symbolFilterString, string pathExtension) + private Dictionary FindTestCaseLocationsInBinary( + string binary, IEnumerable testMethodSignatures, string symbolFilterString, string pathExtension) { using (IDiaResolver diaResolver = _diaResolverFactory.Create(binary, pathExtension, _logger)) { @@ -60,19 +65,28 @@ private IEnumerable FindTestCaseLocationsInBinary( IList allTraitSymbols = diaResolver.GetFunctions("*" + TraitAppendix); _logger.DebugInfo($"Found {allTestMethodSymbols.Count} test method symbols and {allTraitSymbols.Count} trait symbols in binary {binary}"); - return allTestMethodSymbols - .Where(nsfl => testMethodSignatures.Any(tms => Regex.IsMatch(nsfl.Symbol, tms))) // Contains() instead of == because nsfl might contain namespace - .Select(nsfl => ToTestCaseLocation(nsfl, allTraitSymbols)) - .ToList(); // we need to force immediate query execution, otherwise our session object will already be released + var allTestMethods = allTestMethodSymbols + .ToLookup(nsfl => StripTestSymbolNamespace(nsfl.Symbol), nsfl => ToTestCaseLocation(nsfl, allTraitSymbols)); + + return testMethodSignatures + .ToDictionary(tms => tms, tms => allTestMethods[tms].First()); } catch (Exception e) { _logger.DebugError($"Exception while resolving test locations and traits in {binary}\n{e}"); - return new TestCaseLocation[0]; + return new Dictionary(); } } } + private static string StripTestSymbolNamespace(string symbol) + { + var suffixLength = GoogleTestConstants.TestBodySignature.Length; + var namespaceEnd = symbol.LastIndexOf("::", symbol.Length - suffixLength - 1); + var nameStart = namespaceEnd >= 0 ? namespaceEnd + 2 : 0; + return symbol.Substring(nameStart); + } + private TestCaseLocation ToTestCaseLocation(SourceFileLocation location, IEnumerable allTraitSymbols) { List traits = NewTestCaseResolver.GetTraits(location, allTraitSymbols); From 56f58d7bc1ca0d8f2ba27e8a70f9c46cdb2c4afc Mon Sep 17 00:00:00 2001 From: Russell Johnston Date: Mon, 12 Jun 2017 17:35:57 -0700 Subject: [PATCH 2/4] Don't assume a test location exists This shouldn't be an issue anymore after integrating GTA 0.10, which fixes an underlying VS2017/COM bug, but without this change the adapter crashes on the second and subsequent test runs. --- GoogleTestAdapter/Core/Helpers/Extensions.cs | 6 ++++++ GoogleTestAdapter/Core/TestCases/TestCaseFactory.cs | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/GoogleTestAdapter/Core/Helpers/Extensions.cs b/GoogleTestAdapter/Core/Helpers/Extensions.cs index ba95e9161..568d07f13 100644 --- a/GoogleTestAdapter/Core/Helpers/Extensions.cs +++ b/GoogleTestAdapter/Core/Helpers/Extensions.cs @@ -37,6 +37,12 @@ internal static string AppendIfNotEmpty(this string theString, string appendix) return string.IsNullOrWhiteSpace(theString) ? theString : theString + appendix; } + internal static TV GetValue(this IDictionary dict, TK key, TV defaultValue = default(TV)) + { + TV value; + return dict.TryGetValue(key, out value) ? value : defaultValue; + } + } } \ No newline at end of file diff --git a/GoogleTestAdapter/Core/TestCases/TestCaseFactory.cs b/GoogleTestAdapter/Core/TestCases/TestCaseFactory.cs index f2d360993..c873d7cec 100644 --- a/GoogleTestAdapter/Core/TestCases/TestCaseFactory.cs +++ b/GoogleTestAdapter/Core/TestCases/TestCaseFactory.cs @@ -62,7 +62,7 @@ public IList CreateTestCases(Action reportTestCase = null) .SelectMany(descriptor => _signatureCreator.GetTestMethodSignatures(descriptor).Select(s => Tuple.Create(s, descriptor))) .ToDictionary(testCase => testCase.Item1, testCase => testCase.Item2); var testCaseLocations = GetTestCaseLocations(testMethods.Keys, _settings.GetPathExtension(_executable)); - return testMethods.Select(testMethod => CreateTestCase(testMethod.Value, testCaseLocations[testMethod.Key])).ToList(); + return testMethods.Select(testMethod => CreateTestCase(testMethod.Value, testCaseLocations.GetValue(testMethod.Key))).ToList(); } return testCaseDescriptors.Select(CreateTestCase).ToList(); From bf21ade604d8c483a82d018fd8e380877178f4c6 Mon Sep 17 00:00:00 2001 From: Russell Johnston Date: Tue, 13 Jun 2017 18:15:26 -0700 Subject: [PATCH 3/4] Fix failing tests --- GoogleTestAdapter/Core/Helpers/Extensions.cs | 6 --- .../Core/TestCases/TestCaseFactory.cs | 40 +++++++++++++++---- .../Core/TestCases/TestCaseResolver.cs | 30 +++++--------- 3 files changed, 42 insertions(+), 34 deletions(-) diff --git a/GoogleTestAdapter/Core/Helpers/Extensions.cs b/GoogleTestAdapter/Core/Helpers/Extensions.cs index 568d07f13..ba95e9161 100644 --- a/GoogleTestAdapter/Core/Helpers/Extensions.cs +++ b/GoogleTestAdapter/Core/Helpers/Extensions.cs @@ -37,12 +37,6 @@ internal static string AppendIfNotEmpty(this string theString, string appendix) return string.IsNullOrWhiteSpace(theString) ? theString : theString + appendix; } - internal static TV GetValue(this IDictionary dict, TK key, TV defaultValue = default(TV)) - { - TV value; - return dict.TryGetValue(key, out value) ? value : defaultValue; - } - } } \ No newline at end of file diff --git a/GoogleTestAdapter/Core/TestCases/TestCaseFactory.cs b/GoogleTestAdapter/Core/TestCases/TestCaseFactory.cs index c873d7cec..cdb0ec5b9 100644 --- a/GoogleTestAdapter/Core/TestCases/TestCaseFactory.cs +++ b/GoogleTestAdapter/Core/TestCases/TestCaseFactory.cs @@ -58,11 +58,8 @@ public IList CreateTestCases(Action reportTestCase = null) IList testCaseDescriptors = new ListTestsParser(_settings.TestNameSeparator).ParseListTestsOutput(standardOutput); if (_settings.ParseSymbolInformation) { - var testMethods = testCaseDescriptors - .SelectMany(descriptor => _signatureCreator.GetTestMethodSignatures(descriptor).Select(s => Tuple.Create(s, descriptor))) - .ToDictionary(testCase => testCase.Item1, testCase => testCase.Item2); - var testCaseLocations = GetTestCaseLocations(testMethods.Keys, _settings.GetPathExtension(_executable)); - return testMethods.Select(testMethod => CreateTestCase(testMethod.Value, testCaseLocations.GetValue(testMethod.Key))).ToList(); + var testCaseLocations = GetTestCaseLocations(testCaseDescriptors, _settings.GetPathExtension(_executable)); + return testCaseDescriptors.Select(descriptor => CreateTestCase(descriptor, testCaseLocations)).ToList(); } return testCaseDescriptors.Select(CreateTestCase).ToList(); @@ -145,8 +142,17 @@ private bool CheckProcessExitCode(int processExitCode, ICollection stand return true; } - private Dictionary GetTestCaseLocations(IEnumerable testMethodSignatures, string pathExtension) + private Dictionary GetTestCaseLocations(IList testCaseDescriptors, string pathExtension) { + var testMethodSignatures = new HashSet(); + foreach (var descriptor in testCaseDescriptors) + { + foreach (var signature in _signatureCreator.GetTestMethodSignatures(descriptor)) + { + testMethodSignatures.Add(signature); + } + } + string filterString = "*" + GoogleTestConstants.TestBodySignature; var resolver = new TestCaseResolver(_diaResolverFactory, _logger); return resolver.ResolveAllTestCases(_executable, testMethodSignatures, filterString, pathExtension); @@ -160,6 +166,18 @@ private TestCase CreateTestCase(TestCaseDescriptor descriptor) return testCase; } + private TestCase CreateTestCase(TestCaseDescriptor descriptor, Dictionary testCaseLocations) + { + var signature = _signatureCreator.GetTestMethodSignatures(descriptor) + .Select(StripTestSymbolNamespace) + .FirstOrDefault(s => testCaseLocations.ContainsKey(s)); + TestCaseLocation location = null; + if (signature != null) + testCaseLocations.TryGetValue(signature, out location); + + return CreateTestCase(descriptor, location); + } + private TestCase CreateTestCase(TestCaseDescriptor descriptor, TestCaseLocation location) { if (location != null) @@ -175,6 +193,14 @@ private TestCase CreateTestCase(TestCaseDescriptor descriptor, TestCaseLocation descriptor.FullyQualifiedName, _executable, descriptor.DisplayName, "", 0); } + internal static string StripTestSymbolNamespace(string symbol) + { + var suffixLength = GoogleTestConstants.TestBodySignature.Length; + var namespaceEnd = symbol.LastIndexOf("::", symbol.Length - suffixLength - 1); + var nameStart = namespaceEnd >= 0 ? namespaceEnd + 2 : 0; + return symbol.Substring(nameStart); + } + private IList GetFinalTraits(string displayName, List traits) { var afterTraits = @@ -212,4 +238,4 @@ private IList GetFinalTraits(string displayName, List traits) } -} \ No newline at end of file +} diff --git a/GoogleTestAdapter/Core/TestCases/TestCaseResolver.cs b/GoogleTestAdapter/Core/TestCases/TestCaseResolver.cs index 0d74a5465..7589ce3cc 100644 --- a/GoogleTestAdapter/Core/TestCases/TestCaseResolver.cs +++ b/GoogleTestAdapter/Core/TestCases/TestCaseResolver.cs @@ -24,7 +24,7 @@ internal TestCaseResolver(IDiaResolverFactory diaResolverFactory, ILogger logger _logger = logger; } - internal Dictionary ResolveAllTestCases(string executable, IEnumerable testMethodSignatures, string symbolFilterString, string pathExtension) + internal Dictionary ResolveAllTestCases(string executable, HashSet testMethodSignatures, string symbolFilterString, string pathExtension) { var testCaseLocationsFound = FindTestCaseLocationsInBinary(executable, testMethodSignatures, symbolFilterString, pathExtension); @@ -40,12 +40,9 @@ internal Dictionary ResolveAllTestCases(string executa string importedBinary = Path.Combine(moduleDirectory, import); if (File.Exists(importedBinary)) { - foreach (var testCase in FindTestCaseLocationsInBinary(importedBinary, testMethodSignatures, symbolFilterString, pathExtension)) + foreach (var testCaseLocation in FindTestCaseLocationsInBinary(importedBinary, testMethodSignatures, symbolFilterString, pathExtension)) { - if (testCaseLocationsFound.ContainsKey(testCase.Key)) - continue; - - testCaseLocationsFound.Add(testCase.Key, testCase.Value); + testCaseLocationsFound.Add(testCaseLocation.Key, testCaseLocation.Value); } } } @@ -55,7 +52,7 @@ internal Dictionary ResolveAllTestCases(string executa private Dictionary FindTestCaseLocationsInBinary( - string binary, IEnumerable testMethodSignatures, string symbolFilterString, string pathExtension) + string binary, HashSet testMethodSignatures, string symbolFilterString, string pathExtension) { using (IDiaResolver diaResolver = _diaResolverFactory.Create(binary, pathExtension, _logger)) { @@ -65,11 +62,10 @@ private Dictionary FindTestCaseLocationsInBinary( IList allTraitSymbols = diaResolver.GetFunctions("*" + TraitAppendix); _logger.DebugInfo($"Found {allTestMethodSymbols.Count} test method symbols and {allTraitSymbols.Count} trait symbols in binary {binary}"); - var allTestMethods = allTestMethodSymbols - .ToLookup(nsfl => StripTestSymbolNamespace(nsfl.Symbol), nsfl => ToTestCaseLocation(nsfl, allTraitSymbols)); - - return testMethodSignatures - .ToDictionary(tms => tms, tms => allTestMethods[tms].First()); + return allTestMethodSymbols + .Where(nsfl => testMethodSignatures.Contains(TestCaseFactory.StripTestSymbolNamespace(nsfl.Symbol))) + .Select(nsfl => ToTestCaseLocation(nsfl, allTraitSymbols)) + .ToDictionary(nsfl => TestCaseFactory.StripTestSymbolNamespace(nsfl.Symbol)); } catch (Exception e) { @@ -79,14 +75,6 @@ private Dictionary FindTestCaseLocationsInBinary( } } - private static string StripTestSymbolNamespace(string symbol) - { - var suffixLength = GoogleTestConstants.TestBodySignature.Length; - var namespaceEnd = symbol.LastIndexOf("::", symbol.Length - suffixLength - 1); - var nameStart = namespaceEnd >= 0 ? namespaceEnd + 2 : 0; - return symbol.Substring(nameStart); - } - private TestCaseLocation ToTestCaseLocation(SourceFileLocation location, IEnumerable allTraitSymbols) { List traits = NewTestCaseResolver.GetTraits(location, allTraitSymbols); @@ -97,4 +85,4 @@ private TestCaseLocation ToTestCaseLocation(SourceFileLocation location, IEnumer } -} \ No newline at end of file +} From b100656495ec5ac716e069f0a0058c6fc32121ff Mon Sep 17 00:00:00 2001 From: Russell Johnston Date: Mon, 24 Jul 2017 12:49:43 -0700 Subject: [PATCH 4/4] Add Microsoft headers to modified files --- GoogleTestAdapter/Core/Helpers/Extensions.cs | 4 +++- GoogleTestAdapter/Core/TestCases/TestCaseFactory.cs | 4 +++- GoogleTestAdapter/Core/TestCases/TestCaseResolver.cs | 4 +++- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/GoogleTestAdapter/Core/Helpers/Extensions.cs b/GoogleTestAdapter/Core/Helpers/Extensions.cs index ba95e9161..71f3f4339 100644 --- a/GoogleTestAdapter/Core/Helpers/Extensions.cs +++ b/GoogleTestAdapter/Core/Helpers/Extensions.cs @@ -1,4 +1,6 @@ -using System.Collections.Generic; +// This file has been modified by Microsoft on 7/2017. + +using System.Collections.Generic; using GoogleTestAdapter.Model; namespace GoogleTestAdapter.Helpers diff --git a/GoogleTestAdapter/Core/TestCases/TestCaseFactory.cs b/GoogleTestAdapter/Core/TestCases/TestCaseFactory.cs index 3a5000d8b..fb15aa08d 100644 --- a/GoogleTestAdapter/Core/TestCases/TestCaseFactory.cs +++ b/GoogleTestAdapter/Core/TestCases/TestCaseFactory.cs @@ -1,4 +1,6 @@ -using System; +// This file has been modified by Microsoft on 7/2017. + +using System; using System.Collections.Generic; using System.IO; using System.Linq; diff --git a/GoogleTestAdapter/Core/TestCases/TestCaseResolver.cs b/GoogleTestAdapter/Core/TestCases/TestCaseResolver.cs index 7589ce3cc..3c4f8ba14 100644 --- a/GoogleTestAdapter/Core/TestCases/TestCaseResolver.cs +++ b/GoogleTestAdapter/Core/TestCases/TestCaseResolver.cs @@ -1,4 +1,6 @@ -using System; +// This file has been modified by Microsoft on 7/2017. + +using System; using System.Collections.Generic; using System.IO; using System.Linq;