[rel/3.8] Follow-up to ignore fix (#5043)

Co-authored-by: Youssef1313 <[email protected]>
Co-authored-by: Amaury Levé <[email protected]>

Author: 3 people

src/Adapter/MSTest.TestAdapter/Execution/TestMethodRunner.cs

@@ -170,7 +170,7 @@ internal TestResult[] RunTestMethod()
         var parentStopwatch = Stopwatch.StartNew();
         if (_test.DataType == DynamicDataType.ITestDataSource)
         {
-            if (!string.IsNullOrEmpty(_test.TestDataSourceIgnoreMessage))
+            if (_test.TestDataSourceIgnoreMessage is not null)
             {
                 _testContext.SetOutcome(UTF.UnitTestOutcome.Ignored);
                 return [TestResult.CreateIgnoredResult(_test.TestDataSourceIgnoreMessage)];
@@ -269,7 +269,7 @@ private bool TryExecuteFoldedDataDrivenTests(List<TestResult> results)
 
         foreach (UTF.ITestDataSource testDataSource in testDataSources)
         {
-            if (testDataSource is ITestDataSourceIgnoreCapability { IgnoreMessage: { Length: > 0 } ignoreMessage })
+            if (testDataSource is ITestDataSourceIgnoreCapability { IgnoreMessage: { } ignoreMessage })
             {
                 results.Add(TestResult.CreateIgnoredResult(ignoreMessage));
                 continue;

src/Adapter/MSTest.TestAdapter/ObjectModel/TestMethod.cs

@@ -153,12 +153,7 @@ public string? DeclaringClassFullName
     /// Gets or sets the test data source ignore message.
     /// </summary>
     /// <remarks>
-    /// The test is ignored if this is set to non-null and non-empty string.
-    /// If set to empty string, the test is considered as not ignored.
-    /// In VSTest, when tests are expanded during discovery, we end up reading
-    /// this property as empty string, even though it was set to null.
-    /// (probably some serialization issue on VSTest side)
-    /// So, we treat null and empty string the same.
+    /// The test is ignored if this is set to non-null.
     /// </remarks>
     internal string? TestDataSourceIgnoreMessage { get; set; }
 

src/Adapter/MSTest.TestAdapter/ObjectModel/UnitTestElement.cs

@@ -195,7 +195,11 @@ internal TestCase ToTestCase()
 
             testCase.SetPropertyValue(Constants.TestDynamicDataTypeProperty, (int)TestMethod.DataType);
             testCase.SetPropertyValue(Constants.TestDynamicDataProperty, data);
-            testCase.SetPropertyValue(Constants.TestDataSourceIgnoreMessageProperty, TestMethod.TestDataSourceIgnoreMessage);
+            // VSTest serialization doesn't handle null so instead don't set the property so that it's deserialized as null
+            if (TestMethod.TestDataSourceIgnoreMessage is not null)
+            {
+                testCase.SetPropertyValue(Constants.TestDataSourceIgnoreMessageProperty, TestMethod.TestDataSourceIgnoreMessage);
+            }
         }
 
         SetTestCaseId(testCase, testFullName);

src/TestFramework/TestFramework/Interfaces/ITestDataSourceIgnoreCapability.cs

@@ -9,7 +9,7 @@ namespace Microsoft.VisualStudio.TestTools.UnitTesting;
 public interface ITestDataSourceIgnoreCapability
 {
     /// <summary>
-    /// Gets or sets a reason to ignore the test data source. Setting the property to non-null and non-empty string value will ignore the test data source.
+    /// Gets or sets a reason to ignore the test data source. Setting the property to non-null value will ignore the test data source.
     /// </summary>
     string? IgnoreMessage { get; set; }
 }