Merge pull request #471 from nunit/jnm2/filter_without_explicit

Block explicit tests for 3.10

build.cake

@@ -178,9 +178,7 @@ foreach (var (framework, vstestFramework, adapterDir) in new[] {
             VSTest(GetTestAssemblyPath(framework), settings);
         });
 
-    // Workaround for https://github.com/nunit/nunit3-vs-adapter/issues/47
-    // (presence of a filter causes explicit tests to start running)
-    const string NoNavigationTests = "TestCategory!=Navigation&TestCategory!=LongRunning";
+    const string NoNavigationTests = "TestCategory != Navigation";
 
     Task($"DotnetTest-{framework}")
         .IsDependentOn("Build")

src/NUnitTestAdapter/CategoryList.cs

@@ -13,6 +13,8 @@ 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));
 
+        internal static readonly TestProperty NUnitExplicitProperty = TestProperty.Register("NUnit.Explicit", "Explicit", typeof(bool), TestPropertyAttributes.Hidden, 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.
@@ -33,7 +35,7 @@ public void AddRange(IEnumerable<string> categories)
         }
 
         public int LastNodeListCount { get; private set; }
-        public IEnumerable<string> ProcessTestCaseProperties(XmlNode testNode, bool addToCache, string key = null, IDictionary<string, List<Trait>> traitsCache = null)
+        public IEnumerable<string> ProcessTestCaseProperties(XmlNode testNode, bool addToCache, string key = null, IDictionary<string, TraitsFeature.CachedTestCaseInfo> traitsCache = null)
         {
             var nodelist = testNode.SelectNodes("properties/property");
             LastNodeListCount = nodelist.Count;
@@ -56,12 +58,20 @@ public IEnumerable<string> ProcessTestCaseProperties(XmlNode testNode, bool addT
 
             if (testNode.Attributes?["runstate"]?.Value == "Explicit")
             {
+                // Add UI grouping “Explicit”
                 if (!testCase.Traits.Any(trait => trait.Name == ExplicitTraitName))
-                {
                     testCase.Traits.Add(new Trait(ExplicitTraitName, ExplicitTraitValue));
 
-                    if (addToCache)
-                        AddTraitsToCache(traitsCache, key, ExplicitTraitName, ExplicitTraitValue);
+                // Track whether the test is actually explicit since multiple things result in the same UI grouping
+                testCase.SetPropertyValue(NUnitExplicitProperty, true);
+
+                if (addToCache)
+                {
+                    // Add UI grouping “Explicit”
+                    AddTraitsToCache(traitsCache, key, ExplicitTraitName, ExplicitTraitValue);
+
+                    // Track whether the test is actually explicit since multiple things result in the same UI grouping
+                    GetCachedInfo(traitsCache, key).Explicit = true;
                 }
             }
 
@@ -81,23 +91,19 @@ private static bool IsInternalProperty(string propertyName, string propertyValue
             return String.IsNullOrEmpty(propertyName) || propertyName[0] == '_' || String.IsNullOrEmpty(propertyValue);
         }
 
-        private static void AddTraitsToCache(IDictionary<string, List<Trait>> traitsCache, string key, string propertyName, string propertyValue)
+        private static void AddTraitsToCache(IDictionary<string, TraitsFeature.CachedTestCaseInfo> traitsCache, string key, string propertyName, string propertyValue)
         {
-            if (traitsCache.ContainsKey(key))
-            {
-                if (!IsInternalProperty(propertyName, propertyValue))
-                    traitsCache[key].Add(new Trait(propertyName, propertyValue));
-                return;
-            }
+            if (IsInternalProperty(propertyName, propertyValue)) return;
 
-            var traits = new List<Trait>();
+            var info = GetCachedInfo(traitsCache, key);
+            info.Traits.Add(new Trait(propertyName, propertyValue));
+        }
 
-            // 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;
+        private static TraitsFeature.CachedTestCaseInfo GetCachedInfo(IDictionary<string, TraitsFeature.CachedTestCaseInfo> traitsCache, string key)
+        {
+            if (!traitsCache.TryGetValue(key, out var info))
+                traitsCache.Add(key, info = new TraitsFeature.CachedTestCaseInfo());
+            return info;
         }
 
         public void UpdateCategoriesToVs()

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<string, TestCase>();
             _collectSourceInformation = collectSourceInformation;
-            TraitsCache = new Dictionary<string, List<Trait>>();
+            TraitsCache = new Dictionary<string, TraitsFeature.CachedTestCaseInfo>();
 
             if (_collectSourceInformation)
             {
@@ -59,7 +59,7 @@ public void Dispose()
             _navigationDataProvider?.Dispose();
         }
 
-        public IDictionary<string, List<Trait>> TraitsCache { get; }
+        public IDictionary<string, TraitsFeature.CachedTestCaseInfo> TraitsCache { get; }
 
         #region Public Methods
         /// <summary>

src/NUnitTestAdapter/TfsTestFilter.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
@@ -44,7 +44,7 @@ public interface ITfsTestFilter
 
     public class TfsTestFilter : ITfsTestFilter
     {
-        /// <summary>   
+        /// <summary>
         /// Supported properties for filtering
 
         ///</summary>
@@ -104,23 +104,29 @@ public bool IsEmpty
 
         public IEnumerable<TestCase> CheckFilter(IEnumerable<TestCase> tests)
         {
+            return tests.Where(CheckFilter).ToList();
+        }
+
+        private bool CheckFilter(TestCase testCase)
+        {
+            var isExplicit = testCase.GetPropertyValue(CategoryList.NUnitExplicitProperty, false);
 
-            return TfsTestCaseFilterExpression == null ? tests : tests.Where(underTest => !TfsTestCaseFilterExpression.MatchTestCase(underTest, p => PropertyValueProvider(underTest, p)) == false).ToList();
+            return !isExplicit && TfsTestCaseFilterExpression?.MatchTestCase(testCase, p => PropertyValueProvider(testCase, p)) != false;
         }
 
-        /// <summary>    
-        /// Provides value of TestProperty corresponding to property name 'propertyName' as used in filter.    
-        /// /// Return value should be a string for single valued property or array of strings for multi valued property (e.g. TestCategory)   
-        /// /// </summary>     
+        /// <summary>
+        /// Provides value of TestProperty corresponding to property name 'propertyName' as used in filter.
+        /// Return value should be a string for single valued property or array of strings for multi valued property (e.g. TestCategory)
+        /// </summary>
         public static object PropertyValueProvider(TestCase currentTest, string propertyName)
         {
 
             var testProperty = LocalPropertyProvider(propertyName);
             if (testProperty != null)
             {
 
-                // Test case might not have defined this property. In that case GetPropertyValue()             
-                // would return default value. For filtering, if property is not defined return null.           
+                // Test case might not have defined this property. In that case GetPropertyValue()
+                // would return default value. For filtering, if property is not defined return null.
                 if (currentTest.Properties.Contains(testProperty))
                 {
                     return currentTest.GetPropertyValue(testProperty);
@@ -134,7 +140,7 @@ public static object PropertyValueProvider(TestCase currentTest, string property
                 if (val.Length == 0) return null;
                 if (val.Length == 1) // Contains a single string
                     return val[0];  // return that string
-                return val;  // otherwise return the whole array 
+                return val;  // otherwise return the whole array
             }
             return null;
         }
@@ -160,9 +166,9 @@ private static Func<TestCase, string, string[]> TraitContains()
             };
         }
 
-        /// <summary>   
-        /// Provides TestProperty for property name 'propertyName' as used in filter.    
-        /// </summary>    
+        /// <summary>
+        /// Provides TestProperty for property name 'propertyName' as used in filter.
+        /// </summary>
         public static TestProperty LocalPropertyProvider(string propertyName)
         {
             TestProperty testProperty;

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
@@ -35,10 +35,32 @@ public static void AddTrait(this TestCase testCase, string name, string value)
             testCase?.Traits.Add(new Trait(name, value));
         }
         private const string NunitTestCategoryLabel = "Category";
-
-
+
+
+        /// <summary>
+        /// Stores the information needed to initialize a <see cref="TestCase"/>
+        /// which can be inherited from an ancestor node during the conversion of test cases.
+        /// </summary>
+        public sealed class CachedTestCaseInfo
+        {
+            /// <summary>
+            /// Used to populate a test case’s <see cref="TestObject.Traits"/> collection.
+            /// Currently, the only effect this has is to add a Test Explorer grouping header
+            /// for each trait with the name “Name [Value]”, or for an empty value, “Name”.
+            /// </summary>
+            public List<Trait> Traits { get; } = new List<Trait>();
+
+            /// <summary>
+            /// Used by <see cref="TfsTestFilter"/>; does not affect the Test Explorer UI.
+            /// </summary>
+            public bool Explicit { get; set; }
+
+            // Eventually, we might split out the Categories collection and make this
+            // an immutable struct. (https://github.com/nunit/nunit3-vs-adapter/pull/457)
+        }
+
         public static void AddTraitsFromTestNode(this TestCase testCase, XmlNode testNode,
-            IDictionary<string, List<Trait>> traitsCache, ITestLogger logger)
+            IDictionary<string, CachedTestCaseInfo> traitsCache, ITestLogger logger)
         {
             var ancestor = testNode.ParentNode;
             var key = ancestor.Attributes?["id"]?.Value;
@@ -48,18 +70,22 @@ public static void AddTraitsFromTestNode(this TestCase testCase, XmlNode testNod
             {
                 if (traitsCache.ContainsKey(key))
                 {
-                    categorylist.AddRange(traitsCache[key].Where(o => o.Name == NunitTestCategoryLabel).Select(prop => prop.Value).ToList());
-                    var traitslist = traitsCache[key].Where(o => o.Name != NunitTestCategoryLabel).ToList();
+                    categorylist.AddRange(traitsCache[key].Traits.Where(o => o.Name == NunitTestCategoryLabel).Select(prop => prop.Value).ToList());
+
+                    if (traitsCache[key].Explicit)
+                        testCase.SetPropertyValue(CategoryList.NUnitExplicitProperty, true);
+
+                    var traitslist = traitsCache[key].Traits.Where(o => o.Name != NunitTestCategoryLabel).ToList();
                     if (traitslist.Count > 0)
                         testCase.Traits.AddRange(traitslist);
                 }
                 else
                 {
                     categorylist.ProcessTestCaseProperties(ancestor,true,key,traitsCache);
-                    // Adding empty list to dictionary, so that we will not make SelectNodes call again.
+                    // Adding entry to dictionary, so that we will not make SelectNodes call again.
                     if (categorylist.LastNodeListCount == 0 && !traitsCache.ContainsKey(key))
                     {
-                        traitsCache[key] = new List<Trait>();
+                        traitsCache[key] = new CachedTestCaseInfo();
                     }
                 }
                 ancestor = ancestor.ParentNode;
@@ -71,7 +97,7 @@ public static void AddTraitsFromTestNode(this TestCase testCase, XmlNode testNod
             categorylist.UpdateCategoriesToVs();
         }
 
-       
+
         public static IEnumerable<NTrait> GetTraits(this TestCase testCase)
         {
             var traits = new List<NTrait>();

src/NUnitTestAdapterTests/Filtering/FilteringTestUtils.cs

@@ -0,0 +1,78 @@
+// ***********************************************************************
+// 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 System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Reflection;
+using Microsoft.VisualStudio.TestPlatform.ObjectModel;
+using Microsoft.VisualStudio.TestPlatform.ObjectModel.Adapter;
+using NSubstitute;
+using NUnit.Framework;
+using NUnit.VisualStudio.TestAdapter.Tests.Fakes;
+
+namespace NUnit.VisualStudio.TestAdapter.Tests.Filtering
+{
+    public static class FilteringTestUtils
+    {
+        public static ITestCaseFilterExpression CreateVSTestFilterExpression(string filter)
+        {
+            var filterExpressionWrapperType = Type.GetType("Microsoft.VisualStudio.TestPlatform.Common.Filtering.FilterExpressionWrapper, Microsoft.VisualStudio.TestPlatform.Common", throwOnError: true);
+
+            var filterExpressionWrapper =
+                filterExpressionWrapperType.GetTypeInfo()
+                .GetConstructor(new[] { typeof(string) })
+                .Invoke(new object[] { filter });
+
+            return (ITestCaseFilterExpression)
+                Type.GetType("Microsoft.VisualStudio.TestPlatform.Common.Filtering.TestCaseFilterExpression, Microsoft.VisualStudio.TestPlatform.Common", throwOnError: true).GetTypeInfo()
+                .GetConstructor(new[] { filterExpressionWrapperType })
+                .Invoke(new object[] { filterExpressionWrapper });
+        }
+
+        public static TfsTestFilter CreateTestFilter(ITestCaseFilterExpression filterExpression)
+        {
+            var context = Substitute.For<IRunContext>();
+            context.GetTestCaseFilter(null, null).ReturnsForAnyArgs(filterExpression);
+            return new TfsTestFilter(context);
+        }
+
+        public static void AssertExpectedResult(ITestCaseFilterExpression filterExpression, IReadOnlyCollection<TestCase> testCases, IReadOnlyCollection<string> expectedMatchingTestNames)
+        {
+            var matchingTestCases = CreateTestFilter(filterExpression).CheckFilter(testCases);
+
+            Assert.That(matchingTestCases.Select(t => t.FullyQualifiedName), Is.EquivalentTo(expectedMatchingTestNames));
+        }
+
+        public static IReadOnlyCollection<TestCase> ConvertTestCases(string xml)
+        {
+            using (var testConverter = new TestConverter(
+                new TestLogger(new MessageLoggerStub()),
+                FakeTestData.AssemblyPath,
+                collectSourceInformation: false))
+            {
+                return testConverter.ConvertTestCases(xml);
+            }
+        }
+    }
+}