Abstract VSTest discovery sink in PlatformServices (Phase 3 of platform-agnostic effort)

src/Adapter/MSTest.TestAdapter/VSTestAdapter/MSTestDiscoverer.cs

@@ -87,7 +87,7 @@ internal async Task DiscoverTestsAsync(IEnumerable<string> sources, IDiscoveryCo
             IAdapterMessageLogger adapterLogger = logger.ToAdapterMessageLogger();
             if (MSTestDiscovererHelpers.InitializeDiscovery(sources, discoveryContext, adapterLogger, configuration, _testSourceHandler))
             {
-                new UnitTestDiscoverer(_testSourceHandler).DiscoverTests(sources, adapterLogger, discoverySink, discoveryContext, isMTP);
+                new UnitTestDiscoverer(_testSourceHandler).DiscoverTests(sources, adapterLogger, discoverySink.ToUnitTestElementSink(), discoveryContext, isMTP);
             }
         }
         finally

src/Adapter/MSTestAdapter.PlatformServices/Discovery/UnitTestDiscoverer.cs

@@ -31,7 +31,7 @@ internal UnitTestDiscoverer(ITestSourceHandler testSourceHandler)
     internal void DiscoverTests(
         IEnumerable<string> sources,
         IAdapterMessageLogger logger,
-        ITestCaseDiscoverySink discoverySink,
+        IUnitTestElementSink discoverySink,
         IDiscoveryContext discoveryContext,
         bool isMTP)
     {
@@ -52,7 +52,7 @@ internal void DiscoverTests(
     internal virtual void DiscoverTestsInSource(
         string source,
         IAdapterMessageLogger logger,
-        ITestCaseDiscoverySink discoverySink,
+        IUnitTestElementSink discoverySink,
         IDiscoveryContext? discoveryContext,
         bool isMTP)
     {
@@ -108,7 +108,7 @@ internal virtual void DiscoverTestsInSource(
 
     private readonly ITestSourceHandler _testSource;
 
-    internal void SendTestCases(IEnumerable<UnitTestElement> testElements, ITestCaseDiscoverySink discoverySink, IDiscoveryContext? discoveryContext, IAdapterMessageLogger logger)
+    internal void SendTestCases(IEnumerable<UnitTestElement> testElements, IUnitTestElementSink discoverySink, IDiscoveryContext? discoveryContext, IAdapterMessageLogger logger)
     {
         // Get filter and skip discovery in case filter expression has parsing error.
         ITestElementFilter? filter = _testMethodFilter.GetTestElementFilter(discoveryContext, logger, out bool filterHasError);
@@ -125,7 +125,7 @@ internal void SendTestCases(IEnumerable<UnitTestElement> testElements, ITestCase
                 continue;
             }
 
-            discoverySink.SendTestCase(testElement.ToTestCase());
+            discoverySink.SendTestElement(testElement);
         }
     }
 }

src/Adapter/MSTestAdapter.PlatformServices/Execution/TestCaseDiscoverySink.cs

@@ -1,30 +1,32 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
 
+using Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.ObjectModel;
+using Microsoft.VisualStudio.TestPlatform.MSTestAdapter.PlatformServices.Interface;
 using Microsoft.VisualStudio.TestPlatform.ObjectModel;
-using Microsoft.VisualStudio.TestPlatform.ObjectModel.Adapter;
 
 namespace Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.Execution;
 
 /// <summary>
-/// The test case discovery sink.
+/// The test case discovery sink used internally by execution to collect the discovered tests.
 /// </summary>
-internal sealed class TestCaseDiscoverySink : ITestCaseDiscoverySink
+/// <remarks>
+/// It implements the platform-agnostic <see cref="IUnitTestElementSink"/> but still materializes a VSTest
+/// <see cref="TestCase"/> for every discovered element, because the execution pipeline currently consumes
+/// <see cref="TestCase"/> instances. That materialization is expected to disappear once execution flows the
+/// neutral <see cref="UnitTestElement"/> model end-to-end.
+/// </remarks>
+internal sealed class TestCaseDiscoverySink : IUnitTestElementSink
 {
     /// <summary>
     /// Gets the tests.
     /// </summary>
     public ICollection<TestCase> Tests { get; } = [];
 
     /// <summary>
-    /// Sends the test case.
+    /// Collects the discovered test, materializing it as a VSTest <see cref="TestCase"/>.
     /// </summary>
-    /// <param name="discoveredTest"> The discovered test. </param>
-    public void SendTestCase(TestCase? discoveredTest)
-    {
-        if (discoveredTest != null)
-        {
-            Tests.Add(discoveredTest);
-        }
-    }
+    /// <param name="testElement"> The discovered test element. </param>
+    public void SendTestElement(UnitTestElement testElement)
+        => Tests.Add(testElement.ToTestCase());
 }

src/Adapter/MSTestAdapter.PlatformServices/Interfaces/IUnitTestElementSink.cs

@@ -0,0 +1,28 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+
+using Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.ObjectModel;
+
+namespace Microsoft.VisualStudio.TestPlatform.MSTestAdapter.PlatformServices.Interface;
+
+/// <summary>
+/// Platform-agnostic sink that receives the tests discovered by the adapter and hands them to the
+/// discovery consumer (a test host during discovery, or the execution pipeline during a run).
+/// </summary>
+/// <remarks>
+/// This abstraction lets the platform services discovery pipeline emit the neutral
+/// <see cref="UnitTestElement"/> model without taking a dependency on a specific test platform's
+/// discovery object model (for example the VSTest <c>TestCase</c> and <c>ITestCaseDiscoverySink</c>
+/// types). The concrete sink is produced at the platform boundary by a wrapper over the host's
+/// discovery sink (currently <c>UnitTestElementSinkExtensions</c>, which wraps the VSTest
+/// <c>ITestCaseDiscoverySink</c> and materializes a <c>TestCase</c> for each element), and is expected
+/// to move fully out of the platform services layer in a later phase.
+/// </remarks>
+internal interface IUnitTestElementSink
+{
+    /// <summary>
+    /// Reports a discovered <paramref name="testElement"/> to the running test host.
+    /// </summary>
+    /// <param name="testElement">The discovered test element.</param>
+    void SendTestElement(UnitTestElement testElement);
+}

src/Adapter/MSTestAdapter.PlatformServices/Services/UnitTestElementSinkExtensions.cs

@@ -0,0 +1,42 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+
+using Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.ObjectModel;
+using Microsoft.VisualStudio.TestPlatform.MSTestAdapter.PlatformServices.Interface;
+using Microsoft.VisualStudio.TestPlatform.ObjectModel.Adapter;
+
+namespace Microsoft.VisualStudio.TestPlatform.MSTestAdapter.PlatformServices;
+
+/// <summary>
+/// Bridges a VSTest <see cref="ITestCaseDiscoverySink"/> to the platform-agnostic
+/// <see cref="IUnitTestElementSink"/>.
+/// </summary>
+/// <remarks>
+/// This is the single translation point between the neutral <see cref="UnitTestElement"/> model and the
+/// VSTest discovery object model (<c>TestCase</c>, <c>ITestCaseDiscoverySink</c>). It materializes a
+/// VSTest <c>TestCase</c> for each discovered element via <see cref="UnitTestElement.ToTestCase"/>. It is
+/// expected to move entirely into the adapter layer once discovery no longer flows VSTest discovery sinks
+/// through the platform services (see the tracking issue linked in the pull request that removes the VSTest
+/// object model from platform services).
+/// </remarks>
+internal static class UnitTestElementSinkExtensions
+{
+    /// <summary>
+    /// Wraps a VSTest <see cref="ITestCaseDiscoverySink"/> as an <see cref="IUnitTestElementSink"/>.
+    /// </summary>
+    /// <param name="discoverySink">The host discovery sink to wrap.</param>
+    /// <returns>A platform-agnostic sink that forwards to <paramref name="discoverySink"/>.</returns>
+    internal static IUnitTestElementSink ToUnitTestElementSink(this ITestCaseDiscoverySink discoverySink)
+        => new HostDiscoverySink(discoverySink ?? throw new ArgumentNullException(nameof(discoverySink)));
+
+    private sealed class HostDiscoverySink : IUnitTestElementSink
+    {
+        private readonly ITestCaseDiscoverySink _discoverySink;
+
+        public HostDiscoverySink(ITestCaseDiscoverySink discoverySink)
+            => _discoverySink = discoverySink;
+
+        public void SendTestElement(UnitTestElement testElement)
+            => _discoverySink.SendTestCase(testElement.ToTestCase());
+    }
+}

test/IntegrationTests/MSTest.IntegrationTests/Utilities/CLITestBase.discovery.cs

@@ -27,7 +27,7 @@ internal static ImmutableArray<TestCase> DiscoverTests(string assemblyPath, stri
         string runSettingsXml = GetRunSettingsXml(string.Empty);
         var context = new InternalDiscoveryContext(runSettingsXml, testCaseFilter);
 
-        unitTestDiscoverer.DiscoverTestsInSource(assemblyPath, logger.ToAdapterMessageLogger(), sink, context, false);
+        unitTestDiscoverer.DiscoverTestsInSource(assemblyPath, logger.ToAdapterMessageLogger(), sink.ToUnitTestElementSink(), context, false);
 
         return sink.DiscoveredTests;
     }

test/UnitTests/MSTestAdapter.PlatformServices.UnitTests/Discovery/UnitTestDiscovererTests.cs

@@ -80,7 +80,7 @@ public void DiscoverTestsShouldThrowOnFileNotFound()
                 .Returns(false);
         }
 
-        Action act = () => _unitTestDiscoverer.DiscoverTests(sources, _mockMessageLogger.Object.ToAdapterMessageLogger(), _mockTestCaseDiscoverySink.Object, _mockDiscoveryContext.Object, false);
+        Action act = () => _unitTestDiscoverer.DiscoverTests(sources, _mockMessageLogger.Object.ToAdapterMessageLogger(), _mockTestCaseDiscoverySink.Object.ToUnitTestElementSink(), _mockDiscoveryContext.Object, false);
         act.Should().Throw<FileNotFoundException>()
             .WithMessage(string.Format(CultureInfo.CurrentCulture, Resource.TestAssembly_FileDoesNotExist, sources[0]));
     }
@@ -93,7 +93,7 @@ public void DiscoverTestsInSourceShouldThrowOnFileNotFound()
         _testablePlatformServiceProvider.MockFileOperations.Setup(fo => fo.DoesFileExist(Source))
             .Returns(false);
 
-        Action act = () => _unitTestDiscoverer.DiscoverTestsInSource(Source, _mockMessageLogger.Object.ToAdapterMessageLogger(), _mockTestCaseDiscoverySink.Object, _mockDiscoveryContext.Object, false);
+        Action act = () => _unitTestDiscoverer.DiscoverTestsInSource(Source, _mockMessageLogger.Object.ToAdapterMessageLogger(), _mockTestCaseDiscoverySink.Object.ToUnitTestElementSink(), _mockDiscoveryContext.Object, false);
         act.Should().Throw<FileNotFoundException>()
             .WithMessage(string.Format(CultureInfo.CurrentCulture, Resource.TestAssembly_FileDoesNotExist, Source));
     }
@@ -125,7 +125,7 @@ public void DiscoverTestsInSourceShouldSendBackTestCasesDiscovered()
         MSTestSettings.PopulateSettings(_mockDiscoveryContext.Object, _mockMessageLogger.Object.ToAdapterMessageLogger(), null);
 
         // Act
-        _unitTestDiscoverer.DiscoverTestsInSource(Source, _mockMessageLogger.Object.ToAdapterMessageLogger(), _mockTestCaseDiscoverySink.Object, _mockDiscoveryContext.Object, false);
+        _unitTestDiscoverer.DiscoverTestsInSource(Source, _mockMessageLogger.Object.ToAdapterMessageLogger(), _mockTestCaseDiscoverySink.Object.ToUnitTestElementSink(), _mockDiscoveryContext.Object, false);
 
         // Assert.
         _mockTestCaseDiscoverySink.Verify(ds => ds.SendTestCase(It.IsAny<TestCase>()), Times.AtLeastOnce);
@@ -160,7 +160,7 @@ public void DiscoverTestsInSourceShouldThrowWhenTreatDiscoveryWarningsAsErrorsIs
         MSTestSettings.PopulateSettings(_mockDiscoveryContext.Object, _mockMessageLogger.Object.ToAdapterMessageLogger(), null);
 
         // Act
-        Action action = () => _unitTestDiscoverer.DiscoverTestsInSource(Source, _mockMessageLogger.Object.ToAdapterMessageLogger(), _mockTestCaseDiscoverySink.Object, _mockDiscoveryContext.Object, false);
+        Action action = () => _unitTestDiscoverer.DiscoverTestsInSource(Source, _mockMessageLogger.Object.ToAdapterMessageLogger(), _mockTestCaseDiscoverySink.Object.ToUnitTestElementSink(), _mockDiscoveryContext.Object, false);
 
         // Assert
         action.Should().Throw<MSTestException>()
@@ -199,7 +199,7 @@ public void DiscoverTestsInSourceShouldNotThrowWhenTreatDiscoveryWarningsAsError
 
         // Act & Assert
         // Should not throw an exception
-        _unitTestDiscoverer.DiscoverTestsInSource(Source, _mockMessageLogger.Object.ToAdapterMessageLogger(), _mockTestCaseDiscoverySink.Object, _mockDiscoveryContext.Object, false);
+        _unitTestDiscoverer.DiscoverTestsInSource(Source, _mockMessageLogger.Object.ToAdapterMessageLogger(), _mockTestCaseDiscoverySink.Object.ToUnitTestElementSink(), _mockDiscoveryContext.Object, false);
 
         // Verify warning message was sent to logger (not error)
         _mockMessageLogger.Verify(lm => lm.SendMessage(TestMessageLevel.Warning, It.IsAny<string>()), Times.AtLeastOnce);
@@ -209,7 +209,7 @@ public void DiscoverTestsInSourceShouldNotThrowWhenTreatDiscoveryWarningsAsError
     public void SendTestCasesShouldNotSendAnyTestCasesIfThereAreNoTestElements()
     {
         // There is a null check for testElements in the code flow before this function call. So not adding a unit test for that.
-        _unitTestDiscoverer.SendTestCases(new List<UnitTestElement> { }, _mockTestCaseDiscoverySink.Object, _mockDiscoveryContext.Object, _mockMessageLogger.Object.ToAdapterMessageLogger());
+        _unitTestDiscoverer.SendTestCases(new List<UnitTestElement> { }, _mockTestCaseDiscoverySink.Object.ToUnitTestElementSink(), _mockDiscoveryContext.Object, _mockMessageLogger.Object.ToAdapterMessageLogger());
 
         // Assert.
         _mockTestCaseDiscoverySink.Verify(ds => ds.SendTestCase(It.IsAny<TestCase>()), Times.Never);
@@ -221,7 +221,7 @@ public void SendTestCasesShouldSendAllTestCaseData()
         var test2 = new UnitTestElement(CreateTestMethod("M2", "C", "A", displayName: null));
         var testElements = new List<UnitTestElement> { test1, test2 };
 
-        _unitTestDiscoverer.SendTestCases(testElements, _mockTestCaseDiscoverySink.Object, _mockDiscoveryContext.Object, _mockMessageLogger.Object.ToAdapterMessageLogger());
+        _unitTestDiscoverer.SendTestCases(testElements, _mockTestCaseDiscoverySink.Object.ToUnitTestElementSink(), _mockDiscoveryContext.Object, _mockMessageLogger.Object.ToAdapterMessageLogger());
 
         // Assert.
         _mockTestCaseDiscoverySink.Verify(ds => ds.SendTestCase(It.Is<TestCase>(tc => tc.FullyQualifiedName == "C.M1")), Times.Once);
@@ -240,7 +240,7 @@ public void SendTestCasesShouldSendFilteredTestCasesIfValidFilterExpression()
         var testElements = new List<UnitTestElement> { test1, test2 };
 
         // Action
-        _unitTestDiscoverer.SendTestCases(testElements, _mockTestCaseDiscoverySink.Object, discoveryContext, _mockMessageLogger.Object.ToAdapterMessageLogger());
+        _unitTestDiscoverer.SendTestCases(testElements, _mockTestCaseDiscoverySink.Object.ToUnitTestElementSink(), discoveryContext, _mockMessageLogger.Object.ToAdapterMessageLogger());
 
         // Assert.
         _mockTestCaseDiscoverySink.Verify(ds => ds.SendTestCase(It.Is<TestCase>(tc => tc.FullyQualifiedName == "C.M1")), Times.Once);
@@ -259,7 +259,7 @@ public void SendTestCasesShouldSendAllTestCasesIfNullFilterExpression()
         var testElements = new List<UnitTestElement> { test1, test2 };
 
         // Action
-        _unitTestDiscoverer.SendTestCases(testElements, _mockTestCaseDiscoverySink.Object, discoveryContext, _mockMessageLogger.Object.ToAdapterMessageLogger());
+        _unitTestDiscoverer.SendTestCases(testElements, _mockTestCaseDiscoverySink.Object.ToUnitTestElementSink(), discoveryContext, _mockMessageLogger.Object.ToAdapterMessageLogger());
 
         // Assert.
         _mockTestCaseDiscoverySink.Verify(ds => ds.SendTestCase(It.Is<TestCase>(tc => tc.FullyQualifiedName == "C.M1")), Times.Once);
@@ -278,7 +278,7 @@ public void SendTestCasesShouldSendAllTestCasesIfGetTestCaseFilterNotPresent()
         var testElements = new List<UnitTestElement> { test1, test2 };
 
         // Action
-        _unitTestDiscoverer.SendTestCases(testElements, _mockTestCaseDiscoverySink.Object, discoveryContext, _mockMessageLogger.Object.ToAdapterMessageLogger());
+        _unitTestDiscoverer.SendTestCases(testElements, _mockTestCaseDiscoverySink.Object.ToUnitTestElementSink(), discoveryContext, _mockMessageLogger.Object.ToAdapterMessageLogger());
 
         // Assert.
         _mockTestCaseDiscoverySink.Verify(ds => ds.SendTestCase(It.Is<TestCase>(tc => tc.FullyQualifiedName == "C.M1")), Times.Once);
@@ -297,7 +297,7 @@ public void SendTestCasesShouldNotSendAnyTestCasesIfFilterError()
         var testElements = new List<UnitTestElement> { test1, test2 };
 
         // Action
-        _unitTestDiscoverer.SendTestCases(testElements, _mockTestCaseDiscoverySink.Object, discoveryContext, _mockMessageLogger.Object.ToAdapterMessageLogger());
+        _unitTestDiscoverer.SendTestCases(testElements, _mockTestCaseDiscoverySink.Object.ToUnitTestElementSink(), discoveryContext, _mockMessageLogger.Object.ToAdapterMessageLogger());
 
         // Assert.
         _mockTestCaseDiscoverySink.Verify(ds => ds.SendTestCase(It.Is<TestCase>(tc => tc.FullyQualifiedName == "C.M1")), Times.Never);
@@ -326,14 +326,14 @@ public DummyNavigationData(string fileName, int minLineNumber, int maxLineNumber
     internal override void DiscoverTestsInSource(
         string source,
         IAdapterMessageLogger logger,
-        ITestCaseDiscoverySink discoverySink,
+        IUnitTestElementSink discoverySink,
         IDiscoveryContext? discoveryContext,
         bool isMTP)
     {
-        var testCase1 = new TestCase("A", new Uri("executor://testExecutor"), source);
-        var testCase2 = new TestCase("B", new Uri("executor://testExecutor"), source);
-        discoverySink.SendTestCase(testCase1);
-        discoverySink.SendTestCase(testCase2);
+        var testElement1 = new UnitTestElement(new TestMethod("A", "C", source, displayName: null));
+        var testElement2 = new UnitTestElement(new TestMethod("B", "C", source, displayName: null));
+        discoverySink.SendTestElement(testElement1);
+        discoverySink.SendTestElement(testElement2);
     }
 }
 

test/UnitTests/MSTestAdapter.PlatformServices.UnitTests/Execution/TestCaseDiscoverySinkTests.cs

@@ -4,7 +4,7 @@
 using AwesomeAssertions;
 
 using Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.Execution;
-using Microsoft.VisualStudio.TestPlatform.ObjectModel;
+using Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.ObjectModel;
 
 using TestFramework.ForTestingMSTest;
 
@@ -22,21 +22,27 @@ public void TestCaseDiscoverySinkConstructorShouldInitializeTests()
         _testCaseDiscoverySink.Tests.Count.Should().Be(0);
     }
 
-    public void SendTestCaseShouldNotAddTestIfTestCaseIsNull()
+    public void SendTestElementShouldAddTheMaterializedTestCaseToTests()
     {
-        _testCaseDiscoverySink.SendTestCase(null);
+        var testElement = new UnitTestElement(new TestMethod("M", "C", "A", displayName: null));
+
+        _testCaseDiscoverySink.SendTestElement(testElement);
 
         _testCaseDiscoverySink.Tests.Should().NotBeNull();
-        _testCaseDiscoverySink.Tests.Count.Should().Be(0);
+        _testCaseDiscoverySink.Tests.Count.Should().Be(1);
+        _testCaseDiscoverySink.Tests.ToArray()[0].FullyQualifiedName.Should().Be("C.M");
     }
 
-    public void SendTestCaseShouldAddTheTestCaseToTests()
+    public void SendTestElementShouldAddEachTestCaseInOrder()
     {
-        TestCase tc = new("TAttribute", new Uri("executor://TestExecutorUri"), "A");
-        _testCaseDiscoverySink.SendTestCase(tc);
+        var testElement1 = new UnitTestElement(new TestMethod("M1", "C", "A", displayName: null));
+        var testElement2 = new UnitTestElement(new TestMethod("M2", "C", "A", displayName: null));
 
-        _testCaseDiscoverySink.Tests.Should().NotBeNull();
-        _testCaseDiscoverySink.Tests.Count.Should().Be(1);
-        _testCaseDiscoverySink.Tests.ToArray()[0].Should().Be(tc);
+        _testCaseDiscoverySink.SendTestElement(testElement1);
+        _testCaseDiscoverySink.SendTestElement(testElement2);
+
+        _testCaseDiscoverySink.Tests.Count.Should().Be(2);
+        _testCaseDiscoverySink.Tests.ToArray()[0].FullyQualifiedName.Should().Be("C.M1");
+        _testCaseDiscoverySink.Tests.ToArray()[1].FullyQualifiedName.Should().Be("C.M2");
     }
 }