From c18cf61ed5ff66c6b32a3a39e390ce8da6962f14 Mon Sep 17 00:00:00 2001 From: jnm2 Date: Tue, 20 Feb 2018 19:11:03 -0500 Subject: [PATCH 1/4] Enable trait tests on netcoreapp1.0 --- src/NUnitTestAdapterTests/TraitsTests.cs | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/NUnitTestAdapterTests/TraitsTests.cs b/src/NUnitTestAdapterTests/TraitsTests.cs index 6d2b1b0c..d64a0220 100644 --- a/src/NUnitTestAdapterTests/TraitsTests.cs +++ b/src/NUnitTestAdapterTests/TraitsTests.cs @@ -366,8 +366,6 @@ public class TestDataForTraits } -#if !NETCOREAPP1_0 - [Category(nameof(TestTraits))] public class TestTraits { @@ -540,5 +538,4 @@ private void VerifyCategoriesOnly(TestCase testcase, int expectedCategories, str } } -#endif } From 01969b4cb1ff35c06c65d7db573a9d9f3eec56d3 Mon Sep 17 00:00:00 2001 From: jnm2 Date: Tue, 20 Feb 2018 22:29:03 -0500 Subject: [PATCH 2/4] Isolate parsing, caching, combining, and applying of test properties --- src/NUnitTestAdapter/CategoryList.cs | 89 ------------------- .../NUnitTestCaseProperties.cs | 37 ++++++++ src/NUnitTestAdapter/TestConverter.cs | 10 +-- src/NUnitTestAdapter/TestTraitInfo.cs | 87 ++++++++++++++++++ src/NUnitTestAdapter/TfsTestFilter.cs | 2 +- src/NUnitTestAdapter/TraitsFeature.cs | 78 +++++++++------- .../TestConverterTests.cs | 51 ++++------- .../VsExperimentalTests.cs | 12 ++- 8 files changed, 200 insertions(+), 166 deletions(-) delete mode 100644 src/NUnitTestAdapter/CategoryList.cs create mode 100644 src/NUnitTestAdapter/NUnitTestCaseProperties.cs create mode 100644 src/NUnitTestAdapter/TestTraitInfo.cs diff --git a/src/NUnitTestAdapter/CategoryList.cs b/src/NUnitTestAdapter/CategoryList.cs deleted file mode 100644 index d994ff69..00000000 --- a/src/NUnitTestAdapter/CategoryList.cs +++ /dev/null @@ -1,89 +0,0 @@ -using System; -using System.Collections.Generic; -using System.Linq; -using System.Xml; -using Microsoft.VisualStudio.TestPlatform.ObjectModel; - -namespace NUnit.VisualStudio.TestAdapter -{ - public class CategoryList - { - public const string NUnitCategoryName = "NUnit.TestCategory"; - private const string NunitTestCategoryLabel = "Category"; - private const string VsTestCategoryLabel = "TestCategory"; - internal static readonly TestProperty NUnitTestCategoryProperty = TestProperty.Register(NUnitCategoryName, VsTestCategoryLabel, typeof(string[]), TestPropertyAttributes.Hidden | TestPropertyAttributes.Trait, typeof(TestCase)); - - private readonly List categorylist = new List(); - private readonly TestCase testCase; - - public CategoryList(TestCase testCase) - { - this.testCase = testCase; - } - - public void AddRange(IEnumerable categories) - { - categorylist.AddRange(categories); - } - - public int LastNodeListCount { get; private set; } - public IEnumerable ProcessTestCaseProperties(XmlNode testNode, bool addToCache, string key = null, IDictionary> traitsCache = null) - { - var nodelist = testNode.SelectNodes("properties/property"); - LastNodeListCount = nodelist.Count; - foreach (XmlNode propertyNode in nodelist) - { - string propertyName = propertyNode.GetAttribute("name"); - string propertyValue = propertyNode.GetAttribute("value"); - if (addToCache) - AddTraitsToCache(traitsCache, key, propertyName, propertyValue); - if (!IsInternalProperty(propertyName, propertyValue)) - { - if (propertyName != NunitTestCategoryLabel) - testCase.Traits.Add(new Trait(propertyName, propertyValue)); - else - { - categorylist.Add(propertyValue); - } - } - } - return categorylist; - } - - private static bool IsInternalProperty(string propertyName, string propertyValue) - { - // Property names starting with '_' are for internal use only - return String.IsNullOrEmpty(propertyName) || propertyName[0] == '_' || String.IsNullOrEmpty(propertyValue); - } - - private static void AddTraitsToCache(IDictionary> traitsCache, string key, string propertyName, string propertyValue) - { - if (traitsCache.ContainsKey(key)) - { - if (!IsInternalProperty(propertyName, propertyValue)) - traitsCache[key].Add(new Trait(propertyName, propertyValue)); - return; - } - - var traits = new List(); - - // Will add empty list of traits, if the property is internal type. So that we will not make SelectNodes call again. - if (!IsInternalProperty(propertyName, propertyValue)) - { - traits.Add(new Trait(propertyName, propertyValue)); - } - traitsCache[key] = traits; - } - - public void UpdateCategoriesToVs() - { - if (categorylist.Any()) - { - testCase.SetPropertyValue(NUnitTestCategoryProperty, categorylist.Distinct().ToArray()); - } - } - - - - } -} \ No newline at end of file diff --git a/src/NUnitTestAdapter/NUnitTestCaseProperties.cs b/src/NUnitTestAdapter/NUnitTestCaseProperties.cs new file mode 100644 index 00000000..71d94d4a --- /dev/null +++ b/src/NUnitTestAdapter/NUnitTestCaseProperties.cs @@ -0,0 +1,37 @@ +// *********************************************************************** +// Copyright (c) 2018 Charlie Poole, Terje Sandstrom +// +// Permission is hereby granted, free of charge, to any person obtaining +// a copy of this software and associated documentation files (the +// "Software"), to deal in the Software without restriction, including +// without limitation the rights to use, copy, modify, merge, publish, +// distribute, sublicense, and/or sell copies of the Software, and to +// permit persons to whom the Software is furnished to do so, subject to +// the following conditions: +// +// The above copyright notice and this permission notice shall be +// included in all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, +// EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF +// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND +// NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE +// LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION +// OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION +// WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. +// *********************************************************************** + +using Microsoft.VisualStudio.TestPlatform.ObjectModel; + +namespace NUnit.VisualStudio.TestAdapter +{ + internal static class NUnitTestCaseProperties + { + public static readonly TestProperty TestCategory = TestProperty.Register( + id: "NUnit.TestCategory", + label: "TestCategory", + valueType: typeof(string[]), + attributes: TestPropertyAttributes.Hidden | TestPropertyAttributes.Trait, + owner: typeof(TestCase)); + } +} diff --git a/src/NUnitTestAdapter/TestConverter.cs b/src/NUnitTestAdapter/TestConverter.cs index edf25eaf..6f2b78fe 100644 --- a/src/NUnitTestAdapter/TestConverter.cs +++ b/src/NUnitTestAdapter/TestConverter.cs @@ -8,10 +8,10 @@ // distribute, sublicense, and/or sell copies of the Software, and to // permit persons to whom the Software is furnished to do so, subject to // the following conditions: -// +// // The above copyright notice and this permission notice shall be // included in all copies or substantial portions of the Software. -// +// // THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, // EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF // MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND @@ -46,7 +46,7 @@ public TestConverter(ITestLogger logger, string sourceAssembly, bool collectSour _sourceAssembly = sourceAssembly; _vsTestCaseMap = new Dictionary(); _collectSourceInformation = collectSourceInformation; - TraitsCache = new Dictionary>(); + TraitsCache = new Dictionary(); if (_collectSourceInformation) { @@ -59,7 +59,7 @@ public void Dispose() _navigationDataProvider?.Dispose(); } - public IDictionary> TraitsCache { get; } + public IDictionary TraitsCache { get; } #region Public Methods /// @@ -161,7 +161,7 @@ private TestCase MakeTestCaseFromXmlNode(XmlNode testNode) } } - testCase.AddTraitsFromTestNode(testNode, TraitsCache,_logger); + testCase.AddTraitsFromTestNode(testNode, TraitsCache); return testCase; } diff --git a/src/NUnitTestAdapter/TestTraitInfo.cs b/src/NUnitTestAdapter/TestTraitInfo.cs new file mode 100644 index 00000000..aa08e4e6 --- /dev/null +++ b/src/NUnitTestAdapter/TestTraitInfo.cs @@ -0,0 +1,87 @@ +// *********************************************************************** +// Copyright (c) 2013-2018 Charlie Poole, Terje Sandstrom +// +// Permission is hereby granted, free of charge, to any person obtaining +// a copy of this software and associated documentation files (the +// "Software"), to deal in the Software without restriction, including +// without limitation the rights to use, copy, modify, merge, publish, +// distribute, sublicense, and/or sell copies of the Software, and to +// permit persons to whom the Software is furnished to do so, subject to +// the following conditions: +// +// The above copyright notice and this permission notice shall be +// included in all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, +// EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF +// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND +// NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE +// LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION +// OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION +// WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. +// *********************************************************************** + +using System; +using System.Collections.Generic; +using Microsoft.VisualStudio.TestPlatform.ObjectModel; + +namespace NUnit.VisualStudio.TestAdapter +{ + public struct TestTraitInfo + { + public Trait[] Traits { get; } + public string[] Categories { get; } + + public TestTraitInfo(Trait[] traits, string[] categories) + { + Traits = traits; + Categories = categories; + } + + public void ApplyTo(TestCase testCase) + { + if (testCase == null) throw new ArgumentNullException(nameof(testCase)); + + if (Traits != null) testCase.Traits.AddRange(Traits); + if (Categories != null) testCase.SetPropertyValue(NUnitTestCaseProperties.TestCategory, Categories); + } + + public static TestTraitInfo Combine(TestTraitInfo inherited, TestTraitInfo current) + { + return new TestTraitInfo( + CombineTraits(inherited.Traits, current.Traits), + CombineCategories(inherited.Categories, current.Categories)); + } + + private static Trait[] CombineTraits(Trait[] inherited, Trait[] current) + { + if (inherited == null) return current; + if (current == null) return inherited; + if (inherited.Length == 0) return current; + if (current.Length == 0) return inherited; + + var combined = new Trait[inherited.Length + current.Length]; + inherited.CopyTo(combined, 0); + current.CopyTo(combined, inherited.Length); + + return combined; + } + + private static string[] CombineCategories(string[] inherited, string[] current) + { + if (inherited == null) return current; + if (current == null) return inherited; + if (inherited.Length == 0) return current; + if (current.Length == 0) return inherited; + + var combined = new List(inherited.Length + current.Length); + combined.AddRange(inherited); + + foreach (var category in current) + if (!combined.Contains(category)) + combined.Add(category); + + return combined.ToArray(); + } + } +} diff --git a/src/NUnitTestAdapter/TfsTestFilter.cs b/src/NUnitTestAdapter/TfsTestFilter.cs index 8d60e7cb..dc2b33e5 100644 --- a/src/NUnitTestAdapter/TfsTestFilter.cs +++ b/src/NUnitTestAdapter/TfsTestFilter.cs @@ -59,7 +59,7 @@ static TfsTestFilter() SupportedPropertiesCache = new Dictionary(StringComparer.OrdinalIgnoreCase); SupportedPropertiesCache["FullyQualifiedName"] = TestCaseProperties.FullyQualifiedName; SupportedPropertiesCache["Name"] = TestCaseProperties.DisplayName; - SupportedPropertiesCache["TestCategory"] = CategoryList.NUnitTestCategoryProperty; + SupportedPropertiesCache["TestCategory"] = NUnitTestCaseProperties.TestCategory; // Initialize the trait cache var priorityTrait = new NTrait("Priority", ""); var categoryTrait = new NTrait("Category", ""); diff --git a/src/NUnitTestAdapter/TraitsFeature.cs b/src/NUnitTestAdapter/TraitsFeature.cs index f560fd68..6dae3ba2 100644 --- a/src/NUnitTestAdapter/TraitsFeature.cs +++ b/src/NUnitTestAdapter/TraitsFeature.cs @@ -8,10 +8,10 @@ // distribute, sublicense, and/or sell copies of the Software, and to // permit persons to whom the Software is furnished to do so, subject to // the following conditions: -// +// // The above copyright notice and this permission notice shall be // included in all copies or substantial portions of the Software. -// +// // THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, // EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF // MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND @@ -34,44 +34,62 @@ public static void AddTrait(this TestCase testCase, string name, string value) { testCase?.Traits.Add(new Trait(name, value)); } - private const string NunitTestCategoryLabel = "Category"; - - - public static void AddTraitsFromTestNode(this TestCase testCase, XmlNode testNode, - IDictionary> traitsCache, ITestLogger logger) + + + public static void AddTraitsFromTestNode(this TestCase testCase, XmlNode testNode, IDictionary traitsCache) { - var ancestor = testNode.ParentNode; - var key = ancestor.Attributes?["id"]?.Value; - var categorylist = new CategoryList(testCase); - // Reading ancestor properties of a test-case node. And adding to the cache. - while (ancestor != null && key != null) + var combinedTraitInfo = BuildTestTraitInfo(traitsCache, testNode); + + combinedTraitInfo.ApplyTo(testCase); + } + + private static TestTraitInfo BuildTestTraitInfo(IDictionary traitsCache, XmlNode testNode) + { + var currentNodeInfo = ParseNodeTraits(testNode); + + var parentId = testNode.ParentNode?.Attributes?["id"]?.Value; + if (parentId == null) return currentNodeInfo; + + if (!traitsCache.TryGetValue(parentId, out var combinedAncestorInfo)) + traitsCache.Add(parentId, combinedAncestorInfo = BuildTestTraitInfo(traitsCache, testNode.ParentNode)); + + return TestTraitInfo.Combine(combinedAncestorInfo, currentNodeInfo); + } + + private static TestTraitInfo ParseNodeTraits(XmlNode testNode) + { + var nodelist = testNode.SelectNodes("properties/property"); + + var traits = new List(); + var categories = new List(); + + foreach (XmlNode propertyNode in nodelist) { - if (traitsCache.ContainsKey(key)) + string propertyName = propertyNode.GetAttribute("name"); + string propertyValue = propertyNode.GetAttribute("value"); + + if (IsInternalProperty(propertyName, propertyValue)) continue; + + if (propertyName == "Category") { - categorylist.AddRange(traitsCache[key].Where(o => o.Name == NunitTestCategoryLabel).Select(prop => prop.Value).ToList()); - var traitslist = traitsCache[key].Where(o => o.Name != NunitTestCategoryLabel).ToList(); - if (traitslist.Count > 0) - testCase.Traits.AddRange(traitslist); + if (!categories.Contains(propertyValue)) + categories.Add(propertyValue); } else { - categorylist.ProcessTestCaseProperties(ancestor,true,key,traitsCache); - // Adding empty list to dictionary, so that we will not make SelectNodes call again. - if (categorylist.LastNodeListCount == 0 && !traitsCache.ContainsKey(key)) - { - traitsCache[key] = new List(); - } + traits.Add(new Trait(propertyName, propertyValue)); } - ancestor = ancestor.ParentNode; - key = ancestor?.Attributes?["id"]?.Value; } - // No Need to store test-case properties in cache. - categorylist.ProcessTestCaseProperties(testNode,false); - categorylist.UpdateCategoriesToVs(); + return new TestTraitInfo(traits.ToArray(), categories.ToArray()); + } + + private static bool IsInternalProperty(string propertyName, string propertyValue) + { + // Property names starting with '_' are for internal use only + return string.IsNullOrEmpty(propertyName) || propertyName[0] == '_' || string.IsNullOrEmpty(propertyValue); } - public static IEnumerable GetTraits(this TestCase testCase) { var traits = new List(); @@ -85,7 +103,7 @@ public static IEnumerable GetTraits(this TestCase testCase) public static IEnumerable GetCategories(this TestCase testCase) { - var categories = testCase.GetPropertyValue(CategoryList.NUnitTestCategoryProperty) as string[]; + var categories = testCase.GetPropertyValue(NUnitTestCaseProperties.TestCategory) as string[]; return categories; } } diff --git a/src/NUnitTestAdapterTests/TestConverterTests.cs b/src/NUnitTestAdapterTests/TestConverterTests.cs index 66d3da46..c187f6cc 100644 --- a/src/NUnitTestAdapterTests/TestConverterTests.cs +++ b/src/NUnitTestAdapterTests/TestConverterTests.cs @@ -8,10 +8,10 @@ // distribute, sublicense, and/or sell copies of the Software, and to // permit persons to whom the Software is furnished to do so, subject to // the following conditions: -// +// // The above copyright notice and this permission notice shall be // included in all copies or substantial portions of the Software. -// +// // THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, // EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF // MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND @@ -67,11 +67,7 @@ public void CanMakeTestCaseFromTestWithCache() CheckTestCase(testCase); - Assert.That(testConverter.TraitsCache.Keys.Count, Is.EqualTo(1)); - Assert.That(testConverter.TraitsCache["121"].Count, Is.EqualTo(1)); - var parentTrait = testConverter.TraitsCache["121"]; - Assert.That(parentTrait[0].Name, Is.EqualTo("Category")); - Assert.That(parentTrait[0].Value, Is.EqualTo("super")); + CheckNodeProperties(testConverter.TraitsCache, "121", categories: new[] { "super" }); } [Test] @@ -92,22 +88,22 @@ public void CanMakeTestCaseShouldBuildTraitsCache() // Even though ancestor doesn't have any properties. Will be storing their ids. // So that we will not make call SelectNodes call again. - CheckNodesWithNoProperties(traitsCache); + CheckNodeProperties(traitsCache, "2"); // Will not be storing leaf nodes test-case nodes in the cache. CheckNoTestCaseNodesExist(traitsCache); - // Checking assembly level attribute. - CheckNodeProperties(traitsCache, "0-1009", new[] { new KeyValuePair("Category", "AsmCat") }); + // Checking assembly and namespace level attributes. + CheckNodeProperties(traitsCache, "0-1009", categories: new[] { "AsmCat" }); + CheckNodeProperties(traitsCache, "0-1010", categories: new[] { "AsmCat" }); - // Checking Class level attributes base class & dervied class - CheckNodeProperties(traitsCache, "0-1000", new[] { new KeyValuePair("Category", "BaseClass") }); - CheckNodeProperties(traitsCache, "0-1002", new[] { new KeyValuePair("Category", "DerivedClass"), new KeyValuePair("Category", "BaseClass") }); + // Checking Class level attributes base class & derived class + CheckNodeProperties(traitsCache, "0-1000", categories: new[] { "AsmCat", "BaseClass" }); + CheckNodeProperties(traitsCache, "0-1002", categories: new[] { "AsmCat", "DerivedClass", "BaseClass" }); // Checking Nested class attributes. - CheckNodeProperties(traitsCache, "0-1005", new[] { new KeyValuePair("Category", "NS1") }); - CheckNodeProperties(traitsCache, "0-1007", new[] { new KeyValuePair("Category", "NS2") }); - + CheckNodeProperties(traitsCache, "0-1005", categories: new[] { "AsmCat", "NS1" }); + CheckNodeProperties(traitsCache, "0-1007", categories: new[] { "AsmCat", "NS2" }); } [Test] @@ -164,13 +160,7 @@ private void CheckTestCase(TestCase testCase) Assert.That(testCase.GetCategories(),Is.EquivalentTo(new [] { "super", "cat1", })); } - private void CheckNodesWithNoProperties(IDictionary> traits) - { - Assert.That(traits["2"].Count, Is.EqualTo(0)); - Assert.That(traits["0-1010"].Count, Is.EqualTo(0)); - } - - private void CheckNoTestCaseNodesExist(IDictionary> traits) + private void CheckNoTestCaseNodesExist(IDictionary traits) { Assert.That(!traits.ContainsKey("0-1008")); Assert.That(!traits.ContainsKey("0-1006")); @@ -179,18 +169,11 @@ private void CheckNoTestCaseNodesExist(IDictionary> traits) Assert.That(!traits.ContainsKey("0-1001")); } - private void CheckNodeProperties(IDictionary> traitssCache, string id, KeyValuePair[] kps) + private void CheckNodeProperties(IDictionary traitsCache, string id, IEnumerable> traits = null, IEnumerable categories = null) { - Assert.That(traitssCache.ContainsKey(id)); - Assert.That(traitssCache[id].Count, Is.EqualTo(kps.Count())); - var traits = traitssCache[id]; - - foreach(var kp in kps) - { - Assert.That(traits.Any(t => t.Name == kp.Key && t.Value == kp.Value)); - } + Assert.That(traitsCache, Contains.Key(id)); + Assert.That(traitsCache[id], Has.Property("Traits").EquivalentTo(traits ?? Enumerable.Empty>())); + Assert.That(traitsCache[id], Has.Property("Categories").EquivalentTo(categories ?? Enumerable.Empty())); } - - } } diff --git a/src/NUnitTestAdapterTests/VsExperimentalTests.cs b/src/NUnitTestAdapterTests/VsExperimentalTests.cs index 041c390f..900dae64 100644 --- a/src/NUnitTestAdapterTests/VsExperimentalTests.cs +++ b/src/NUnitTestAdapterTests/VsExperimentalTests.cs @@ -8,10 +8,10 @@ // distribute, sublicense, and/or sell copies of the Software, and to // permit persons to whom the Software is furnished to do so, subject to // the following conditions: -// +// // The above copyright notice and this permission notice shall be // included in all copies or substantial portions of the Software. -// +// // THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, // EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF // MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND @@ -48,13 +48,11 @@ public void ThatCategoriesAreDistinct() CodeFilePath = null, LineNumber = 0 }; - var cl = new CategoryList(testCase); - cl.AddRange(new List {"one","one","two","two"}); - cl.UpdateCategoriesToVs(); + var traitInfo = new TestTraitInfo(traits: null, categories: new[] { "one", "one", "two", "two" }); + traitInfo.ApplyTo(testCase); var returnedCategoryList = testCase.GetCategories(); - Assert.That(returnedCategoryList.Count(),Is.EqualTo(2),$"Found {testCase.GetCategories().Count()} category entries"); - + Assert.That(returnedCategoryList.Count(), Is.EqualTo(4), $"Found {testCase.GetCategories().Count()} category entries"); } } } From e521ef3e076bc275f3e5fd3963ed402b5fd0b97c Mon Sep 17 00:00:00 2001 From: jnm2 Date: Tue, 20 Feb 2018 23:02:06 -0500 Subject: [PATCH 3/4] Refactor TestCase testing extension methods --- src/NUnitTestAdapter/TestTraitInfo.cs | 10 ++++ src/NUnitTestAdapter/TfsTestFilter.cs | 22 ++++---- src/NUnitTestAdapter/TraitsFeature.cs | 36 ------------- src/NUnitTestAdapterTests/Extensions.cs | 45 ++++++++++++++++ .../TestConverterTests.cs | 26 +++++++--- src/NUnitTestAdapterTests/TraitComparer.cs | 51 +++++++++++++++++++ 6 files changed, 137 insertions(+), 53 deletions(-) create mode 100644 src/NUnitTestAdapterTests/Extensions.cs create mode 100644 src/NUnitTestAdapterTests/TraitComparer.cs diff --git a/src/NUnitTestAdapter/TestTraitInfo.cs b/src/NUnitTestAdapter/TestTraitInfo.cs index aa08e4e6..a305ac88 100644 --- a/src/NUnitTestAdapter/TestTraitInfo.cs +++ b/src/NUnitTestAdapter/TestTraitInfo.cs @@ -23,6 +23,7 @@ using System; using System.Collections.Generic; +using System.Linq; using Microsoft.VisualStudio.TestPlatform.ObjectModel; namespace NUnit.VisualStudio.TestAdapter @@ -46,6 +47,15 @@ public void ApplyTo(TestCase testCase) if (Categories != null) testCase.SetPropertyValue(NUnitTestCaseProperties.TestCategory, Categories); } + public static TestTraitInfo FromTestCase(TestCase testCase) + { + if (testCase == null) throw new ArgumentNullException(nameof(testCase)); + + return new TestTraitInfo( + testCase.Traits.ToArray(), + (string[])testCase.GetPropertyValue(NUnitTestCaseProperties.TestCategory)); + } + public static TestTraitInfo Combine(TestTraitInfo inherited, TestTraitInfo current) { return new TestTraitInfo( diff --git a/src/NUnitTestAdapter/TfsTestFilter.cs b/src/NUnitTestAdapter/TfsTestFilter.cs index dc2b33e5..79c163bd 100644 --- a/src/NUnitTestAdapter/TfsTestFilter.cs +++ b/src/NUnitTestAdapter/TfsTestFilter.cs @@ -49,8 +49,8 @@ public class TfsTestFilter : ITfsTestFilter /// private static readonly Dictionary SupportedPropertiesCache; - private static readonly Dictionary SupportedTraitCache; - private static readonly Dictionary TraitPropertyMap; + private static readonly Dictionary SupportedTraitCache; + private static readonly Dictionary TraitPropertyMap; private static readonly List SupportedProperties; static TfsTestFilter() @@ -61,13 +61,13 @@ static TfsTestFilter() SupportedPropertiesCache["Name"] = TestCaseProperties.DisplayName; SupportedPropertiesCache["TestCategory"] = NUnitTestCaseProperties.TestCategory; // Initialize the trait cache - var priorityTrait = new NTrait("Priority", ""); - var categoryTrait = new NTrait("Category", ""); - SupportedTraitCache = new Dictionary(StringComparer.OrdinalIgnoreCase); + var priorityTrait = new Trait("Priority", ""); + var categoryTrait = new Trait("Category", ""); + SupportedTraitCache = new Dictionary(StringComparer.OrdinalIgnoreCase); SupportedTraitCache["Priority"] = priorityTrait; SupportedTraitCache["TestCategory"] = categoryTrait; // Initialize the trait property map, since TFS doesnt know about traits - TraitPropertyMap = new Dictionary(new NTraitNameComparer()); + TraitPropertyMap = new Dictionary(new TraitNameComparer()); var priorityProperty = TestProperty.Find("Priority") ?? TestProperty.Register("Priority", "Priority", typeof(string), typeof(TestCase)); TraitPropertyMap[priorityTrait] = priorityProperty; @@ -189,9 +189,9 @@ public static TestProperty PropertyProvider(string propertyName) return null; } - public static NTrait TraitProvider(string traitName) + public static Trait TraitProvider(string traitName) { - NTrait testTrait; + Trait testTrait; SupportedTraitCache.TryGetValue(traitName, out testTrait); return testTrait; } @@ -200,15 +200,15 @@ public static NTrait TraitProvider(string traitName) } - public class NTraitNameComparer : IEqualityComparer + public class TraitNameComparer : IEqualityComparer { - public bool Equals(NTrait n, NTrait y) + public bool Equals(Trait n, Trait y) { return n.Name == y.Name; } - public int GetHashCode(NTrait obj) + public int GetHashCode(Trait obj) { return obj.Name.GetHashCode(); } diff --git a/src/NUnitTestAdapter/TraitsFeature.cs b/src/NUnitTestAdapter/TraitsFeature.cs index 6dae3ba2..1a2c0ed5 100644 --- a/src/NUnitTestAdapter/TraitsFeature.cs +++ b/src/NUnitTestAdapter/TraitsFeature.cs @@ -22,7 +22,6 @@ // *********************************************************************** using System.Collections.Generic; -using System.Linq; using System.Xml; using Microsoft.VisualStudio.TestPlatform.ObjectModel; @@ -30,12 +29,6 @@ namespace NUnit.VisualStudio.TestAdapter { public static class TraitsFeature { - public static void AddTrait(this TestCase testCase, string name, string value) - { - testCase?.Traits.Add(new Trait(name, value)); - } - - public static void AddTraitsFromTestNode(this TestCase testCase, XmlNode testNode, IDictionary traitsCache) { var combinedTraitInfo = BuildTestTraitInfo(traitsCache, testNode); @@ -89,34 +82,5 @@ private static bool IsInternalProperty(string propertyName, string propertyValue // Property names starting with '_' are for internal use only return string.IsNullOrEmpty(propertyName) || propertyName[0] == '_' || string.IsNullOrEmpty(propertyValue); } - - public static IEnumerable GetTraits(this TestCase testCase) - { - var traits = new List(); - - if (testCase?.Traits != null) - { - traits.AddRange(from trait in testCase.Traits let name = trait.Name let value = trait.Value select new NTrait(name, value)); - } - return traits; - } - - public static IEnumerable GetCategories(this TestCase testCase) - { - var categories = testCase.GetPropertyValue(NUnitTestCaseProperties.TestCategory) as string[]; - return categories; - } - } - - public class NTrait - { - public string Name { get; } - public string Value { get; } - - public NTrait(string name, string value) - { - Name = name; - Value = value; - } } } diff --git a/src/NUnitTestAdapterTests/Extensions.cs b/src/NUnitTestAdapterTests/Extensions.cs new file mode 100644 index 00000000..389ea3d5 --- /dev/null +++ b/src/NUnitTestAdapterTests/Extensions.cs @@ -0,0 +1,45 @@ +// *********************************************************************** +// Copyright (c) 2018 Charlie Poole, Terje Sandstrom +// +// Permission is hereby granted, free of charge, to any person obtaining +// a copy of this software and associated documentation files (the +// "Software"), to deal in the Software without restriction, including +// without limitation the rights to use, copy, modify, merge, publish, +// distribute, sublicense, and/or sell copies of the Software, and to +// permit persons to whom the Software is furnished to do so, subject to +// the following conditions: +// +// The above copyright notice and this permission notice shall be +// included in all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, +// EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF +// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND +// NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE +// LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION +// OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION +// WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. +// *********************************************************************** + +using Microsoft.VisualStudio.TestPlatform.ObjectModel; +using System; + +namespace NUnit.VisualStudio.TestAdapter.Tests +{ + public static class Extensions + { + public static void AddTrait(this TestCase testCase, string name, string value) + { + if (testCase == null) throw new ArgumentNullException(nameof(testCase)); + + testCase.Traits.Add(new Trait(name, value)); + } + + public static string[] GetCategories(this TestCase testCase) + { + if (testCase == null) throw new ArgumentNullException(nameof(testCase)); + + return TestTraitInfo.FromTestCase(testCase).Categories; + } + } +} diff --git a/src/NUnitTestAdapterTests/TestConverterTests.cs b/src/NUnitTestAdapterTests/TestConverterTests.cs index c187f6cc..b81287ba 100644 --- a/src/NUnitTestAdapterTests/TestConverterTests.cs +++ b/src/NUnitTestAdapterTests/TestConverterTests.cs @@ -155,9 +155,9 @@ private void CheckTestCase(TestCase testCase) Assert.That(testCase.LineNumber, Is.EqualTo(FakeTestData.LineNumber)); } - var traitList = testCase.GetTraits().Select(trait => trait.Name + ":" + trait.Value).ToList(); - Assert.That(traitList, Is.EquivalentTo(new[] { "Priority:medium" })); - Assert.That(testCase.GetCategories(),Is.EquivalentTo(new [] { "super", "cat1", })); + CheckTraitInfo(testCase, + traits: new[] { new Trait("Priority", "medium") }, + categories: new[] { "super", "cat1" }); } private void CheckNoTestCaseNodesExist(IDictionary traits) @@ -169,11 +169,25 @@ private void CheckNoTestCaseNodesExist(IDictionary traits Assert.That(!traits.ContainsKey("0-1001")); } - private void CheckNodeProperties(IDictionary traitsCache, string id, IEnumerable> traits = null, IEnumerable categories = null) + private void CheckNodeProperties(IDictionary traitsCache, string id, IEnumerable traits = null, IEnumerable categories = null) { Assert.That(traitsCache, Contains.Key(id)); - Assert.That(traitsCache[id], Has.Property("Traits").EquivalentTo(traits ?? Enumerable.Empty>())); - Assert.That(traitsCache[id], Has.Property("Categories").EquivalentTo(categories ?? Enumerable.Empty())); + CheckTraitInfo(traitsCache[id], traits, categories); + } + + private void CheckTraitInfo(TestCase testCase, IEnumerable traits = null, IEnumerable categories = null) + { + CheckTraitInfo(TestTraitInfo.FromTestCase(testCase), traits, categories); + } + + private void CheckTraitInfo(TestTraitInfo traitInfo, IEnumerable traits = null, IEnumerable categories = null) + { + Assert.That(traitInfo, Has.Property("Traits") + .EquivalentTo(traits ?? Enumerable.Empty()) + .Using(TraitComparer.Instance)); + + Assert.That(traitInfo, Has.Property("Categories") + .EquivalentTo(categories ?? Enumerable.Empty())); } } } diff --git a/src/NUnitTestAdapterTests/TraitComparer.cs b/src/NUnitTestAdapterTests/TraitComparer.cs new file mode 100644 index 00000000..09cb49f8 --- /dev/null +++ b/src/NUnitTestAdapterTests/TraitComparer.cs @@ -0,0 +1,51 @@ +// *********************************************************************** +// Copyright (c) 2011-2018 Charlie Poole, Terje Sandstrom +// +// Permission is hereby granted, free of charge, to any person obtaining +// a copy of this software and associated documentation files (the +// "Software"), to deal in the Software without restriction, including +// without limitation the rights to use, copy, modify, merge, publish, +// distribute, sublicense, and/or sell copies of the Software, and to +// permit persons to whom the Software is furnished to do so, subject to +// the following conditions: +// +// The above copyright notice and this permission notice shall be +// included in all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, +// EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF +// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND +// NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE +// LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION +// OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION +// WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. +// *********************************************************************** + +using System.Collections.Generic; + +using Microsoft.VisualStudio.TestPlatform.ObjectModel; + +namespace NUnit.VisualStudio.TestAdapter.Tests +{ + public sealed class TraitComparer : IEqualityComparer + { + public static TraitComparer Instance { get; } = new TraitComparer(); + public TraitComparer() { } + + public bool Equals(Trait x, Trait y) + { + if (x == y) return true; + if (x == null | y == null) return false; + return x.Name == y.Name && x.Value == y.Value; + } + + public int GetHashCode(Trait obj) + { + if (obj == null) return 0; + var hashCode = 1477024672; + hashCode = hashCode * -1521134295 + EqualityComparer.Default.GetHashCode(obj.Name); + hashCode = hashCode * -1521134295 + EqualityComparer.Default.GetHashCode(obj.Value); + return hashCode; + } + } +} From 3582d1ad96928c8aea2130d18fee98bc44f3b358 Mon Sep 17 00:00:00 2001 From: jnm2 Date: Tue, 20 Feb 2018 23:11:59 -0500 Subject: [PATCH 4/4] Moved cache into TraitsFeature as instance state --- src/NUnitTestAdapter/TestConverter.cs | 7 +++---- .../{TraitsFeature.cs => TraitsProvider.cs} | 15 ++++++--------- src/NUnitTestAdapterTests/TestConverterTests.cs | 4 ++-- 3 files changed, 11 insertions(+), 15 deletions(-) rename src/NUnitTestAdapter/{TraitsFeature.cs => TraitsProvider.cs} (82%) diff --git a/src/NUnitTestAdapter/TestConverter.cs b/src/NUnitTestAdapter/TestConverter.cs index 6f2b78fe..fcfcd7d4 100644 --- a/src/NUnitTestAdapter/TestConverter.cs +++ b/src/NUnitTestAdapter/TestConverter.cs @@ -40,13 +40,14 @@ public sealed class TestConverter : IDisposable private readonly NavigationDataProvider _navigationDataProvider; private readonly bool _collectSourceInformation; + internal TraitsProvider TraitsProvider { get; } = new TraitsProvider(); + public TestConverter(ITestLogger logger, string sourceAssembly, bool collectSourceInformation) { _logger = logger; _sourceAssembly = sourceAssembly; _vsTestCaseMap = new Dictionary(); _collectSourceInformation = collectSourceInformation; - TraitsCache = new Dictionary(); if (_collectSourceInformation) { @@ -59,8 +60,6 @@ public void Dispose() _navigationDataProvider?.Dispose(); } - public IDictionary TraitsCache { get; } - #region Public Methods /// /// Converts an NUnit test into a TestCase for Visual Studio, @@ -161,7 +160,7 @@ private TestCase MakeTestCaseFromXmlNode(XmlNode testNode) } } - testCase.AddTraitsFromTestNode(testNode, TraitsCache); + TraitsProvider.GetTraitInfo(testNode).ApplyTo(testCase); return testCase; } diff --git a/src/NUnitTestAdapter/TraitsFeature.cs b/src/NUnitTestAdapter/TraitsProvider.cs similarity index 82% rename from src/NUnitTestAdapter/TraitsFeature.cs rename to src/NUnitTestAdapter/TraitsProvider.cs index 1a2c0ed5..fd1598ab 100644 --- a/src/NUnitTestAdapter/TraitsFeature.cs +++ b/src/NUnitTestAdapter/TraitsProvider.cs @@ -27,24 +27,21 @@ namespace NUnit.VisualStudio.TestAdapter { - public static class TraitsFeature + public sealed class TraitsProvider { - public static void AddTraitsFromTestNode(this TestCase testCase, XmlNode testNode, IDictionary traitsCache) - { - var combinedTraitInfo = BuildTestTraitInfo(traitsCache, testNode); + private readonly Dictionary cachedInfoByTestId = new Dictionary(); - combinedTraitInfo.ApplyTo(testCase); - } + internal IDictionary CachedInfoByTestId => cachedInfoByTestId; - private static TestTraitInfo BuildTestTraitInfo(IDictionary traitsCache, XmlNode testNode) + public TestTraitInfo GetTraitInfo(XmlNode testNode) { var currentNodeInfo = ParseNodeTraits(testNode); var parentId = testNode.ParentNode?.Attributes?["id"]?.Value; if (parentId == null) return currentNodeInfo; - if (!traitsCache.TryGetValue(parentId, out var combinedAncestorInfo)) - traitsCache.Add(parentId, combinedAncestorInfo = BuildTestTraitInfo(traitsCache, testNode.ParentNode)); + if (!cachedInfoByTestId.TryGetValue(parentId, out var combinedAncestorInfo)) + cachedInfoByTestId.Add(parentId, combinedAncestorInfo = GetTraitInfo(testNode.ParentNode)); return TestTraitInfo.Combine(combinedAncestorInfo, currentNodeInfo); } diff --git a/src/NUnitTestAdapterTests/TestConverterTests.cs b/src/NUnitTestAdapterTests/TestConverterTests.cs index b81287ba..b3f2f452 100644 --- a/src/NUnitTestAdapterTests/TestConverterTests.cs +++ b/src/NUnitTestAdapterTests/TestConverterTests.cs @@ -67,7 +67,7 @@ public void CanMakeTestCaseFromTestWithCache() CheckTestCase(testCase); - CheckNodeProperties(testConverter.TraitsCache, "121", categories: new[] { "super" }); + CheckNodeProperties(testConverter.TraitsProvider.CachedInfoByTestId, "121", categories: new[] { "super" }); } [Test] @@ -80,7 +80,7 @@ public void CanMakeTestCaseShouldBuildTraitsCache() var testCase = testConverter.ConvertTestCase(node); } - var traitsCache = testConverter.TraitsCache; + var traitsCache = testConverter.TraitsProvider.CachedInfoByTestId; // There are 12 ids in the TestXml2, but will be storing only ancestor properties. // Not the leaf node test-case properties.