Test | Fix Diagnostic Tests and remove Remote Executor

[cheenamalhotra]

Fixes AB#39873 and enables Diagnostics Test to run successfully.

[Copilot]

Pull request overview

This PR addresses AB#39873 by enabling the ManualTests diagnostics/tracing suite to run successfully under RemoteExecutor, including a build-time workaround to ensure child processes can resolve Microsoft.Data.SqlClient when the tests reference the project directly.

Changes:

• Add an MSBuild target (PatchTestDeps.targets) that patches the generated deps.json to include a runtime entry for Microsoft.Data.SqlClient when referenced via ProjectReference.
• Update TracingTests/DiagnosticTest.cs to invoke tests via RemoteExecutor with stdout/stderr forwarding to xUnit output.
• Import the new patch target from Microsoft.Data.SqlClient.ManualTests.csproj.

Reviewed changes

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

File	Description
tools/targets/PatchTestDeps.targets	Adds an inline MSBuild task to patch deps.json so dotnet exec in RemoteExecutor can resolve Microsoft.Data.SqlClient in child processes.
src/Microsoft.Data.SqlClient/tests/ManualTests/TracingTests/DiagnosticTest.cs	Refactors diagnostics tests to run via RemoteExecutor and capture child-process output.
src/Microsoft.Data.SqlClient/tests/ManualTests/Microsoft.Data.SqlClient.ManualTests.csproj	Imports the deps.json patch target for manual tests.

Comments suppressed due to low confidence (2)

src/Microsoft.Data.SqlClient/tests/ManualTests/TracingTests/DiagnosticTest.cs:234

• This test now only conditions on RemoteExecutor support, but it still connects to a real SQL Server via DataTestUtility.TCPConnectionString (and the comment indicates it fails on Azure Synapse). Without reintroducing the DataTestUtility connection-string and Synapse guards, this will run and fail in environments without manual-test config or on Synapse. Please restore the appropriate conditional checks (or combine them into a single ConditionalFact condition method) so the test is skipped when prerequisites aren’t met.

        // Synapse: Parse error at line: 1, column: 27: Incorrect syntax near 'for'.
        [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
        public void ExecuteXmlReaderTest()
        {
            InvokeRemote(() =>
            {
                CollectStatisticsDiagnostics(_ =>
                {
                    // @TODO: Test TDS server doesn't support ExecuteXmlReader, so connect to real server as workaround
                    using (SqlConnection conn = new SqlConnection(DataTestUtility.TCPConnectionString))

src/Microsoft.Data.SqlClient/tests/ManualTests/TracingTests/DiagnosticTest.cs:448

• These XML reader tests use DataTestUtility.TCPConnectionString (real server) but are no longer guarded by DataTestUtility.AreConnStringsSetup / IsNotAzureSynapse. As written they will execute whenever RemoteExecutor is supported, even when the manual-test config isn’t present, and may fail on Synapse per the comment. Please add back the connection-string + Synapse prerequisite conditions for this test.

        // Synapse:  Parse error at line: 1, column: 27: Incorrect syntax near 'for'.
        [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
        public void ExecuteXmlReaderAsyncTest()
        {
            // @TODO: TestTdsServer does not handle xml reader, so connect to a real server as a workaround
            InvokeRemote(() =>
            {
                CollectStatisticsDiagnosticsAsync(async _ =>
                {
#if NET
                    await using (SqlConnection conn = new SqlConnection(DataTestUtility.TCPConnectionString))
                    await using (SqlCommand cmd = new SqlCommand())
#else
                    using (SqlConnection conn = new SqlConnection(DataTestUtility.TCPConnectionString))

[Copilot]

@cheenamalhotra I've opened a new pull request, #4079, to work on those changes. Once the pull request is ready, I'll request review from you.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (3)

src/Microsoft.Data.SqlClient/tests/ManualTests/TracingTests/DiagnosticTest.cs:246

• ExecuteXmlReaderTest now runs whenever RemoteExecutor is supported, but the body connects via DataTestUtility.TCPConnectionString (real server). Without DataTestUtility.AreConnStringsSetup this will throw when the manual test config isn’t present, and the existing comment indicates this query is incompatible with Azure Synapse. Consider restoring the connection-string and IsNotAzureSynapse gating on the [ConditionalFact] so the test is skipped in those environments.

        // Synapse: Parse error at line: 1, column: 27: Incorrect syntax near 'for'.
        [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
        public void ExecuteXmlReaderTest()
        {
            InvokeRemote(() =>
            {
                CollectStatisticsDiagnostics(_ =>
                {
                    // @TODO: Test TDS server doesn't support ExecuteXmlReader, so connect to real server as workaround
                    using (SqlConnection conn = new SqlConnection(DataTestUtility.TCPConnectionString))
                    using (SqlCommand cmd = new SqlCommand())

src/Microsoft.Data.SqlClient/tests/ManualTests/TracingTests/DiagnosticTest.cs:461

• ExecuteXmlReaderAsyncTest connects using DataTestUtility.TCPConnectionString but the [ConditionalFact] no longer checks DataTestUtility.AreConnStringsSetup (or IsNotAzureSynapse, which the comment above still calls out). This can cause failures when manual test connection strings aren’t configured or when running against Synapse. Restore those conditions on the test attribute to keep the test appropriately gated.

        // Synapse:  Parse error at line: 1, column: 27: Incorrect syntax near 'for'.
        [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
        public void ExecuteXmlReaderAsyncTest()
        {
            // @TODO: TestTdsServer does not handle xml reader, so connect to a real server as a workaround
            InvokeRemote(() =>
            {
                CollectStatisticsDiagnosticsAsync(async _ =>
                {
#if NET
                    await using (SqlConnection conn = new SqlConnection(DataTestUtility.TCPConnectionString))
                    await using (SqlCommand cmd = new SqlCommand())
#else
                    using (SqlConnection conn = new SqlConnection(DataTestUtility.TCPConnectionString))
                    using (SqlCommand cmd = new SqlCommand())
#endif

src/Microsoft.Data.SqlClient/tests/ManualTests/TracingTests/DiagnosticTest.cs:490

• ExecuteXmlReaderAsyncErrorTest uses DataTestUtility.TCPConnectionString (real DB) but is only gated on RemoteExecutor.IsSupported. This will fail when manual test connection strings aren’t configured; it should be conditioned on DataTestUtility.AreConnStringsSetup (and likely IsNotAzureSynapse as with the other FOR XML tests) to avoid running in unsupported environments.

        [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
        public void ExecuteXmlReaderAsyncErrorTest()
        {
            // @TODO: TestTdsServer does not handle xml reader, so connect to a real server as a workaround
            InvokeRemote(() =>
            {

                CollectStatisticsDiagnosticsAsync(async _ =>
                {
#if NET
                    await using (SqlConnection conn = new SqlConnection(DataTestUtility.TCPConnectionString))
                    await using (SqlCommand cmd = new SqlCommand())
#else

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 66.14%. Comparing base (60d4b92) to head (a7d283c).

❗ There is a different number of reports uploaded between BASE (60d4b92) and HEAD (a7d283c). Click for more details.

HEAD has 1 upload less than BASE

Flag	BASE (60d4b92)	HEAD (a7d283c)
CI-SqlClient	1	0

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4078      +/-   ##
==========================================
- Coverage   73.22%   66.14%   -7.09%     
==========================================
  Files         280      275       -5     
  Lines       43000    65827   +22827     
==========================================
+ Hits        31486    43540   +12054     
- Misses      11514    22287   +10773     

Flag	Coverage Δ
CI-SqlClient	?
PR-SqlClient-Project	66.14% <100.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (3)

src/Microsoft.Data.SqlClient/tests/ManualTests/TracingTests/DiagnosticTest.cs:246

• This test now runs whenever RemoteExecutor is supported, but it still uses DataTestUtility.TCPConnectionString (real server) without checking DataTestUtility.AreConnStringsSetup, and the comment above indicates Synapse incompatibility. This will fail in environments without config.json or when targeting Azure Synapse. Reintroduce the connection-string and Synapse skip conditions for this test (and keep the RemoteExecutor gate if needed).

        // Synapse: Parse error at line: 1, column: 27: Incorrect syntax near 'for'.
        [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
        public void ExecuteXmlReaderTest()
        {
            InvokeRemote(() =>
            {
                CollectStatisticsDiagnostics(_ =>
                {
                    // @TODO: Test TDS server doesn't support ExecuteXmlReader, so connect to real server as workaround
                    using (SqlConnection conn = new SqlConnection(DataTestUtility.TCPConnectionString))
                    using (SqlCommand cmd = new SqlCommand())

src/Microsoft.Data.SqlClient/tests/ManualTests/TracingTests/DiagnosticTest.cs:461

• This async XML reader test now only checks RemoteExecutor.IsSupported, but it connects to a real server via DataTestUtility.TCPConnectionString. Without gating on DataTestUtility.AreConnStringsSetup (and Synapse exclusion per the comment), this will fail when manual-test config isn’t present or on Synapse. Add back the appropriate ConditionalFact conditions (potentially via a combined helper condition).

        // Synapse:  Parse error at line: 1, column: 27: Incorrect syntax near 'for'.
        [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
        public void ExecuteXmlReaderAsyncTest()
        {
            // @TODO: TestTdsServer does not handle xml reader, so connect to a real server as a workaround
            InvokeRemote(() =>
            {
                CollectStatisticsDiagnosticsAsync(async _ =>
                {
#if NET
                    await using (SqlConnection conn = new SqlConnection(DataTestUtility.TCPConnectionString))
                    await using (SqlCommand cmd = new SqlCommand())
#else
                    using (SqlConnection conn = new SqlConnection(DataTestUtility.TCPConnectionString))
                    using (SqlCommand cmd = new SqlCommand())
#endif

src/Microsoft.Data.SqlClient/tests/ManualTests/TracingTests/DiagnosticTest.cs:492

• This async XML reader error test also uses DataTestUtility.TCPConnectionString but no longer checks DataTestUtility.AreConnStringsSetup. That can cause null/invalid connection strings (and unexpected failures) when config.json isn’t configured. Restore the connection-string setup condition (and keep any other environment guards needed).

        [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
        public void ExecuteXmlReaderAsyncErrorTest()
        {
            // @TODO: TestTdsServer does not handle xml reader, so connect to a real server as a workaround
            InvokeRemote(() =>
            {

                CollectStatisticsDiagnosticsAsync(async _ =>
                {
#if NET
                    await using (SqlConnection conn = new SqlConnection(DataTestUtility.TCPConnectionString))
                    await using (SqlCommand cmd = new SqlCommand())
#else
                    using (SqlConnection conn = new SqlConnection(DataTestUtility.TCPConnectionString))
                    using (SqlCommand cmd = new SqlCommand())