Resolve issue with custom tests names and test Ids (#668)

* always use the actual method name as the TestCase.FullyQualifiedName, and set a unique Id based on the NUnit ID value.

* use type aliases, per PR feedback.  Also change to NUnit-constrraint style assert for typeof

* when discovering parameterized tests, use the "ParameterizedMethod"'s fullname instead of the possibly-customized test case's fullname.

* fix acceptance test that I broke in my last commit

.gitignore

@@ -329,4 +329,6 @@ tools/
 
 # VisualStudioCode Patch
 # Ignore all local history of files
-.history
+.history
+/Dump
+/src/NUnitTestAdapterTests/Dump

src/NUnit.TestAdapter.Tests.Acceptance/TestSourceWithCustomNames.cs

@@ -0,0 +1,130 @@
+using System.Linq;
+using NUnit.Framework;
+using NUnit.VisualStudio.TestAdapter.Tests.Acceptance.WorkspaceTools;
+
+namespace NUnit.VisualStudio.TestAdapter.Tests.Acceptance
+{
+    public sealed class TestSourceWithCustomNames : AcceptanceTests
+    {
+        private static void AddTestsCs(IsolatedWorkspace workspace)
+        {
+            workspace.AddFile("Tests.cs", @"
+                using System;
+                using System.Collections;
+                using System.Collections.Generic;
+                using NUnit.Framework;
+
+                namespace Test
+                {
+                    public class Tests
+                    {
+                        [Test]
+                        [TestCaseSource(nameof(TestCaseSourceMethod))]
+                        public void PassingTestStr(object arg)
+                        {
+                            Assert.Pass();
+                        }
+
+
+                        private static IEnumerable<TestCaseData> TestCaseSourceMethod()
+                        {
+                            yield return new TestCaseData(""Name with mismatched parenthesis 'pp:-) :-)'"").SetName(""Name with mismatched parenthesis 'pp:-) :-)'"");
+                            yield return new TestCaseData(""Name with mismatched quote '\""c'"").SetName(""Name with mismatched quote '\""c'"");
+                            
+                            // Adding a parenthesis to the end of this test name will stop the exception from throwing (e.g. $""TestName(...)"")
+                            yield return new TestCaseData(1).SetName($""TestName(..."");
+
+                            // Cannot be duplicated without a second test included that ends with a ']'
+                            yield return new TestCaseData(2).SetName($""TestName(...)]"");
+                        }
+
+                        [Test]
+                        [TestCase(typeof(IEnumerable<(string oneValue, int twoValue)>))]
+                        public void UnitTest_TestCaseWithTuple_TestIsNotExecuted(Type targetType)
+                        {
+                            Assert.That(targetType, Is.EqualTo(typeof(IEnumerable<(string oneValue, int twoValue)>)));
+                        }
+
+                        [Test, TestCaseSource(nameof(SourceA))]
+                        public void TestA((int a, int b) x, int y) { }
+                        public static IEnumerable SourceA => new[] {new TestCaseData((a: 1, b: 2), 5)};
+
+                        [Test, TestCaseSource(nameof(SourceB))]
+                        public void TestB(int y, (int a, int b) x) { }
+                        public static IEnumerable SourceB => new[] {new TestCaseData(5, (a: 1, b: 2))};
+
+                        [Test, TestCaseSource(nameof(SourceC))]
+                        public void TestC((int a, int b) x, int y) { }
+                        public static IEnumerable SourceC => new[] {new TestCaseData((a: 1, b: 2), 5).SetArgDisplayNames(""a+b"", ""y"")};
+
+                        [Test(), TestCaseSource(typeof(CaseTestData), nameof(CaseTestData.EqualsData))]
+                        public void EqualsTest(Case case1, Case case2)
+                        {
+                            Assert.AreEqual(case1, case2);
+                        }
+
+                        public class CaseTestData
+                        {
+                            public static IEnumerable EqualsData()
+                            {
+                                yield return new object[] { new Case { Name = ""case1"" }, new Case { Name = ""case1"" } };
+                            }
+                        }
+
+                        public class Case
+                        {
+                            public string Name;
+                            public override string ToString() => Name;
+                            public override bool Equals(object obj) => obj is Case other && other.Name == this.Name;
+                        }
+
+                    }
+                }");
+        }
+
+        [Test]
+        [TestCase("net47")] //test code requires ValueTuple support, so can't got to net35
+        [TestCase("netcoreapp2.1")]
+        public static void Single_target_csproj(string targetFramework)
+        {
+            var workspace = CreateWorkspace()
+                .AddProject("Test.csproj", $@"
+                    <Project Sdk='Microsoft.NET.Sdk'>
+
+                      <PropertyGroup>
+                        <TargetFramework>{targetFramework}</TargetFramework>
+                      </PropertyGroup>
+
+                      <ItemGroup>
+                        <PackageReference Include='Microsoft.NET.Test.Sdk' Version='*' />
+                        <PackageReference Include='NUnit' Version='*' />
+                        <PackageReference Include='NUnit3TestAdapter' Version='{NuGetPackageVersion}' />
+                      </ItemGroup>
+
+                    </Project>");
+
+            AddTestsCs(workspace);
+
+            workspace.MSBuild(restore: true);
+
+            var results = workspace.VSTest(new[] { $@"bin\Debug\{targetFramework}\Test.dll" });
+
+            //Total Tests =
+            //              3 from PassingTestStr/TestCaseSourceMethod
+            //              1 from UnitTest_TestCaseWithTuple_TestIsNotExecuted
+            //              1 from TestA/SourceA
+            //              1 from TestB/SourceB
+            //              1 from TestC/SourceC
+            //              2 from EqualsTest/EqualsData
+            //-------------------
+            //              9 Total Tests
+
+
+
+            Assert.That(results.Counters.Total, Is.EqualTo(9), "Total tests counter did not match expectation" );
+            Assert.That(results.Counters.Executed, Is.EqualTo(9), "Executed tests counter did not match expectation");
+            Assert.That(results.Counters.Passed, Is.EqualTo(9), "Passed tests counter did not match expectation");
+        }
+
+    }
+}

src/NUnitTestAdapter/TestConverter.cs

@@ -28,6 +28,7 @@
 using System.Text.RegularExpressions;
 using System.Xml;
 using Microsoft.VisualStudio.TestPlatform.ObjectModel;
+using Microsoft.VisualStudio.TestPlatform.ObjectModel.Utilities;
 using VSTestResult = Microsoft.VisualStudio.TestPlatform.ObjectModel.TestResult;
 
 namespace NUnit.VisualStudio.TestAdapter
@@ -159,20 +160,52 @@ public struct TestResultSet
         /// </summary>
         private TestCase MakeTestCaseFromXmlNode(XmlNode testNode)
         {
+            var fullyQualifiedName = testNode.GetAttribute("fullname");
+            var parentType = testNode.ParentNode.GetAttribute("type");
+            if (parentType == "ParameterizedMethod")
+            {
+                var parameterizedTestFullName = testNode.ParentNode.GetAttribute("fullname");
+
+                // VS expected FullyQualifiedName to be the actual class+type name,optionally with parameter types
+                // in parenthesis, but they must fit the pattern of a value returned by object.GetType().
+                // It should _not_ include custom name or param values (just their types).
+                // However, the "fullname" from NUnit's file generation is the custom name of the test, so
+                // this code must convert from one to the other.
+                // Reference: https://github.com/microsoft/vstest-docs/blob/master/RFCs/0017-Managed-TestCase-Properties.md
+
+                // Using the nUnit-provided "fullname" will cause failures at test execution time due to
+                // the FilterExpressionWrapper not being able to parse the test names passed-in as filters.
+
+                // To resolve this issue, for parameterized tests (which are the only tests that allow custom names),
+                // the parent node's "fullname" value is used instead. This is the name of the actual test method
+                // and will allow the filtering to work as expected.
+
+                if (!string.IsNullOrEmpty(parameterizedTestFullName))
+                {
+                    fullyQualifiedName = parameterizedTestFullName;
+                }
+
+            }
+
+
+            var id = testNode.GetAttribute("id");
+
             var testCase = new TestCase(
-                                     testNode.GetAttribute("fullname"),
+                                    fullyQualifiedName,
                                      new Uri(NUnitTestAdapter.ExecutorUri),
                                      _sourceAssembly)
             {
                 DisplayName = testNode.GetAttribute("name"),
                 CodeFilePath = null,
-                LineNumber = 0
+                LineNumber = 0,
+                Id = EqtHash.GuidFromString(id)
             };
 
             if (CollectSourceInformation && _navigationDataProvider != null)
             {
                 var className = testNode.GetAttribute("classname");
                 var methodName = testNode.GetAttribute("methodname");
+
                 var navData = _navigationDataProvider.GetNavigationData(className, methodName);
                 if (navData.IsValid)
                 {

src/NUnitTestAdapterTests/TestDiscoveryTests.cs

@@ -87,7 +87,7 @@ public void VerifyTestCaseCount()
         [TestCase("MockTest3", "NUnit.Tests.Assemblies.MockTestFixture.MockTest3")]
         [TestCase("MockTest4", "NUnit.Tests.Assemblies.MockTestFixture.MockTest4")]
         [TestCase("ExplicitlyRunTest", "NUnit.Tests.Assemblies.MockTestFixture.ExplicitlyRunTest")]
-        [TestCase("MethodWithParameters(9,11)", "NUnit.Tests.FixtureWithTestCases.MethodWithParameters(9,11)")]
+        [TestCase("MethodWithParameters(9,11)", "NUnit.Tests.FixtureWithTestCases.MethodWithParameters")]
         public void VerifyTestCaseIsFound(string name, string fullName)
         {
             var testCase = testCases.Find(tc => tc.DisplayName == name);