DRAFT | Wire Manual Tests to Common MDS #3916

benrr101 · 2026-01-27T00:40:39Z

Wiring up manual tests project to common MDS. Since a lot of these tests fail locally for me on main, I'm going to run here to validate. This probably breaks all the things.

…ts, rework test set parameter, group the easy files to group.

…why this is just now a problem, but ok, I'll fix it.

Copilot

Pull request overview

Wires the ManualTests project to the common Microsoft.Data.SqlClient (MDS) build/test infrastructure, including OS-conditional compilation and updated build orchestration.

Changes:

Reworks Microsoft.Data.SqlClient.ManualTests.csproj to compile against common test infrastructure, support “test sets”, and add OS preprocessor constants.
Adds OS / framework guards (#if _WINDOWS, #if NET, #if NETFRAMEWORK, #if DEBUG) to prevent cross-OS/framework compilation issues.
Updates build/solution wiring (project rename/path changes, build target adjustments, test invocation changes).

Reviewed changes

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

Show a summary per file

File	Description
src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/UdtTest/SqlServerTypesTest.cs	Refactors UDT read tests to parameterize behavior and centralize query/expected bytes.
src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/TransactionTest/DistributedTransactionTest.windows.cs	Wraps Windows-only test with `_WINDOWS` compilation guard.
src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlDSEnumeratorTest/SqlDataSourceEnumeratorTest.windows.cs	Wraps Windows-only enumerator test with `_WINDOWS` compilation guard and removes platform attributes.
src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlCommand/SqlCommandCancelTest.cs	Renames/reshapes test methods (TCP/NamedPipe/MARS variants) and modernizes async test signatures.
src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionPoolTest/TransactionPoolTest.netfx.cs	Wraps .NET Framework-only test code with `NETFRAMEWORK` guard.
src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionPoolTest/ConnectionPoolTest.Debug.cs	Adds license header and gates debug-only helpers behind `#if DEBUG`.
src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/Batch/BatchTests.netcore.cs	Wraps netcore-only tests behind `#if NET` and adjusts conditional compilation.
src/Microsoft.Data.SqlClient/tests/ManualTests/Microsoft.Data.SqlClient.ManualTests.csproj	Major restructure: OS constants, test-set compilation model, new reference wiring, and content/resource copy rules.
src/Microsoft.Data.SqlClient/tests/ManualTests/AlwaysEncrypted/TestFixtures/SqlSetupStrategyCspProvider.windows.cs	Adds `_WINDOWS` guard for CSP setup fixture.
src/Microsoft.Data.SqlClient/tests/ManualTests/AlwaysEncrypted/TestFixtures/Setup/CspProviderColumnMasterKey.windows.cs	Adds `_WINDOWS` guard for CSP CMK helper.
src/Microsoft.Data.SqlClient/tests/ManualTests/AlwaysEncrypted/DateOnlyReadTests.netcore.cs	Adds `NET` guard for netcore-only DateOnly tests.
src/Microsoft.Data.SqlClient/tests/ManualTests/AlwaysEncrypted/CspProviderExt.windows.cs	Adds `_WINDOWS` guard and removes runtime platform attributes.
src/Microsoft.Data.SqlClient/tests/FunctionalTests/Microsoft.Data.SqlClient.FunctionalTests.csproj	Updates comments and changes how `xunit.runner.json` is included/copied.
src/Microsoft.Data.SqlClient/add-ons/AzureKeyVaultProvider/Microsoft.Data.SqlClient.AlwaysEncrypted.AzureKeyVaultProvider.csproj	Reworks output/reference properties and reference-to-MDS strategy.
src/Microsoft.Data.SqlClient.sln	Renames ManualTests project entry to `Microsoft.Data.SqlClient.ManualTests`.
build.proj	Updates ManualTests project path and changes test build/run targets and command formatting.

Comments suppressed due to low confidence (3)

src/Microsoft.Data.SqlClient/tests/ManualTests/Microsoft.Data.SqlClient.ManualTests.csproj:374

The netcore ItemGroup includes Microsoft.Data.SqlClient.TestCommon.csproj twice (once via $(RepoRoot) and once via $(TestsPath)). This can lead to duplicate compile items / assembly conflicts. Keep only one reference and standardize on a single path pattern for this repo.
src/Microsoft.Data.SqlClient/tests/ManualTests/AlwaysEncrypted/TestFixtures/SqlSetupStrategyCspProvider.windows.cs:4
The license header line appears to have an accidental using System; appended ("...more information.using System;"). This looks like a copy/paste typo and should be split so the comment ends at the period and the using is on its own line (or removed if redundant).
src/Microsoft.Data.SqlClient/tests/ManualTests/Microsoft.Data.SqlClient.ManualTests.csproj:381
Duplicate project reference: $(TestsPath)Common/Microsoft.Data.SqlClient.TestCommon.csproj is referenced here, but the same project is already referenced earlier in this ItemGroup via $(RepoRoot)/src/Microsoft.Data.SqlClient/tests/Common/.... Remove one to avoid duplicate builds/inputs.

src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/UdtTest/SqlServerTypesTest.cs

build.proj

.../AzureKeyVaultProvider/Microsoft.Data.SqlClient.AlwaysEncrypted.AzureKeyVaultProvider.csproj

...crosoft.Data.SqlClient/tests/FunctionalTests/Microsoft.Data.SqlClient.FunctionalTests.csproj

src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/UdtTest/SqlServerTypesTest.cs

…rverTypesTest

…ill come back to address pipeline and build.proj issues

Copilot

Pull request overview

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

Comments suppressed due to low confidence (3)

src/Microsoft.Data.SqlClient/tests/ManualTests/AlwaysEncrypted/TestFixtures/SqlSetupStrategyCspProvider.windows.cs:7

The header comment and a using directive appear to be accidentally concatenated: // ... information.using System;. This reads like a typo and can confuse readers.

Consider splitting this into a normal license header line and leaving using System; as an actual using directive below.
src/Microsoft.Data.SqlClient/tests/ManualTests/Microsoft.Data.SqlClient.ManualTests.csproj:36

ValidateOs compares $(TargetOS) to $(OS), but this project defines TargetOs (lowercase s) and there is no TargetOS property in the repo. As written, $(TargetOS) will be empty and the build will fail with the <Error> on every build.

Use $(TargetOs) (or $(NormalizedTargetOs)) in the condition so the check only triggers when the OS is intentionally overridden.
src/Microsoft.Data.SqlClient/tests/ManualTests/Microsoft.Data.SqlClient.ManualTests.csproj:379

The netcore ItemGroup includes Microsoft.Data.SqlClient.TestCommon.csproj twice (once via $(RepoRoot) and once via $(TestsPath)), and these paths resolve to the same file. Keeping both can cause duplicate ProjectReference items and makes dependency management harder.

Remove one of the two references and standardize on a single path style.

src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/UdtTest/SqlServerTypesTest.cs

build.proj

.../AzureKeyVaultProvider/Microsoft.Data.SqlClient.AlwaysEncrypted.AzureKeyVaultProvider.csproj

build.proj

Copilot

Pull request overview

Copilot reviewed 17 out of 22 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (2)

src/Microsoft.Data.SqlClient/tests/ManualTests/AlwaysEncrypted/TestFixtures/SqlSetupStrategyCspProvider.windows.cs:3

The header comment appears to accidentally include "using System;" ("...more information.using System;"). Remove the stray text so the header comment matches the standard license header format.
src/Microsoft.Data.SqlClient/tests/ManualTests/Microsoft.Data.SqlClient.ManualTests.csproj:399
The netcore ItemGroup includes Microsoft.Data.SqlClient.TestCommon.csproj twice (once via $(RepoRoot)/... and again via $(TestsPath)...). This can cause duplicate project references and unpredictable build/copy-to-output behavior. Keep only one reference (prefer the existing $(TestsPath) form for consistency with the rest of the file).

Copilot · 2026-01-28T20:43:08Z

.../AzureKeyVaultProvider/Microsoft.Data.SqlClient.AlwaysEncrypted.AzureKeyVaultProvider.csproj

+
+  <!-- Build Output ==================================================== -->
+  <PropertyGroup>
+    <!-- @TODO: $(BindFolder) is OS dependent but we're only ever building AnyOS, so ... can we simplify this? -->


Typo in the comment: "$(BindFolder)" doesn't match the actual property name used elsewhere ("$(BinFolder)").

Suggested change

Copilot · 2026-01-28T20:43:09Z

build.proj

  <Target Name="BuildAKVNetFx" Condition="'$(IsEnabledWindows)' == 'true'">
-    <MSBuild Projects="@(AKVProvider)" Targets="restore" Properties="TestTargetOS=$(TestOS)netfx" />
-    <Message Text=">>> Building AKVNetFx [$(CI);TestTargetOS=$(TestOS)netfx;Platform=AnyCPU;$(TestProjectProperties)] ..." Condition="!$(ReferenceType.Contains('Package'))" />
-    <MSBuild Projects="@(AKVProvider)" Properties="$(CI);TestTargetOS=$(TestOS)netfx;Platform=AnyCPU;$(TestProjectProperties);$(NugetPackProperties);" Condition="!$(ReferenceType.Contains('Package'))" />
-
-    <!-- Only build platform specific builds for Package reference types -->
-    <Message Text=">>> Building AKVNetFx [$(CI);TestTargetOS=$(TestOS)netfx;Platform=$(Platform);$(TestProjectProperties)] ..." Condition="$(ReferenceType.Contains('Package'))" />
-    <MSBuild Projects="@(AKVProvider)" Properties="$(CI);TestTargetOS=$(TestOS)netfx;Platform=$(Platform);$(TestProjectProperties);$(NugetPackProperties);" Condition="$(ReferenceType.Contains('Package'))" />
+    <Message Text=">>> No op. This will be removed" />
  </Target>

  <Target Name="BuildAKVNetCore">
-    <MSBuild Projects="@(AKVProvider)" Targets="restore" Properties="TestTargetOS=$(TestOS)netcoreapp" />
-    <Message Text=">>> Building AKVNetCore [$(CI);TestTargetOS=$(TestOS)netcoreapp;$(ProjectProperties);Platform=AnyCPU;ReferenceType=$(ReferenceType);] ..." Condition="!$(ReferenceType.Contains('Package'))" />
-    <MSBuild Projects="@(AKVProvider)" Properties="$(CI);TestTargetOS=$(TestOS)netcoreapp;$(ProjectProperties);Platform=AnyCPU;" Condition="!$(ReferenceType.Contains('Package'))" />
-
-    <!-- Only build platform specific builds for Package reference types -->
-    <Message Text=">>> Building AKVNetCore [$(CI);TestTargetOS=$(TestOS)netcoreapp;$(ProjectProperties);Platform=$(Platform);ReferenceType=$(ReferenceType);] ..." Condition="$(ReferenceType.Contains('Package'))" />
-    <MSBuild Projects="@(AKVProvider)" Properties="$(CI);TestTargetOS=$(TestOS)netcoreapp;$(ProjectProperties);Platform=$(Platform);" Condition="$(ReferenceType.Contains('Package'))" />
+    <Message Text=">>> No op. This will be removed" />
  </Target>

  <Target Name="BuildAKVNetCoreAllOS">
-    <MSBuild Projects="@(AKVProvider)" Targets="restore" Properties="TestTargetOS=$(TestOS)netcoreapp" />
-    <MSBuild Projects="@(AKVProvider)" Properties="$(CI);TestTargetOS=$(TestOS)netcoreapp;$(ProjectProperties);Platform=AnyCPU;OSGroup=Unix;" RemoveProperties="TargetsWindows;TargetsUnix;" />
-    <MSBuild Projects="@(AKVProvider)" Properties="$(CI);TestTargetOS=$(TestOS)netcoreapp;$(ProjectProperties);Platform=AnyCPU;OSGroup=Windows_NT;" RemoveProperties="TargetsWindows;TargetsUnix;" Condition="'$(IsEnabledWindows)' == 'true'" />
-    <MSBuild Projects="@(AKVProvider)" Properties="$(CI);TestTargetOS=$(TestOS)netcoreapp;$(ProjectProperties);Platform=AnyCPU;OSGroup=AnyOS;" RemoveProperties="TargetsWindows;TargetsUnix;" />
+    <Message Text=">>> No op. This will be removed" />
  </Target>


BuildAKVNetFx/BuildAKVNetCore/BuildAKVNetCoreAllOS are now no-ops, but CI templates invoke these targets (e.g., eng/pipelines/**). This will prevent the AKV provider from being built where expected. Either restore the original build logic for these targets or update the pipeline to use the new BuildAkv target in the same change set.

Copilot · 2026-01-28T20:43:09Z

src/Microsoft.Data.SqlClient/tests/ManualTests/DDBasics/DDDataTypesTest/DDDataTypesTest.cs


                    cmd2.Parameters.Add("@x", SqlDbType.Xml);
-                    XmlReader xr = XmlReader.Create("data.xml");
+                    XmlReader xr = XmlReader.Create("DDDataTypesTest_Data.xml");


Disposable 'XmlReader' is created but not disposed.

benrr101 added 12 commits January 15, 2026 17:29

Rename manual test project csproj

336ef5a

Start cleaning up the ManualTests project file - introduce OS constan…

fc241f3

…ts, rework test set parameter, group the easy files to group.

Windows only files for AE tests

c73ec06

More conditionally included files

d320804

fix casing on DistributedTransactionTest

94dec48

Rest of the conditional files.

8ccddd5

References and such

61eb660

Comments from *last* PR

15e61d9

Point AKV Provider at the common MDS project

4ac324f

Fix tests that had names that conflict with helper methods. Not sure …

71d2c73

…why this is just now a problem, but ok, I'll fix it.

Finish work on the Manual tests csproj

6a52161

Wire up manual test targets as per last two projects

94d21f7

Copilot AI review requested due to automatic review settings January 27, 2026 00:40

Copilot started reviewing on behalf of benrr101 January 27, 2026 00:41 View session

Copilot AI reviewed Jan 27, 2026

View reviewed changes

benrr101 added 3 commits January 27, 2026 13:15

Remove unnecesary projects from build.proj, copilot comments on SqlSe…

1734137

…rverTypesTest

Replace $(AddonName) with AzureKeyVaultProvider

b1a43d6

I just want to see if I can get the manual tests running correctly. W…

e1fd1af

…ill come back to address pipeline and build.proj issues

Copilot AI review requested due to automatic review settings January 27, 2026 20:06

Copilot started reviewing on behalf of benrr101 January 27, 2026 20:06 View session

Copilot AI reviewed Jan 27, 2026

View reviewed changes

benrr101 added 2 commits January 28, 2026 14:29

Move test data files, fix references

80838ed

Comments from copilot

7377604

Copilot AI review requested due to automatic review settings January 28, 2026 20:36

Copilot started reviewing on behalf of benrr101 January 28, 2026 20:36 View session

Copilot AI reviewed Jan 28, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DRAFT | Wire Manual Tests to Common MDS #3916

DRAFT | Wire Manual Tests to Common MDS #3916

Uh oh!

benrr101 commented Jan 27, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jan 28, 2026

Uh oh!

Copilot AI Jan 28, 2026

Uh oh!

Copilot AI Jan 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	<!-- @TODO: $(BindFolder) is OS dependent but we're only ever building AnyOS, so ... can we simplify this? -->
	<!-- @TODO: $(BinFolder) is OS dependent but we're only ever building AnyOS, so ... can we simplify this? -->

DRAFT | Wire Manual Tests to Common MDS #3916

Are you sure you want to change the base?

DRAFT | Wire Manual Tests to Common MDS #3916

Uh oh!

Conversation

benrr101 commented Jan 27, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants