Remove ConsiderFixturesAsSpecialTests (#7189)

Author: Youssef1313

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

@@ -7,8 +7,6 @@
 using Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.Execution;
 using Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.Helpers;
 using Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.ObjectModel;
-using Microsoft.VisualStudio.TestPlatform.MSTestAdapter.PlatformServices;
-using Microsoft.VisualStudio.TestPlatform.ObjectModel;
 using Microsoft.VisualStudio.TestTools.UnitTesting;
 using Microsoft.VisualStudio.TestTools.UnitTesting.Internal;
 
@@ -69,8 +67,6 @@ internal AssemblyEnumerationResult EnumerateAssembly(string assemblyFileName)
         List<string> warnings = [];
         DebugEx.Assert(!StringEx.IsNullOrWhiteSpace(assemblyFileName), "Invalid assembly file name.");
         var tests = new List<UnitTestElement>();
-        // Contains list of assembly/class names for which we have already added fixture tests.
-        var fixturesTests = new HashSet<string>();
 
         Assembly assembly = PlatformServiceProvider.Instance.FileOperations.LoadAssembly(assemblyFileName);
 
@@ -94,7 +90,7 @@ internal AssemblyEnumerationResult EnumerateAssembly(string assemblyFileName)
         foreach (Type type in types)
         {
             List<UnitTestElement> testsInType = DiscoverTestsInType(assemblyFileName, type, warnings, discoverInternals,
-                dataSourcesUnfoldingStrategy, fixturesTests);
+                dataSourcesUnfoldingStrategy);
             tests.AddRange(testsInType);
         }
 
@@ -154,8 +150,7 @@ private List<UnitTestElement> DiscoverTestsInType(
         Type type,
         List<string> warningMessages,
         bool discoverInternals,
-        TestDataSourceUnfoldingStrategy dataSourcesUnfoldingStrategy,
-        HashSet<string> fixturesTests)
+        TestDataSourceUnfoldingStrategy dataSourcesUnfoldingStrategy)
     {
         string? typeFullName = null;
         var tests = new List<UnitTestElement>();
@@ -172,12 +167,6 @@ private List<UnitTestElement> DiscoverTestsInType(
                 {
                     if (_typeCache.GetTestMethodInfoForDiscovery(test.TestMethod) is { } testMethodInfo)
                     {
-                        // Add fixture tests like AssemblyInitialize, AssemblyCleanup, ClassInitialize, ClassCleanup.
-                        if (MSTestSettings.CurrentSettings.ConsiderFixturesAsSpecialTests)
-                        {
-                            AddFixtureTests(testMethodInfo, tests, fixturesTests);
-                        }
-
                         if (TryUnfoldITestDataSources(test, testMethodInfo, dataSourcesUnfoldingStrategy, tests))
                         {
                             continue;
@@ -200,88 +189,6 @@ private List<UnitTestElement> DiscoverTestsInType(
         return tests;
     }
 
-    private static void AddFixtureTests(DiscoveryTestMethodInfo testMethodInfo, List<UnitTestElement> tests, HashSet<string> fixtureTests)
-    {
-        string assemblyName = testMethodInfo.Parent.Parent.Assembly.GetName().Name!;
-        string assemblyLocation = testMethodInfo.Parent.Parent.Assembly.Location;
-        string classFullName = testMethodInfo.Parent.ClassType.FullName!;
-
-        // Check if fixtures for this assembly has already been added.
-        if (!fixtureTests.Contains(assemblyLocation))
-        {
-            _ = fixtureTests.Add(assemblyLocation);
-
-            // Add AssemblyInitialize and AssemblyCleanup fixture tests if they exist.
-            if (testMethodInfo.Parent.Parent.AssemblyInitializeMethod is not null)
-            {
-                tests.Add(GetAssemblyFixtureTest(testMethodInfo.Parent.Parent.AssemblyInitializeMethod, assemblyName,
-                    classFullName, assemblyLocation, EngineConstants.AssemblyInitializeFixtureTrait));
-            }
-
-            if (testMethodInfo.Parent.Parent.AssemblyCleanupMethod is not null)
-            {
-                tests.Add(GetAssemblyFixtureTest(testMethodInfo.Parent.Parent.AssemblyCleanupMethod, assemblyName,
-                    classFullName, assemblyLocation, EngineConstants.AssemblyCleanupFixtureTrait));
-            }
-        }
-
-        // Check if fixtures for this class has already been added.
-        if (!fixtureTests.Contains(assemblyLocation + classFullName))
-        {
-            _ = fixtureTests.Add(assemblyLocation + classFullName);
-
-            // Add ClassInitialize and ClassCleanup fixture tests if they exist.
-            if (testMethodInfo.Parent.ClassInitializeMethod is not null)
-            {
-                tests.Add(GetClassFixtureTest(testMethodInfo.Parent.ClassInitializeMethod, classFullName,
-                    assemblyLocation, EngineConstants.ClassInitializeFixtureTrait));
-            }
-
-            if (testMethodInfo.Parent.ClassCleanupMethod is not null)
-            {
-                tests.Add(GetClassFixtureTest(testMethodInfo.Parent.ClassCleanupMethod, classFullName,
-                    assemblyLocation, EngineConstants.ClassCleanupFixtureTrait));
-            }
-        }
-
-        static UnitTestElement GetAssemblyFixtureTest(MethodInfo methodInfo, string assemblyName, string classFullName,
-            string assemblyLocation, string fixtureType)
-        {
-            string methodName = GetMethodName(methodInfo);
-            string[] hierarchy = [null!, assemblyName, EngineConstants.AssemblyFixturesHierarchyClassName, methodName];
-            return GetFixtureTest(classFullName, assemblyLocation, fixtureType, methodName, hierarchy, methodInfo);
-        }
-
-        static UnitTestElement GetClassFixtureTest(MethodInfo methodInfo, string classFullName,
-            string assemblyLocation, string fixtureType)
-        {
-            string methodName = GetMethodName(methodInfo);
-            string[] hierarchy = [null!, classFullName, methodName];
-            return GetFixtureTest(classFullName, assemblyLocation, fixtureType, methodName, hierarchy, methodInfo);
-        }
-
-        static string GetMethodName(MethodInfo methodInfo)
-        {
-            ParameterInfo[] args = methodInfo.GetParameters();
-            return args.Length > 0
-                ? $"{methodInfo.Name}({string.Join(',', args.Select(a => a.ParameterType.FullName))})"
-                : methodInfo.Name;
-        }
-
-        static UnitTestElement GetFixtureTest(string classFullName, string assemblyLocation, string fixtureType, string methodName, string[] hierarchy, MethodInfo methodInfo)
-        {
-            string displayName = $"[{fixtureType}] {methodName}";
-            var method = new TestMethod(classFullName, methodName, hierarchy, methodName, classFullName, assemblyLocation, displayName, null)
-            {
-                MethodInfo = methodInfo,
-            };
-            return new UnitTestElement(method)
-            {
-                Traits = [new Trait(EngineConstants.FixturesTestTrait, fixtureType)],
-            };
-        }
-    }
-
     private static bool TryUnfoldITestDataSources(UnitTestElement test, DiscoveryTestMethodInfo testMethodInfo, TestDataSourceUnfoldingStrategy dataSourcesUnfoldingStrategy, List<UnitTestElement> tests)
     {
         // It should always be `true`, but if any part of the chain is obsolete; it might not contain those.

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

@@ -3,9 +3,7 @@
 
 using Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.Discovery;
 using Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.ObjectModel;
-using Microsoft.VisualStudio.TestPlatform.MSTestAdapter.PlatformServices;
 using Microsoft.VisualStudio.TestPlatform.MSTestAdapter.PlatformServices.Interface;
-using Microsoft.VisualStudio.TestPlatform.ObjectModel;
 using Microsoft.VisualStudio.TestPlatform.ObjectModel.Adapter;
 using Microsoft.VisualStudio.TestPlatform.ObjectModel.Logging;
 using Microsoft.VisualStudio.TestTools.UnitTesting;
@@ -102,9 +100,6 @@ internal virtual void DiscoverTestsInSource(
 
     internal void SendTestCases(IEnumerable<UnitTestElement> testElements, ITestCaseDiscoverySink discoverySink, IDiscoveryContext? discoveryContext, IMessageLogger logger)
     {
-        bool hasAnyRunnableTests = false;
-        var fixtureTests = new List<TestCase>();
-
         // Get filter expression and skip discovery in case filter expression has parsing error.
         ITestCaseFilterExpression? filterExpression = _testMethodFilter.GetFilterExpression(discoveryContext, logger, out bool filterHasError);
         if (filterHasError)
@@ -115,38 +110,14 @@ internal void SendTestCases(IEnumerable<UnitTestElement> testElements, ITestCase
         foreach (UnitTestElement testElement in testElements)
         {
             var testCase = testElement.ToTestCase();
-            bool hasFixtureTraits = testElement.Traits?.Any(t => t.Name == EngineConstants.FixturesTestTrait) == true;
 
             // Filter tests based on test case filters
             if (filterExpression != null && !filterExpression.MatchTestCase(testCase, p => _testMethodFilter.PropertyValueProvider(testCase, p)))
             {
-                // If test is a fixture test, add it to the list of fixture tests.
-                if (hasFixtureTraits)
-                {
-                    fixtureTests.Add(testCase);
-                }
-
                 continue;
             }
 
-            if (!hasAnyRunnableTests)
-            {
-                hasAnyRunnableTests = !hasFixtureTraits;
-            }
-
             discoverySink.SendTestCase(testCase);
         }
-
-        // If there are runnable tests, then add all fixture tests to the discovery sink.
-        // Scenarios:
-        // 1. Execute only a fixture test => In this case, we do not need to track any other fixture tests. Selected fixture test will be tracked as will be marked as skipped.
-        // 2. Execute a runnable test => In this case, case add all fixture tests. We will update status of only those fixtures which are triggered by the selected test.
-        if (hasAnyRunnableTests)
-        {
-            foreach (TestCase testCase in fixtureTests)
-            {
-                discoverySink.SendTestCase(testCase);
-            }
-        }
     }
 }

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

@@ -443,9 +443,6 @@ private async Task ExecuteTestsWithTestRunnerAsync(
         UnitTestRunner testRunner,
         bool usesAppDomains)
     {
-        bool hasAnyRunnableTests = false;
-        List<TestCase>? fixtureTests = null;
-
         IEnumerable<TestCase> orderedTests = MSTestSettings.CurrentSettings.OrderTestsByNameInClass
             ? tests.OrderBy(t => t.GetManagedType()).ThenBy(t => t.GetManagedMethod())
             : tests;
@@ -464,15 +461,6 @@ private async Task ExecuteTestsWithTestRunnerAsync(
                 break;
             }
 
-            // If it is a fixture test, add it to the list of fixture tests and do not execute it.
-            // It is executed by test itself.
-            if (currentTest.Traits.Any(t => t.Name == EngineConstants.FixturesTestTrait))
-            {
-                (fixtureTests ??= []).Add(currentTest);
-                continue;
-            }
-
-            hasAnyRunnableTests = true;
             UnitTestElement unitTestElement = currentTest.ToUnitTestElementWithUpdatedSource(source);
 
             testExecutionRecorder.RecordStart(currentTest);
@@ -509,44 +497,6 @@ private async Task ExecuteTestsWithTestRunnerAsync(
 
             SendTestResults(currentTest, unitTestResult, startTime, endTime, testExecutionRecorder);
         }
-
-        // Once all tests have been executed, update the status of fixture tests.
-        if (fixtureTests is null)
-        {
-            return;
-        }
-
-        foreach (TestCase currentTest in fixtureTests)
-        {
-            _testRunCancellationToken?.ThrowIfCancellationRequested();
-
-            testExecutionRecorder.RecordStart(currentTest);
-
-            // If there were only fixture tests, send an inconclusive result.
-            if (!hasAnyRunnableTests)
-            {
-                var result = new TestTools.UnitTesting.TestResult
-                {
-                    Outcome = TestTools.UnitTesting.UnitTestOutcome.Inconclusive,
-                };
-
-                SendTestResults(currentTest, [result], DateTimeOffset.Now, DateTimeOffset.Now, testExecutionRecorder);
-                continue;
-            }
-
-            Trait trait = currentTest.Traits.First(t => t.Name == EngineConstants.FixturesTestTrait);
-            UnitTestElement unitTestElement = currentTest.ToUnitTestElementWithUpdatedSource(source);
-            FixtureTestResult fixtureTestResult = testRunner.GetFixtureTestResult(unitTestElement.TestMethod, trait.Value);
-
-            if (fixtureTestResult.IsExecuted)
-            {
-                var result = new TestTools.UnitTesting.TestResult
-                {
-                    Outcome = fixtureTestResult.Outcome,
-                };
-                SendTestResults(currentTest, [result], DateTimeOffset.Now, DateTimeOffset.Now, testExecutionRecorder);
-            }
-        }
     }
 
     /// <summary>

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

@@ -23,8 +23,6 @@ namespace Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.Execution;
 /// </summary>
 internal sealed class UnitTestRunner : MarshalByRefObject
 {
-    private readonly ConcurrentDictionary<string, TestAssemblyInfo> _assemblyFixtureTests = new();
-    private readonly ConcurrentDictionary<string, TestClassInfo> _classFixtureTests = new();
     private readonly TypeCache _typeCache;
     private readonly ClassCleanupManager _classCleanupManager;
 
@@ -81,41 +79,6 @@ public void Cancel()
 #endif
     public override object InitializeLifetimeService() => null!;
 
-    internal FixtureTestResult GetFixtureTestResult(TestMethod testMethod, string fixtureType)
-    {
-        // For the fixture methods, we need to return the appropriate result.
-        // Get matching testMethodInfo from the cache and return UnitTestOutcome for the fixture test.
-        if (fixtureType is EngineConstants.ClassInitializeFixtureTrait or EngineConstants.ClassCleanupFixtureTrait &&
-            _classFixtureTests.TryGetValue(testMethod.AssemblyName + testMethod.FullClassName, out TestClassInfo? testClassInfo))
-        {
-            UnitTestOutcome outcome = fixtureType switch
-            {
-                EngineConstants.ClassInitializeFixtureTrait => testClassInfo.IsClassInitializeExecuted ? GetOutcome(testClassInfo.ClassInitializationException) : UnitTestOutcome.Inconclusive,
-                EngineConstants.ClassCleanupFixtureTrait => testClassInfo.IsClassCleanupExecuted ? GetOutcome(testClassInfo.ClassCleanupException) : UnitTestOutcome.Inconclusive,
-                _ => throw ApplicationStateGuard.Unreachable(),
-            };
-
-            return new FixtureTestResult(true, outcome);
-        }
-        else if (fixtureType is EngineConstants.AssemblyInitializeFixtureTrait or EngineConstants.AssemblyCleanupFixtureTrait &&
-            _assemblyFixtureTests.TryGetValue(testMethod.AssemblyName, out TestAssemblyInfo? testAssemblyInfo))
-        {
-            Exception? exception = fixtureType switch
-            {
-                EngineConstants.AssemblyInitializeFixtureTrait => testAssemblyInfo.AssemblyInitializationException,
-                EngineConstants.AssemblyCleanupFixtureTrait => testAssemblyInfo.AssemblyCleanupException,
-                _ => throw ApplicationStateGuard.Unreachable(),
-            };
-
-            return new(true, GetOutcome(exception));
-        }
-
-        return new(false, UnitTestOutcome.Inconclusive);
-
-        // Local functions
-        static UnitTestOutcome GetOutcome(Exception? exception) => exception == null ? UnitTestOutcome.Passed : UnitTestOutcome.Failed;
-    }
-
     // Task cannot cross app domains.
     // For now, TestExecutionManager will call this sync method which is hacky.
     // If we removed AppDomains in v4, we should use the async method and remove this one.
@@ -158,13 +121,6 @@ internal async Task<TestResult[]> RunSingleTestAsync(TestMethod testMethod, IDic
             {
                 DebugEx.Assert(testMethodInfo is not null, "testMethodInfo should not be null.");
 
-                // Keep track of all non-runnable methods so that we can return the appropriate result at the end.
-                if (MSTestSettings.CurrentSettings.ConsiderFixturesAsSpecialTests)
-                {
-                    _assemblyFixtureTests.TryAdd(testMethod.AssemblyName, testMethodInfo.Parent.Parent);
-                    _classFixtureTests.TryAdd(testMethod.AssemblyName + testMethod.FullClassName, testMethodInfo.Parent);
-                }
-
                 testContextForAssemblyInit = PlatformServiceProvider.Instance.GetTestContext(testMethod: null, null, testContextProperties, messageLogger, testContextForTestExecution.Context.CurrentTestOutcome);
 
                 TestResult assemblyInitializeResult = await RunAssemblyInitializeIfNeededAsync(testMethodInfo, testContextForAssemblyInit).ConfigureAwait(false);