Fix xUnit Theory serialization warnings #3186

paulmedynski · 2025-02-28T14:21:39Z

xUnit makes 2 passes over unit tests:

Discover all of the tests, and for Theory tests, enumerate all of the test data.
Run all of the tests, enumerating the test data again.

If the enumeration won't produce the same data in both steps, xUnit emits a warning. We have many test cases whose Theory data doesn't enumerate identically in both passes, for a variety of reasons. These tests will still run correctly, but they won't appear in test runner UIs like Visual Studio. Instead, xUnit collapses them into a single test for these UIs.

The warnings are clutter in the console (where I like to run tests), so xUnit provides a mechanism to disable enumeration completely in the discovery phase. I have updated the offending tests to use the MemberData DisableDiscoveryEnumeration flag. This doesn't affect how the tests run, and it doesn't change how UIs display them (collapsed). All it does is suppress the console warnings.

Further reading here: https://xunit.net/faq/theory-data-stability-in-vs

I also added 4 new targets to build.proj to make running the tests easier. The existing test running targets function as before.

Commit notes:

Suppressed xUnit discovery data enumeration warnings in Function and Manual test projects.
Fixed xUnit test data enumeration warnings specific to .NET Framework (enums in the GAC).
Added targets to run Windows and Unix tests.
Cleaned up test run command lines for improved maintenance and runtime logging.

- Suppressed xUnit discovery data enumeration warnings in Function and Manual test projects. - Fixed xUnit test data enumeration warnings specific to .NET Framework (enums in the GAC). - Added targets to run Windows and Unix tests. - Cleaned up test run command lines for improved maintenance and runtime logging.

paulmedynski · 2025-02-28T14:29:58Z

build.proj

+  <Target Name="RunFunctionalTestsWindows" Condition="'$(IsEnabledWindows)' == 'true'">
+    <PropertyGroup>
+      <TestCommand>
+        $(DotnetPath)dotnet test "@(FunctionalTestsProj)"


Formatted for readability, and then squished back into a single line since <Exec> treats each line in the Command as a separate command!

paulmedynski · 2025-02-28T14:31:12Z

build.proj

+        --filter "category!=non$(TargetGroup)tests&amp;category!=failing&amp;category!=nonwindowstests"
+        --logger:"trx;LogFilePrefix=Functional-Windows$(TargetGroup)-$(TestSet)"
+      </TestCommand>
+      <TestCommand>$(TestCommand.Replace($([System.Environment]::NewLine), " "))</TestCommand>


Modern dotnet tooling supports String.ReplaceLineEndings(), but we're using the old .NET Framework msbuild.exe tooling to build, so we can't have the nice toys :(

paulmedynski · 2025-02-28T14:33:09Z

...ts/FunctionalTests/AlwaysEncryptedTests/SqlColumnEncryptionCertificateStoreProviderShould.cs

+        [MemberData(
+            nameof(CEKEncryptionReversalData)
+#if NETFRAMEWORK
+            // .NET Framework puts system enums in something called the Global


I've left a comment once in each file where we're using DisableDiscoveryEnumeration, to make it clear why we're using that flag.

paulmedynski · 2025-02-28T14:34:31Z

...ts/FunctionalTests/AlwaysEncryptedTests/SqlColumnEncryptionCertificateStoreProviderShould.cs

@@ -505,33 +519,25 @@ public override IEnumerable<Object[]> GetData(MethodInfo testMethod)
            }
        }

-        public class CEKEncryptionReversalParameters : DataAttribute
+        public static IEnumerable<object[]> CEKEncryptionReversalData()


Since xUnit only supports DisableDiscoveryEnumeration on MemberData attributes, I have converted all of the data sources to be compatible with MemberData.

You will see similar changes throughout the tests, where data source classes become methods.

paulmedynski · 2025-02-28T14:35:29Z

...ts/FunctionalTests/AlwaysEncryptedTests/SqlColumnEncryptionCertificateStoreProviderShould.cs

-        [CEKEncryptionReversalParameters]
+        [MemberData(
+            nameof(CEKEncryptionReversalData)
+#if NETFRAMEWORK


.NET doesn't have a GAC, so these tests will enumerate correctly for those targets. We only need to disable for .NET Framework targets.

paulmedynski · 2025-02-28T14:35:49Z

src/Microsoft.Data.SqlClient/tests/FunctionalTests/AmbientTransactionFailureTest.cs

@@ -4,6 +4,7 @@

 using System;
 using System.Runtime.InteropServices;
+using System.Threading;


I don't think this is necessary.

paulmedynski · 2025-02-28T14:37:53Z

src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlMetaDataTest.cs

+#endif
+            )]
+        [MemberData(
+            nameof(ConstructorTextData)


This adds Text to the mix, which seems to have been missing from the InlineData.

paulmedynski · 2025-02-28T14:41:21Z

src/Microsoft.Data.SqlClient/tests/ManualTests/AlwaysEncrypted/ConversionTests.cs

@@ -565,7 +584,20 @@ private void ExecuteAndCheckForError(SqlCommand sqlCmd, bool expectError)
                try
                {
                    sqlCmd.ExecuteNonQuery();
-                    Assert.Fail("We should have gotten an error but passed instead.");
+                    StringBuilder builder = new(


With broken (now disabled) enumeration, xUnit doesn't include the test data in any failure output, so we need to do it ourselves.

This test fails for me on main as well, so I wanted to see what data set was causing the problem. Turns out to be the Date data set (line 1450).

paulmedynski · 2025-02-28T14:45:47Z

...rosoft.Data.SqlClient/tests/tools/Microsoft.Data.SqlClient.TestUtilities/config.default.json

@@ -1,6 +1,13 @@
 {
+    // Typical Windows SQL Server on the local host via TCP.


These comments are breaking our CI "Update config.json" task because PowerShell's ConvertFrom-Json fails But as of PowerShell v6, ConvertFrom-Json handles comments:

https://learn.microsoft.com/en-us/powershell/module/microsoft.powershell.utility/convertfrom-json?view=powershell-7.5

Are we really using ancient PowerShell 5.1 from 2016? I will have to dig into this further.

- Emitting PowerShell version to debug JSON parsing failure.

- Changed config.json update step to use modern PowerShell via the "pwsh" shortcut instead of ancient PowerShell via the "powershell" shortcut.

paulmedynski · 2025-02-28T20:08:43Z

eng/pipelines/common/templates/steps/update-config-file-step.yml

@@ -126,7 +126,7 @@ parameters:

 steps:
 # All properties should be added here, and this template should be used for any manipulation of the config.json file.
- powershell: |
+- pwsh: |
    $PSVersionTable


Remove this once we solve the JSON-with-comments problem.

paulmedynski commented Feb 28, 2025

View reviewed changes

paulmedynski added 2 commits February 28, 2025 15:31

User Story 33279: Fix xUnit Theory serialization warnings

a47a5f3

- Emitting PowerShell version to debug JSON parsing failure.

User Story 33279: Fix xUnit Theory serialization warnings

a04396b

- Changed config.json update step to use modern PowerShell via the "pwsh" shortcut instead of ancient PowerShell via the "powershell" shortcut.

paulmedynski commented Feb 28, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix xUnit Theory serialization warnings #3186

Fix xUnit Theory serialization warnings #3186

paulmedynski commented Feb 28, 2025 •

edited

Loading

paulmedynski Feb 28, 2025 •

edited

Loading

paulmedynski Feb 28, 2025

paulmedynski Feb 28, 2025

paulmedynski Feb 28, 2025

paulmedynski Feb 28, 2025

paulmedynski Feb 28, 2025

paulmedynski Feb 28, 2025

paulmedynski Feb 28, 2025 •

edited

Loading

paulmedynski Feb 28, 2025

paulmedynski Feb 28, 2025

		@@ -1,6 +1,13 @@
		{
		// Typical Windows SQL Server on the local host via TCP.

Fix xUnit Theory serialization warnings #3186

Are you sure you want to change the base?

Fix xUnit Theory serialization warnings #3186

Conversation

paulmedynski commented Feb 28, 2025 • edited Loading

paulmedynski Feb 28, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paulmedynski Feb 28, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paulmedynski commented Feb 28, 2025 •

edited

Loading

paulmedynski Feb 28, 2025 •

edited

Loading

paulmedynski Feb 28, 2025 •

edited

Loading