Fix DataSourceAttribute not being discovered with TestDataSourceDisco…

…veryOption.DuringDiscovery (#4058)

azure-pipelines.yml

@@ -77,6 +77,14 @@ stages:
             failOnStderr: true
             showWarnings: true
 
+        - task: PowerShell@2
+          displayName: 'Install Access Database Engine'
+          inputs:
+            targetType: filePath
+            filePath: ./eng/install-access-database-engine.ps1
+            failOnStderr: true
+            showWarnings: true
+
         - script: eng\common\CIBuild.cmd
             -configuration $(_BuildConfig)
             -prepareMachine

eng/install-access-database-engine.ps1

@@ -0,0 +1,7 @@
+# This is required for DataSourceTests
+# Otherwise, the tests will fail with:
+# The unit test adapter failed to connect to the data source or to read the data. For more information on troubleshooting this error, see "Troubleshooting Data-Driven Unit Tests" (http://go.microsoft.com/fwlink/?LinkId=62412) in the MSDN Library. Error details: The 'Microsoft.Ace.OLEDB.12.0' provider is not registered on the local machine.
+#     at Microsoft.VisualStudio.TestPlatform.MSTestAdapter.PlatformServices.TestDataSource.GetData(ITestMethod testMethodInfo, ITestContext testContext) in /_/src/Adapter/MSTestAdapter.PlatformServices/Services/TestDataSource.cs:84
+# The direct download link originates from https://www.microsoft.com/en-us/download/details.aspx?id=54920&msockid=01fa77be234c617f31936293223560aa
+Invoke-RestMethod https://download.microsoft.com/download/3/5/C/35C84C36-661A-44E6-9324-8786B8DBE231/accessdatabaseengine_X64.exe -OutFile ./accessdatabaseengine_X64.exe
+Start-Process ./accessdatabaseengine_X64.exe -Wait -ArgumentList "/quiet /passive /norestart"

src/Adapter/MSTest.TestAdapter/Discovery/AssemblyEnumerator.cs

@@ -285,10 +285,7 @@ private static bool DynamicDataAttached(UnitTestElement test, Lazy<TestMethodInf
             return false;
         }
 
-        if (test.TestMethod.DataType == DynamicDataType.None)
-        {
-            return false;
-        }
+        DynamicDataType originalDataType = test.TestMethod.DataType;
 
         // PERF: For perf we started setting DataType in TypeEnumerator, so when it is None we will not reach this line.
         // But if we do run this code, we still reset it to None, because the code that determines if this is data drive test expects the value to be None
@@ -297,7 +294,15 @@ private static bool DynamicDataAttached(UnitTestElement test, Lazy<TestMethodInf
         // If you remove this line and acceptance tests still pass you are okay.
         test.TestMethod.DataType = DynamicDataType.None;
 
-        return testMethodInfo.Value != null && TryProcessTestDataSourceTests(test, testMethodInfo.Value, tests);
+        // The data source tests that we can process currently are those using attributes that
+        // implement ITestDataSource (i.e, DataRow and DynamicData attributes).
+        // However, for DataSourceAttribute, we currently don't have anyway to process it during discovery.
+        // (Note: this method is only called under discoveryOption == TestDataSourceDiscoveryOption.DuringDiscovery)
+        // So we want to return false from this method for non ITestDataSource (whether it's None or DataSourceAttribute). Otherwise, the test
+        // will be completely skipped which is wrong behavior.
+        return originalDataType == DynamicDataType.ITestDataSource &&
+            testMethodInfo.Value != null &&
+            TryProcessITestDataSourceTests(test, testMethodInfo.Value, tests);
     }
 
     private static void AddFixtureTests(TestMethodInfo testMethodInfo, List<UnitTestElement> tests, HashSet<string> fixtureTests)
@@ -381,7 +386,7 @@ static UnitTestElement GetFixtureTest(string classFullName, string assemblyLocat
         }
     }
 
-    private static bool TryProcessTestDataSourceTests(UnitTestElement test, TestMethodInfo testMethodInfo, List<UnitTestElement> tests)
+    private static bool TryProcessITestDataSourceTests(UnitTestElement test, TestMethodInfo testMethodInfo, List<UnitTestElement> tests)
     {
         // We don't have a special method to filter attributes that are not derived from Attribute, so we take all
         // attributes and filter them. We don't have to care if there is one, because this method is only entered when
@@ -390,7 +395,7 @@ private static bool TryProcessTestDataSourceTests(UnitTestElement test, TestMeth
 
         try
         {
-            return ProcessTestDataSourceTests(test, new(testMethodInfo.MethodInfo, test.DisplayName), testDataSources, tests);
+            return ProcessITestDataSourceTests(test, new(testMethodInfo.MethodInfo, test.DisplayName), testDataSources, tests);
         }
         catch (Exception ex)
         {
@@ -400,7 +405,7 @@ private static bool TryProcessTestDataSourceTests(UnitTestElement test, TestMeth
         }
     }
 
-    private static bool ProcessTestDataSourceTests(UnitTestElement test, ReflectionTestMethodInfo methodInfo, IEnumerable<ITestDataSource> testDataSources,
+    private static bool ProcessITestDataSourceTests(UnitTestElement test, ReflectionTestMethodInfo methodInfo, IEnumerable<ITestDataSource> testDataSources,
         List<UnitTestElement> tests)
     {
         foreach (ITestDataSource dataSource in testDataSources)

test/IntegrationTests/MSTest.Acceptance.IntegrationTests/DataSourceTests.cs

@@ -0,0 +1,117 @@
+// 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.Testing.Platform.Acceptance.IntegrationTests;
+using Microsoft.Testing.Platform.Acceptance.IntegrationTests.Helpers;
+using Microsoft.Testing.Platform.Helpers;
+
+namespace MSTest.Acceptance.IntegrationTests;
+
+[TestGroup]
+public class DataSourceTests : AcceptanceTestBase
+{
+    private const string SourceCode = """
+#file DataSourceTests.csproj
+<Project Sdk="Microsoft.NET.Sdk">
+    <PropertyGroup>
+        <TargetFramework>net472</TargetFramework>
+        <ImplicitUsings>enable</ImplicitUsings>
+        <OutputType>Exe</OutputType>
+        <LangVersion>preview</LangVersion>
+        <EnableMSTestRunner>true</EnableMSTestRunner>
+    </PropertyGroup>
+    <ItemGroup>
+        <PackageReference Include="Microsoft.NET.Test.Sdk" Version="$MicrosoftNETTestSdkVersion$" />
+        <PackageReference Include="MSTest.TestAdapter" Version="$MSTestVersion$" />
+        <PackageReference Include="MSTest.TestFramework" Version="$MSTestVersion$" />
+
+        <None Include="TestData.csv">
+            <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
+        </None>
+    </ItemGroup>
+</Project>
+
+#file App.config
+<?xml version="1.0" encoding="utf-8"?>
+<configuration>
+  <configSections>
+    <section name="microsoft.visualstudio.testtools" type="Microsoft.VisualStudio.TestTools.UnitTesting.TestConfigurationSection, Microsoft.VisualStudio.TestPlatform.TestFramework.Extensions"/>
+  </configSections>
+  <connectionStrings>
+    <add name="ConnString" connectionString="TestData.csv" providerName="Microsoft.VisualStudio.TestTools.DataSource.CSV"/>
+  </connectionStrings>
+  <microsoft.visualstudio.testtools>
+    <dataSources>
+      <add name="TestData" connectionString="ConnString" dataTableName="TestData#csv" dataAccessMethod="Sequential"/>
+    </dataSources>
+  </microsoft.visualstudio.testtools>
+  <startup>
+    <supportedRuntime version="v4.0" sku=".NETFramework,Version=v4.8"/>
+  </startup>
+</configuration>
+
+
+#file MyTestClass.cs
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+
+[TestClass]
+public class MyTestClass
+{
+    public TestContext TestContext { get; set; }
+
+    [DataTestMethod]
+    [DataSource("TestData")]
+    public void TestSum()
+    {
+        int expected = (int)TestContext.DataRow["expectedSum"];
+        int num1 = (int)TestContext.DataRow["num1"];
+        int num2 = (int)TestContext.DataRow["num2"];
+        Assert.AreEqual(expected, num1 + num2);
+    }
+
+    [TestMethod]
+    public void MyTest()
+    {
+    }
+}
+
+#file TestData.csv
+num1,num2,expectedSum
+1,1,2
+5,6,11
+10,30,40
+1,1,1
+""";
+
+    private readonly AcceptanceFixture _acceptanceFixture;
+
+    public DataSourceTests(ITestExecutionContext testExecutionContext, AcceptanceFixture acceptanceFixture)
+        : base(testExecutionContext) => _acceptanceFixture = acceptanceFixture;
+
+    public async Task TestDataSourceFromAppConfig()
+    {
+        if (!OperatingSystem.IsWindows())
+        {
+            // Test is specific to .NET Framework.
+            return;
+        }
+
+        using TestAsset generator = await TestAsset.GenerateAssetAsync(
+            "DataSourceTests",
+            SourceCode
+            .PatchCodeWithReplace("$MSTestVersion$", MSTestVersion)
+            .PatchCodeWithReplace("$MicrosoftNETTestSdkVersion$", MicrosoftNETTestSdkVersion),
+            addPublicFeeds: true);
+
+        await DotnetCli.RunAsync(
+            $"build {generator.TargetAssetPath} -c Release",
+            _acceptanceFixture.NuGetGlobalPackagesFolder.Path,
+            retryCount: 0);
+
+        var testHost = TestHost.LocateFrom(generator.TargetAssetPath, "DataSourceTests", "net472");
+
+        TestHostResult result = await testHost.ExecuteAsync();
+        result.AssertExitCodeIs(ExitCodes.AtLeastOneTestFailed);
+        result.AssertOutputContainsSummary(failed: 1, passed: 4, skipped: 0);
+    }
+}