Test | Convert DateTimeVariantTests to xunit by mdaigle · Pull Request #4196 · dotnet/SqlClient

mdaigle · 2026-04-14T23:34:29Z

Parameterizes tests using shared test data. Expected exceptions are made explicit in test data. Logging and baseline file comparisons are replaced with assertions.
Each commit can be reviewed individually to see the logical progression of the conversion.

Remove the [Collection], [Trait("Category", "flaky")] attributes and CultureInfo workaround. These were masking underlying issues that will be addressed by converting to direct xunit assertions.

Convert DateTimeVariantTests from a single monolithic test to xunit [ConditionalTheory] with [MemberData] providing parameterized test cases for each date/time type combination. Each test case now calls SendInfo on its specific (value, typeName, baseTypeName) combination and compares against its own per-test baseline file. Remove the TestAllDateTimeWithDataTypeAndVariant dispatch method from DateTimeVariantTest since each test case now invokes SendInfo directly. Make SendInfo public and pass the connection string explicitly.

Split test logic from error handling in DateTimeVariantTest.cs. Each test method now separates the core SQL operations from the try/catch exception handling. Consolidate error handling patterns across all 16 test variations. Pull some expected errors up into parameterized test input in DateTimeVariantTests.cs so callers declare which exceptions are expected. Remove exception handlers that are no longer needed.

Add TestVariations enum to explicitly identify each of the 16 test methods (TestSimpleParameter_Type, TestSimpleParameter_Variant, TVP variations, DataReader, BulkCopy, etc.). Associate expected exceptions with specific test variations in DateTimeVariantTests using dictionaries passed through MemberData. Register expected-but-uncaught exceptions so they can be tracked and asserted later.

Consolidate verification into a single method. Simplify date comparison logic. Remove unnecessary checks, casts, and unused parameters from the helper methods. Pull expected value mismatches and unexpected value overrides up into the parameterized test input so each test case declares its full expected behavior upfront.

Add xunit Assert.Equal/Assert.True for exception validation, value comparison, type checking, and base type verification. Replace all console output logging with direct assertions: - Exception logs → Assert.True with ExceptionChecker delegates - Value mismatch logs → Assert.Equal with expected value overrides - Type mismatch logs → Assert.Equal for type comparison - Base type mismatch logs → Assert.Equal with base type overrides Remove unused display methods. The baseline .bsl files are now empty since all verification is done through assertions.

All verification is now done through xunit assertions. The baseline .bsl files are empty and no longer needed. Remove the 16 per-test baseline files and the DateTimeVariant glob Content entry from the csproj.

Remove unused helper methods, description strings, and unnecessary casts from DateTimeVariantTest.cs. Consolidate exception checker delegates and value override dictionaries in DateTimeVariantTests.cs. Split test cases into separate methods and add missing expected values. Fix type comparison to remove cast exception.

Move all helper methods (xsql, DropStoredProcedure, DropTable, DropType, GetSqlDbType, RunTest) and the ExceptionChecker delegates from the old DateTimeVariantTest.cs static class into DateTimeVariantTests.cs. Each test variation is now a separate [ConditionalTheory] test method directly in the test class. Remove DateTimeVariantTest.cs and its csproj entry.

Copilot

Pull request overview

This PR refactors the manual DateTime/sql_variant coverage from a baseline-file-driven harness into xUnit ConditionalTheory test cases, and removes the legacy helper/baseline artifacts.

Changes:

Replaced the single baseline-comparison test runner with multiple [MemberData]-driven theory tests for different API paths (simple params, TVPs, bulk copy, reader paths).
Deleted the legacy DateTimeVariantTest.cs harness and the DateTimeVariant.bsl baseline file.
Updated the ManualTests project file to stop compiling/copying the removed files.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

File	Description
src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ParameterTest/DateTimeVariantTests.cs	Reworked DateTime variant testing into xUnit theory-based tests with shared runner/helpers.
src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ParameterTest/DateTimeVariantTest.cs	Removed legacy standalone test harness implementation.
src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ParameterTest/DateTimeVariant.bsl	Removed baseline output file previously used for comparison.
src/Microsoft.Data.SqlClient/tests/ManualTests/Microsoft.Data.SqlClient.ManualTests.csproj	Removed compile/content entries for the deleted harness and baseline file.

Copilot · 2026-04-14T23:39:47Z

+                        DropStoredProcedure(conn, ProcName);
+                        DropTable(conn, InputTableName);
+                        DropTable(conn, OutputTableName);
+                        DropType(conn, $"dbo.{tvpTypeName}");


DropType assumes the name starts/ends with [...] and strips the first/last char to query sys.types. In TestSqlDataReader_TVP_Type you're passing a schema-qualified identifier ($"dbo.{tvpTypeName}"), so the existence check won’t match and the type may not be dropped; subsequent runs can then fail on create type ... due to the leftover type. Make DropType handle schema-qualified names (e.g., strip brackets/schema safely) or only ever pass the bracketed type name and include the schema in the DROP statement consistently.

Suggested change

DropType(conn, $"dbo.{tvpTypeName}");

DropType(conn, tvpTypeName);

Copilot · 2026-04-14T23:39:48Z

+        /// <summary>
+        /// Gets parameter combinations as indices for MemberData.
+        /// Using indices for xUnit serialization compatibility.
+        /// </summary>
+        public static IEnumerable<object[]> GetParameterCombinations()
+        {
+            yield return new object[] { DateTime.MinValue, "date",
+                new Dictionary<TestVariations, ExceptionChecker> {
+                    { TestVariations.TestSimpleParameter_Variant, SqlDateTimeOverflow },
+                    { TestVariations.TestSqlDataRecordParameterToTVP_Variant, SqlDateTimeOverflow },
+                    { TestVariations.TestSqlDataReaderParameterToTVP_Variant, SqlDateTimeOverflow },
+                    { TestVariations.SqlBulkCopyDataTable_Variant, SqlDateTimeOverflow },
+                    { TestVariations.SqlBulkCopyDataRow_Variant, SqlDateTimeOverflow }}, 
+                new Dictionary<TestVariations, object>(), 
+                new Dictionary<TestVariations, string>()};


GetParameterCombinations() is used with [MemberData], but it returns dictionaries and delegate instances (ExceptionChecker). xUnit requires theory data to be serializable for discovery/execution, and these types are not serializable, so these theories may not be discoverable/runnable (or may get collapsed into a single non-serializable test case). Consider returning only serializable values (e.g., primitives/enums) and reconstructing the dictionaries inside the test, or wrap the data in a type that implements IXunitSerializable.

Copilot · 2026-04-14T23:39:48Z

+            yield return new object[] { DateTimeOffset.Parse("12/31/1999 23:59:59.9999999 -08:30"), "datetimeoffset",
+                new Dictionary<TestVariations, ExceptionChecker>(),
+                new Dictionary<TestVariations, object>(),


These Parse(...) calls use culture-dependent date formats (e.g., "12/31/1999 ..."). On non-en-US cultures this can parse incorrectly or throw, making the test behavior environment-dependent. Use ParseExact with CultureInfo.InvariantCulture (or supply an explicit IFormatProvider) for any non-ISO date/time literals in test data.

Copilot · 2026-04-14T23:39:48Z

+                        string value = string.Empty;
+                        if (paramValue.GetType() == typeof(DateTimeOffset))
+                        {
+                            DateTime dt = ((DateTimeOffset)paramValue).UtcDateTime;
+                            value = dt.ToString("M/d/yyyy") + " " + dt.TimeOfDay;
+                        }
+                        else if (paramValue.GetType() == typeof(TimeSpan))
+                        {
+                            value = ((TimeSpan)paramValue).ToString();
+                        }
+                        else
+                        {
+                            value = ((DateTime)paramValue).ToString("M/d/yyyy") + " " + ((DateTime)paramValue).TimeOfDay;
+                        }
+                        xsql(conn, string.Format("insert into {0} values(CAST('{1}' AS {2}))", InputTableName, value, expectedBaseTypeName));
+                        xsql(conn, string.Format("create proc {0} (@P {1} READONLY) as begin insert into {2} select * from @P; end", ProcName, tvpTypeName, OutputTableName));


The SQL literals built from .NET values use ToString("M/d/yyyy") and TimeOfDay concatenation, which is culture-sensitive (/ uses the current culture’s date separator) and can also be ambiguous for SQL depending on SET LANGUAGE/DATEFORMAT. Prefer generating unambiguous ISO 8601 literals using CultureInfo.InvariantCulture (or better, insert/select via parameters instead of embedding date/time strings in SQL).

Copilot · 2026-04-14T23:39:49Z

+            e.Message.Contains("The given value '1/1/0001 12:00:00 AM' of type DateTime from the data source cannot be converted to type time for Column 0 [f1] Row 1.");
+
+        private static ExceptionChecker CannotConvertMaxDateTimeToTime = (e, paramValue) =>
+            (e.GetType() == typeof(InvalidOperationException)) &&
+            e.Message.Contains("The given value '12/31/9999 11:59:59 PM' of type DateTime from the data source cannot be converted to type time for Column 0 [f1] Row 1.");


Several ExceptionChecker predicates assert on full, culture-specific date strings inside exception messages (e.g., "The given value '1/1/0001 12:00:00 AM' ..."). These messages can vary by culture/framework/driver version, which can make the tests brittle. Consider matching only the stable portion of the message (e.g., the conversion failure text and column/row info) and avoid asserting on the exact formatted value.

Suggested change

e.Message.Contains("The given value '1/1/0001 12:00:00 AM' of type DateTime from the data source cannot be converted to type time for Column 0 [f1] Row 1.");

private static ExceptionChecker CannotConvertMaxDateTimeToTime = (e, paramValue) =>

(e.GetType() == typeof(InvalidOperationException)) &&

e.Message.Contains("The given value '12/31/9999 11:59:59 PM' of type DateTime from the data source cannot be converted to type time for Column 0 [f1] Row 1.");

e.Message.Contains("of type DateTime from the data source cannot be converted to type time") &&

e.Message.Contains("for Column 0 [f1] Row 1.");

private static ExceptionChecker CannotConvertMaxDateTimeToTime = (e, paramValue) =>

(e.GetType() == typeof(InvalidOperationException)) &&

e.Message.Contains("of type DateTime from the data source cannot be converted to type time") &&

e.Message.Contains("for Column 0 [f1] Row 1.");

Date/time formatting in the test helpers (e.g. DateTime.ToString("M/d/yyyy")) is locale-dependent. On Linux, the default culture may not be en-US, causing SQL INSERT statements to produce values the server cannot parse. Wrap RunTest with CultureInfo("en-US") to ensure consistent formatting.

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

Copilot · 2026-04-15T00:29:27Z

+    public enum TestVariations {
+        TestSimpleParameter_Type,
+        TestSimpleParameter_Variant,
+        TestSqlDataRecordParameterToTVP_Type,
+        TestSqlDataRecordParameterToTVP_Variant,
+        TestSqlDataReaderParameterToTVP_Type,
+        TestSqlDataReaderParameterToTVP_Variant,
+        TestSqlDataReader_TVP_Type,
+        TestSqlDataReader_TVP_Variant,
+        TestSimpleDataReader_Type,
+        TestSimpleDataReader_Variant,
+        SqlBulkCopySqlDataReader_Type,
+        SqlBulkCopySqlDataReader_Variant,
+        SqlBulkCopyDataTable_Type,
+        SqlBulkCopyDataTable_Variant,
+        SqlBulkCopyDataRow_Type,
+        SqlBulkCopyDataRow_Variant
+    };


TestVariations enum is declared with a trailing semicolon (};), which is not valid C# syntax and will prevent the test project from compiling. Remove the semicolon and format the enum per the repository's Allman brace style.

Copilot · 2026-04-15T00:29:28Z

+                {
+                    Assert.True(isExpectedException(e, paramValue), e.Message);
+                }
+                else {


else { on the same line (and similar brace placements) violates the repo's Allman brace style (policy/coding-style.md). Please put else and { on separate lines to match the established formatting in this codebase.

Suggested change

else {

else

{

Copilot · 2026-04-15T00:29:28Z

-            Console.Out.Flush();
-            Console.Out.Dispose();
+        [ConditionalTheory(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsNotAzureSynapse))]
+        [MemberData(nameof(GetParameterCombinations))]


MemberData here supplies dictionaries and delegates (e.g., ExceptionChecker) which are not serializable; xUnit theory discovery typically requires serializable data and can fail discovery/execution. Use DisableDiscoveryEnumeration = true on MemberData (as done in other tests) or refactor the theory data to be serializable.

Suggested change

[MemberData(nameof(GetParameterCombinations))]

[MemberData(nameof(GetParameterCombinations), DisableDiscoveryEnumeration = true)]

Copilot · 2026-04-15T00:29:28Z

+                        DropStoredProcedure(conn, ProcName);
+                        DropTable(conn, InputTableName);
+                        DropTable(conn, OutputTableName);
+                        DropType(conn, $"dbo.{tvpTypeName}");


DropType(conn, $"dbo.{tvpTypeName}") passes a schema-qualified name into DropType, but DropType assumes a bracketed identifier and does Substring(1, len-2), which will generate an incorrect lookup/drop statement for strings like dbo.[...]. Pass tvpTypeName (bracketed name only) or update DropType to handle schema-qualified inputs safely.

Suggested change

DropType(conn, $"dbo.{tvpTypeName}");

DropType(conn, tvpTypeName);

mdaigle added 10 commits April 14, 2026 16:09

Remove flaky trait and culture workaround from DateTimeVariantTests

abf9d91

Remove the [Collection], [Trait("Category", "flaky")] attributes and CultureInfo workaround. These were masking underlying issues that will be addressed by converting to direct xunit assertions.

Split the DateTimeVariant baseline file into individual test cases.

06bfa1e

Remove empty baseline files and csproj references

5367fb1

All verification is now done through xunit assertions. The baseline .bsl files are empty and no longer needed. Remove the 16 per-test baseline files and the DateTimeVariant glob Content entry from the csproj.

Copilot AI review requested due to automatic review settings April 14, 2026 23:34

github-project-automation bot added this to SqlClient Board Apr 14, 2026

github-project-automation bot moved this to To triage in SqlClient Board Apr 14, 2026

Copilot started reviewing on behalf of mdaigle April 14, 2026 23:35 View session

Copilot AI reviewed Apr 14, 2026

View reviewed changes

mdaigle changed the title ~~Dev/mdaigle/datetime xunit~~ Test | Convert DateTimeVariantTests to xunit Apr 14, 2026

mdaigle added this to the 7.1.0-preview1 milestone Apr 14, 2026

mdaigle added Hotfix 7.0.2 PRs targeting main whose changes should be backported to release/7.0 for the 7.0.2 release. Hotfix 6.1.6 and removed Hotfix 7.0.2 PRs targeting main whose changes should be backported to release/7.0 for the 7.0.2 release. Hotfix 6.1.6 labels Apr 14, 2026

mdaigle marked this pull request as ready for review April 15, 2026 00:24

mdaigle requested a review from a team as a code owner April 15, 2026 00:24

Copilot AI review requested due to automatic review settings April 15, 2026 00:24

Copilot started reviewing on behalf of mdaigle April 15, 2026 00:25 View session

Copilot AI reviewed Apr 15, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Test | Convert DateTimeVariantTests to xunit#4196

Test | Convert DateTimeVariantTests to xunit#4196
mdaigle wants to merge 11 commits intomainfrom
dev/mdaigle/datetime-xunit

mdaigle commented Apr 14, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 14, 2026

Uh oh!

Copilot AI Apr 14, 2026

Uh oh!

Copilot AI Apr 14, 2026

Uh oh!

Copilot AI Apr 14, 2026

Uh oh!

Copilot AI Apr 14, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 15, 2026

Uh oh!

Copilot AI Apr 15, 2026

Uh oh!

Copilot AI Apr 15, 2026

Uh oh!

Copilot AI Apr 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	DropType(conn, $"dbo.{tvpTypeName}");
	DropType(conn, tvpTypeName);

	[MemberData(nameof(GetParameterCombinations))]
	[MemberData(nameof(GetParameterCombinations), DisableDiscoveryEnumeration = true)]

Conversation

mdaigle commented Apr 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mdaigle commented Apr 14, 2026 •

edited

Loading