From 0bffd932d8f9d0edac167dd792534af9a9d19ab6 Mon Sep 17 00:00:00 2001 From: jnm2 Date: Fri, 2 Mar 2018 18:04:28 -0500 Subject: [PATCH 1/7] Enable trait tests to run on netcoreapp1.0 --- src/NUnitTestAdapterTests/TraitsTests.cs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/NUnitTestAdapterTests/TraitsTests.cs b/src/NUnitTestAdapterTests/TraitsTests.cs index 6d2b1b0c..616e846c 100644 --- a/src/NUnitTestAdapterTests/TraitsTests.cs +++ b/src/NUnitTestAdapterTests/TraitsTests.cs @@ -9,7 +9,6 @@ namespace NUnit.VisualStudio.TestAdapter.Tests { - public class TestDataForTraits { #region TestXml Data @@ -366,8 +365,6 @@ public class TestDataForTraits } -#if !NETCOREAPP1_0 - [Category(nameof(TestTraits))] public class TestTraits { @@ -540,5 +537,4 @@ private void VerifyCategoriesOnly(TestCase testcase, int expectedCategories, str } } -#endif } From b79f43afd4a4e07d281e0c749fc833b27f04a8ca Mon Sep 17 00:00:00 2001 From: jnm2 Date: Tue, 20 Feb 2018 23:23:01 -0500 Subject: [PATCH 2/7] Failing tests for converting Explicit to a category --- src/NUnitTestAdapterTests/TestCaseUtils.cs | 55 ++++++++++++++++++++++ src/NUnitTestAdapterTests/TraitsTests.cs | 34 ++++++++++++- 2 files changed, 88 insertions(+), 1 deletion(-) create mode 100644 src/NUnitTestAdapterTests/TestCaseUtils.cs diff --git a/src/NUnitTestAdapterTests/TestCaseUtils.cs b/src/NUnitTestAdapterTests/TestCaseUtils.cs new file mode 100644 index 00000000..1cc1b1d4 --- /dev/null +++ b/src/NUnitTestAdapterTests/TestCaseUtils.cs @@ -0,0 +1,55 @@ +// *********************************************************************** +// 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; +using System.Collections.Generic; +using System.Xml; + +namespace NUnit.VisualStudio.TestAdapter.Tests +{ + /// + /// Helper methods for testing against test cases. + /// + internal static class TestCaseUtils + { + /// + /// Knows how to convert an entire XML fragment. + /// + public static IList ConvertTestCases(this TestConverter testConverter, string xml) + { + if (testConverter == null) throw new ArgumentNullException(nameof(testConverter)); + + var fragment = new XmlDocument().CreateDocumentFragment(); + fragment.InnerXml = xml; + var testCaseNodes = fragment.SelectNodes("//test-case"); + + var testCases = new TestCase[testCaseNodes.Count]; + + for (var i = 0; i < testCases.Length; i++) + testCases[i] = testConverter.ConvertTestCase(testCaseNodes[i]); + + return testCases; + } + } +} diff --git a/src/NUnitTestAdapterTests/TraitsTests.cs b/src/NUnitTestAdapterTests/TraitsTests.cs index 616e846c..6b5d092a 100644 --- a/src/NUnitTestAdapterTests/TraitsTests.cs +++ b/src/NUnitTestAdapterTests/TraitsTests.cs @@ -5,7 +5,7 @@ using Microsoft.VisualStudio.TestPlatform.ObjectModel.Logging; using NSubstitute; using NUnit.Framework; - +using NUnit.VisualStudio.TestAdapter.Tests.Fakes; namespace NUnit.VisualStudio.TestAdapter.Tests { @@ -536,5 +536,37 @@ private void VerifyCategoriesOnly(TestCase testcase, int expectedCategories, str }); } + private static IList GetTestCases(string xml) + { + using (var converter = new TestConverter( + new TestLogger(new MessageLoggerStub()), + sourceAssembly: "unused", + collectSourceInformation: false)) + { + return converter.ConvertTestCases(xml); + } + } + + [Test] + public static void ThatExplicitTestCaseHasExplicitCategory() + { + var testCase = GetTestCases( + @" + + ").Single(); + + Assert.That(testCase.GetCategories(), Contains.Item("Explicit")); + } + + [Test] + public static void ThatTestCaseWithExplicitParentHasExplicitCategory() + { + var testCase = GetTestCases( + @" + + ").Single(); + + Assert.That(testCase.GetCategories(), Contains.Item("Explicit")); + } } } From 5a06e47aab0caaa34f3b9773a5977903c568b906 Mon Sep 17 00:00:00 2001 From: jnm2 Date: Fri, 2 Mar 2018 17:09:37 -0500 Subject: [PATCH 3/7] Added Explicit as a trait --- src/NUnitTestAdapter/CategoryList.cs | 7 +++++++ src/NUnitTestAdapterTests/TestCaseUtils.cs | 2 +- src/NUnitTestAdapterTests/TraitsTests.cs | 2 +- 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/NUnitTestAdapter/CategoryList.cs b/src/NUnitTestAdapter/CategoryList.cs index d994ff69..d62f9f49 100644 --- a/src/NUnitTestAdapter/CategoryList.cs +++ b/src/NUnitTestAdapter/CategoryList.cs @@ -47,6 +47,13 @@ public IEnumerable ProcessTestCaseProperties(XmlNode testNode, bool addT } } } + + const string explicitTraitName = "Explicit"; + if (!categorylist.Contains(explicitTraitName) && testNode.Attributes?["runstate"]?.Value == "Explicit") + { + categorylist.Add(explicitTraitName); + } + return categorylist; } diff --git a/src/NUnitTestAdapterTests/TestCaseUtils.cs b/src/NUnitTestAdapterTests/TestCaseUtils.cs index 1cc1b1d4..957b7c7e 100644 --- a/src/NUnitTestAdapterTests/TestCaseUtils.cs +++ b/src/NUnitTestAdapterTests/TestCaseUtils.cs @@ -36,7 +36,7 @@ internal static class TestCaseUtils /// /// Knows how to convert an entire XML fragment. /// - public static IList ConvertTestCases(this TestConverter testConverter, string xml) + public static IReadOnlyList ConvertTestCases(this TestConverter testConverter, string xml) { if (testConverter == null) throw new ArgumentNullException(nameof(testConverter)); diff --git a/src/NUnitTestAdapterTests/TraitsTests.cs b/src/NUnitTestAdapterTests/TraitsTests.cs index 6b5d092a..a3011311 100644 --- a/src/NUnitTestAdapterTests/TraitsTests.cs +++ b/src/NUnitTestAdapterTests/TraitsTests.cs @@ -536,7 +536,7 @@ private void VerifyCategoriesOnly(TestCase testcase, int expectedCategories, str }); } - private static IList GetTestCases(string xml) + private static IReadOnlyList GetTestCases(string xml) { using (var converter = new TestConverter( new TestLogger(new MessageLoggerStub()), From d4a6e6792143c41f8ae085284089c6a70214a824 Mon Sep 17 00:00:00 2001 From: jnm2 Date: Fri, 2 Mar 2018 18:43:08 -0500 Subject: [PATCH 4/7] Move Explicit to a plain trait so that it doesn't match TestCategory --- src/NUnitTestAdapter/CategoryList.cs | 10 ++++++++-- src/NUnitTestAdapterTests/TraitsTests.cs | 19 +++++++++++++++---- 2 files changed, 23 insertions(+), 6 deletions(-) diff --git a/src/NUnitTestAdapter/CategoryList.cs b/src/NUnitTestAdapter/CategoryList.cs index d62f9f49..dd8b00eb 100644 --- a/src/NUnitTestAdapter/CategoryList.cs +++ b/src/NUnitTestAdapter/CategoryList.cs @@ -49,9 +49,15 @@ public IEnumerable ProcessTestCaseProperties(XmlNode testNode, bool addT } const string explicitTraitName = "Explicit"; - if (!categorylist.Contains(explicitTraitName) && testNode.Attributes?["runstate"]?.Value == "Explicit") + if (testNode.Attributes?["runstate"]?.Value == "Explicit") { - categorylist.Add(explicitTraitName); + if (!testCase.Traits.Any(trait => trait.Name == explicitTraitName)) + { + // The empty string causes the UI we want. + // If it's null, the explicit trait doesn't show up in Test Explorer. + // If it's not empty, it shows up as “Explicit [value]” in Test Explorer. + testCase.Traits.Add(new Trait(explicitTraitName, value: string.Empty)); + } } return categorylist; diff --git a/src/NUnitTestAdapterTests/TraitsTests.cs b/src/NUnitTestAdapterTests/TraitsTests.cs index a3011311..89e96bff 100644 --- a/src/NUnitTestAdapterTests/TraitsTests.cs +++ b/src/NUnitTestAdapterTests/TraitsTests.cs @@ -548,25 +548,36 @@ private static IReadOnlyList GetTestCases(string xml) } [Test] - public static void ThatExplicitTestCaseHasExplicitCategory() + public static void ThatExplicitTestCaseHasExplicitTrait() { var testCase = GetTestCases( @" ").Single(); - Assert.That(testCase.GetCategories(), Contains.Item("Explicit")); + Assert.That(testCase.Traits, Has.One.With.Property("Name").EqualTo("Explicit")); } [Test] - public static void ThatTestCaseWithExplicitParentHasExplicitCategory() + public static void ThatTestCaseWithExplicitParentHasExplicitTrait() { var testCase = GetTestCases( @" ").Single(); - Assert.That(testCase.GetCategories(), Contains.Item("Explicit")); + Assert.That(testCase.Traits, Has.One.With.Property("Name").EqualTo("Explicit")); + } + + [Test] + public static void ThatExplicitTraitValueIsEmptyString() + { + var testCase = GetTestCases( + @" + + ").Single(); + + Assert.That(testCase.Traits, Has.One.With.Property("Name").EqualTo("Explicit").And.Property("Value").SameAs(string.Empty)); } } } From 5567540408f6201c60b7c2a266425e526754d8f3 Mon Sep 17 00:00:00 2001 From: jnm2 Date: Fri, 2 Mar 2018 18:58:50 -0500 Subject: [PATCH 5/7] Test, noticed only first child of parent was getting Explicit trait --- src/NUnitTestAdapterTests/TraitsTests.cs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/NUnitTestAdapterTests/TraitsTests.cs b/src/NUnitTestAdapterTests/TraitsTests.cs index 89e96bff..962ac335 100644 --- a/src/NUnitTestAdapterTests/TraitsTests.cs +++ b/src/NUnitTestAdapterTests/TraitsTests.cs @@ -569,6 +569,19 @@ public static void ThatTestCaseWithExplicitParentHasExplicitTrait() Assert.That(testCase.Traits, Has.One.With.Property("Name").EqualTo("Explicit")); } + [Test] + public static void ThatMultipleChildTestCasesWithExplicitParentHaveExplicitTraits() + { + var testCases = GetTestCases( + @" + + + "); + + foreach (var testCase in testCases) + Assert.That(testCase.Traits, Has.One.With.Property("Name").EqualTo("Explicit")); + } + [Test] public static void ThatExplicitTraitValueIsEmptyString() { From fb2aebb9d46410aea2ed6cae6eebda05f6dab802 Mon Sep 17 00:00:00 2001 From: jnm2 Date: Fri, 2 Mar 2018 18:59:39 -0500 Subject: [PATCH 6/7] Missed this, first attempt at fixing caching bug --- src/NUnitTestAdapter/CategoryList.cs | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/NUnitTestAdapter/CategoryList.cs b/src/NUnitTestAdapter/CategoryList.cs index dd8b00eb..83b72ef7 100644 --- a/src/NUnitTestAdapter/CategoryList.cs +++ b/src/NUnitTestAdapter/CategoryList.cs @@ -49,14 +49,19 @@ public IEnumerable ProcessTestCaseProperties(XmlNode testNode, bool addT } const string explicitTraitName = "Explicit"; + // The empty string causes the UI we want. + // If it's null, the explicit trait doesn't show up in Test Explorer. + // If it's not empty, it shows up as “Explicit [value]” in Test Explorer. + const string explicitTraitValue = ""; + if (testNode.Attributes?["runstate"]?.Value == "Explicit") { if (!testCase.Traits.Any(trait => trait.Name == explicitTraitName)) { - // The empty string causes the UI we want. - // If it's null, the explicit trait doesn't show up in Test Explorer. - // If it's not empty, it shows up as “Explicit [value]” in Test Explorer. - testCase.Traits.Add(new Trait(explicitTraitName, value: string.Empty)); + testCase.Traits.Add(new Trait(explicitTraitName, explicitTraitValue)); + + if (addToCache) + AddTraitsToCache(traitsCache, key, explicitTraitName, explicitTraitValue); } } From 14659745b1f0713bb85f0d9d08e62c99aa2630dd Mon Sep 17 00:00:00 2001 From: jnm2 Date: Fri, 2 Mar 2018 19:03:30 -0500 Subject: [PATCH 7/7] Fixed Explicit trait caching bug --- src/NUnitTestAdapter/CategoryList.cs | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/src/NUnitTestAdapter/CategoryList.cs b/src/NUnitTestAdapter/CategoryList.cs index 83b72ef7..1ceae098 100644 --- a/src/NUnitTestAdapter/CategoryList.cs +++ b/src/NUnitTestAdapter/CategoryList.cs @@ -13,6 +13,12 @@ public class CategoryList private const string VsTestCategoryLabel = "TestCategory"; internal static readonly TestProperty NUnitTestCategoryProperty = TestProperty.Register(NUnitCategoryName, VsTestCategoryLabel, typeof(string[]), TestPropertyAttributes.Hidden | TestPropertyAttributes.Trait, typeof(TestCase)); + private const string ExplicitTraitName = "Explicit"; + // The empty string causes the UI we want. + // If it's null, the explicit trait doesn't show up in Test Explorer. + // If it's not empty, it shows up as “Explicit [value]” in Test Explorer. + private const string ExplicitTraitValue = ""; + private readonly List categorylist = new List(); private readonly TestCase testCase; @@ -48,20 +54,14 @@ public IEnumerable ProcessTestCaseProperties(XmlNode testNode, bool addT } } - const string explicitTraitName = "Explicit"; - // The empty string causes the UI we want. - // If it's null, the explicit trait doesn't show up in Test Explorer. - // If it's not empty, it shows up as “Explicit [value]” in Test Explorer. - const string explicitTraitValue = ""; - if (testNode.Attributes?["runstate"]?.Value == "Explicit") { - if (!testCase.Traits.Any(trait => trait.Name == explicitTraitName)) + if (!testCase.Traits.Any(trait => trait.Name == ExplicitTraitName)) { - testCase.Traits.Add(new Trait(explicitTraitName, explicitTraitValue)); + testCase.Traits.Add(new Trait(ExplicitTraitName, ExplicitTraitValue)); if (addToCache) - AddTraitsToCache(traitsCache, key, explicitTraitName, explicitTraitValue); + AddTraitsToCache(traitsCache, key, ExplicitTraitName, ExplicitTraitValue); } } @@ -70,6 +70,13 @@ public IEnumerable ProcessTestCaseProperties(XmlNode testNode, bool addT private static bool IsInternalProperty(string propertyName, string propertyValue) { + if (propertyName == ExplicitTraitName) + { + // Otherwise the IsNullOrEmpty check does the wrong thing, + // but I'm not sure of the consequences of allowing all empty strings. + return false; + } + // Property names starting with '_' are for internal use only return String.IsNullOrEmpty(propertyName) || propertyName[0] == '_' || String.IsNullOrEmpty(propertyValue); }