Tests | ActiveIssues (Part 4) #3750

benrr101 · 2025-11-06T22:22:25Z

Description

This PR enables another set of tests that were previously marked as [ActiveIssue]. The active issue numbers predate our Github repository, so I cannot really know why the tests were disabled in the first place. And may of them ... just work with no modifications. Here's a rundown of the changes:

Enable tests in SqlCommandCancelTest
- TimeoutCancel (renamed TimeoutCancelTcp to differentiate)
- TimeoutCancelNP (renamed TimeoutCancelNamedPipe)
- TimeOutDuringRead (renamed TimeoutDuringReadTcp)
- TimeOutDuringReadNP (renamed TimeoutDuringReadNamedPipe)
- Rewrote the Timeout test method to use less custom helpers to make it easier to diagnose CI failures
Enable tests in SqlCredentialTest
- SqlConnectionChangePasswordPlaintext - no changes, it just worked
- SqlConnectionChangePasswordSecureString - no changes, it just worked
- Some light cleanup of SqlCredentialTest
  - Make some strings constants
  - Rename methods to match naming conventions
  - Use keyword type names for built-ins
Rewrite MARSSessionPoolingTest
- Split big bulky "one stop shop" test method into many separate tests
- Separated tests are based on matrix of scenarios and expected behavior
- Introduce back-off of DMV query to verify expected session and request count
- MARSSessionPoolingTest => MarsSessionPoolingTest

Issues

Separation of work from #3012, and investigation towards #3682

Testing

Reactivated tests were tested locally and pass
MARS tests were thoroughly ran and potential flakiness has been resolved

…elNamedPipe, TimeoutDuringReadTcp, TimeoutDuringReadNamedPipe

…ntext, SqlConnectionChangePasswordSecureString

…MarsExecuteReader_StoredProcedure_WithGC # Conflicts: # src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/MARSSessionPoolingTest/MARSSessionPoolingTest.cs

…ts are flaky

…ed to investigate if it's a bug

…to match guidelines

…able enable

Copilot

Pull Request Overview

This pull request refactors MARS (Multiple Active Result Sets) session pooling tests and fixes several test-related issues. The main focus is on rewriting the MARS session pooling tests to be more maintainable and clearer, while also fixing various test issues across multiple test files.

Rewrites MARS session pooling tests with improved structure and clarity
Removes [ActiveIssue] attributes from tests that have been fixed
Improves naming conventions in test methods and helper utilities

Reviewed Changes

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

Show a summary per file

File	Description
src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/MARSSessionPoolingTest/MarsSessionPoolingTest.cs	Complete rewrite of MARS session pooling tests with improved test organization and clarity
src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/MARSSessionPoolingTest/MARSSessionPoolingTest.cs	Deleted old MARS session pooling test file
src/Microsoft.Data.SqlClient/tests/ManualTests/Microsoft.Data.SqlClient.ManualTesting.Tests.csproj	Updated file reference to match new casing
src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlCredentialTest/SqlCredentialTest.cs	Improved code quality: made password strings const, renamed helper methods to PascalCase, changed String to string, removed fixed ActiveIssue attributes
src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlCommand/SqlCommandCancelTest.cs	Renamed test methods for clarity and removed fixed ActiveIssue attributes
src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs	Added helper method combining connection string and Synapse checks
src/Microsoft.Data.SqlClient/tests/Common/DisposableArray.cs	Added new utility class for managing disposable arrays

...rosoft.Data.SqlClient/tests/ManualTests/SQL/MARSSessionPoolingTest/MarsSessionPoolingTest.cs

…ill make it easier to sort out why we have these failures

src/Microsoft.Data.SqlClient/tests/Common/DisposableArray.cs

src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlCommand/SqlCommandCancelTest.cs

...rosoft.Data.SqlClient/tests/ManualTests/SQL/MARSSessionPoolingTest/MARSSessionPoolingTest.cs

...rosoft.Data.SqlClient/tests/ManualTests/SQL/MARSSessionPoolingTest/MarsSessionPoolingTest.cs

Copilot

Pull Request Overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

…ation

src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs

src/Microsoft.Data.SqlClient/tests/Common/DisposableArray.cs

...rosoft.Data.SqlClient/tests/ManualTests/SQL/MARSSessionPoolingTest/MARSSessionPoolingTest.cs

Adding doc comments to DisposableArray

Copilot

Pull Request Overview

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

Copilot · 2025-11-11T23:56:39Z

src/Microsoft.Data.SqlClient/tests/Common/DisposableArray.cs

+        }
+
+        /// <summary>
+        /// Gets or sets the element at index <see cref="i"/>.


The XML doc comment refers to parameter i using <see cref="i"/>, but this should use <paramref name="i"/> instead. The <see cref> tag is for referencing types, members, or properties, while <paramref> is specifically for referencing parameters within the same member's documentation.

Suggested change

/// Gets or sets the element at index <see cref="i"/>.

/// Gets or sets the element at index <paramref name="i"/>.

benrr101 added 10 commits November 5, 2025 13:34

Reenable tests in SqlCommandCancelTest: TimeoutCancelTcp, TimeoutCanc…

d543a1b

…elNamedPipe, TimeoutDuringReadTcp, TimeoutDuringReadNamedPipe

Reenable tests in SqlCredentialTest - SqlConnectionChangePasswordPlai…

cafa83b

…ntext, SqlConnectionChangePasswordSecureString

Very light cleanup of SqlCredentialTest

b5dd2fe

Reenable MarsSessionPoolingTest in MarsExecuteReader_Text_WithGC and …

0815d29

…MarsExecuteReader_StoredProcedure_WithGC # Conflicts: # src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/MARSSessionPoolingTest/MARSSessionPoolingTest.cs

Split tests into separate tests for each scenario, discover which tes…

317f456

…ts are flaky

Mostly working . Except for those no close tests

20f46f7

Clean up old code, but the nocloses test still breaks everything - ne…

c1f9ac1

…ed to investigate if it's a bug

Clean up a biiiit more

814736f

Finishing test matrix, improving DisposableArray, renaming test file …

d2a7930

…to match guidelines

Final iteration of tests, adding backoff to DMV queries, adding #null…

099bc0b

…able enable

benrr101 added this to the 7.0.0-preview3 milestone Nov 6, 2025

benrr101 requested a review from a team as a code owner November 6, 2025 22:22

Copilot AI review requested due to automatic review settings November 6, 2025 22:22

benrr101 added the Area\Tests Issues that are targeted to tests or test projects label Nov 6, 2025

Copilot AI reviewed Nov 6, 2025

View reviewed changes

benrr101 added 2 commits November 6, 2025 17:53

Fix mistakes in MARS tests

976c6fe

Rewrite TimeoutCancel to use less custom utilities - hopefully this w…

f17d382

…ill make it easier to sort out why we have these failures

paulmedynski self-assigned this Nov 7, 2025

paulmedynski requested changes Nov 7, 2025

View reviewed changes

Fix session => request comment

9dec519

Copilot AI review requested due to automatic review settings November 7, 2025 17:45

Copilot AI reviewed Nov 7, 2025

View reviewed changes

paulmedynski previously approved these changes Nov 7, 2025

View reviewed changes

Re-disable TimeoutCancelNamedPipe due to bug in managed SNI implement…

55887af

…ation

benrr101 dismissed paulmedynski’s stale review via 55887af November 10, 2025 23:35

paulmedynski previously approved these changes Nov 11, 2025

View reviewed changes

mdaigle requested changes Nov 11, 2025

View reviewed changes

Remove AreConnStringsNotAzureSynapse

27e9f3d

Adding doc comments to DisposableArray

Copilot AI review requested due to automatic review settings November 11, 2025 23:52

benrr101 dismissed paulmedynski’s stale review via 27e9f3d November 11, 2025 23:52

Copilot started reviewing on behalf of benrr101 November 11, 2025 23:52 View session

Copilot finished reviewing on behalf of benrr101 November 11, 2025 23:54

Copilot AI reviewed Nov 11, 2025

View reviewed changes

mdaigle approved these changes Nov 12, 2025

View reviewed changes

paulmedynski approved these changes Nov 12, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Tests | ActiveIssues (Part 4) #3750

Tests | ActiveIssues (Part 4) #3750

benrr101 commented Nov 6, 2025 •

edited

Loading

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!

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 Nov 11, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

	/// Gets or sets the element at index <see cref="i"/>.
	/// Gets or sets the element at index <paramref name="i"/>.

Tests | ActiveIssues (Part 4) #3750

Are you sure you want to change the base?

Tests | ActiveIssues (Part 4) #3750

Conversation

benrr101 commented Nov 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Issues

Testing

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!

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 Nov 11, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

benrr101 commented Nov 6, 2025 •

edited

Loading