Flaky tests / lighter test queries by mdaigle · Pull Request #4344 · dotnet/SqlClient

mdaigle · 2026-06-05T23:11:31Z

Marks a few flaky tests. Rewrites a manual test as a unit test. Reduces CPU/scan impact on a few high volume tests queries.

Copilot

Pull request overview

This pull request updates the SqlClient test suite by quarantining a small set of intermittently failing unit tests, replacing a manual transaction-pool cleanup scenario with a deterministic unit test, and reducing CPU/scan overhead in high-volume MARS manual tests (notably for Azure SQL environments).

Changes:

Quarantines a few simulated-server transient fault/failover tests as Category=flaky with inline failure notes where helpful.
Adds a deterministic unit test validating that pool pruning does not impact transacted connections, and exposes the pruning callback internally to support that test.
Optimizes MARS manual test queries to avoid expensive system scanning (sys.databases, sp_who) while still producing sufficient observable request activity.

Reviewed changes

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

Show a summary per file

File	Description
src/Microsoft.Data.SqlClient/tests/UnitTests/SimulatedServerTests/ConnectionTests.cs	Marks select transient-fault test cases as flaky to prevent intermittent CI failures.
src/Microsoft.Data.SqlClient/tests/UnitTests/SimulatedServerTests/ConnectionFailoverTests.cs	Quarantines an intermittently failing failover-related async test with a documented failure symptom.
src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/WaitHandleDbConnectionPoolTransactionTest.cs	Adds a pruning-focused unit test asserting transacted connections are not pruned/destroyed.
src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/MARSSessionPoolingTest/MarsSessionPoolingTest.cs	Replaces high-cost SQL text/SP usage with lighter alternatives to reduce CPU/scan impact while preserving test intent.
src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionPoolTest/TransactionPoolTest.cs	Removes the older manual transaction-cleanup test (scenario now covered deterministically via unit test).
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/WaitHandleDbConnectionPool.cs	Makes the pruning callback `internal` (with documentation) so unit tests can invoke pruning deterministically.

github-actions · 2026-06-23T18:45:01Z

@mdaigle This pull request has been marked as Author attention needed.

When you have addressed the reviewer feedback and are ready for another review, please post a comment with /ready to remove the label and re-engage reviewers.

cheenamalhotra · 2026-06-23T18:45:08Z

@mdaigle I see test failures that needs attention here.

The VALUES-based query and sp_server_info produced result sets too small for MARS flow control — the server completed requests immediately, so dm_exec_requests showed no active requests. This broke ExecuteReader_DisposeCommand which asserts open request counts. Revert to sys.databases and sp_who which produce enough data to keep server-side requests active while readers remain open. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

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

# Conflicts: # src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/WaitHandleDbConnectionPool.cs

Copilot

Pull request overview

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

codecov · 2026-07-17T18:48:42Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 65.00%. Comparing base (332e17f) to head (1941b8f).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4344      +/-   ##
==========================================
- Coverage   67.00%   65.00%   -2.00%     
==========================================
  Files         292      286       -6     
  Lines       45471    68600   +23129     
==========================================
+ Hits        30466    44593   +14127     
- Misses      15005    24007    +9002

Flag	Coverage Δ
CI-SqlClient	`?`
PR-SqlClient-Project	`65.00% <ø> (?)`

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

☔ View full report in Codecov by Harness.
📢 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.

benrr101

🚢

Preserve the removed manual cleanup test, make callback state nullable, and exercise enabled pruning with an idle control connection. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot

🟡 Changes recommended

It introduces duplicate quarantine metadata and two source-encoding policy violations.

3 open findings

Replace literal em dash with ASCII punctuation · New
Replace literal arrow with ASCII marker · New
Remove duplicate flaky test trait · New

2 resolved since last review

This test is newly marked as flaky, but unlike the other quarantined tests in this PR it doesn't…
CleanupCallback is invoked by TimerCallback with a nullable state object (and tests already…

🧠 Review effort: Balanced

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot

🔵 Needs a closer look

Quarantine documentation and trait conventions need correction, and the PR description no longer matches the current diff.

0 open findings

3 resolved since last review

Previously missed (4)

In code that hasn't changed since last review

Align PR metadata with remaining test-only changes

src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/WaitHandleDbConnectionPoolTransactionTest.cs:924

The current five-file head contains no high-volume query rewrites, although the title and description still advertise lighter test queries; the earlier MARS query file is no longer part of this diff. Update the PR metadata to reflect the remaining quarantine/unit-test scope, or restore the intended query changes.

Use canonical lowercase flaky quarantine trait

src/Microsoft.Data.SqlClient/tests/UnitTests/SimulatedServerTests/ConnectionFailoverTests.cs:755

Use the repository's canonical quarantine trait spelling, [Trait("category", "flaky")], documented in .github/instructions/testing.instructions.md:84-102. Keeping the lowercase key consistent avoids divergent test metadata.

Remove inaccurate claim about missing exception details

src/Microsoft.Data.SqlClient/tests/UnitTests/SimulatedServerTests/ConnectionTests.cs:102

This sentence contradicts the existing quarantine note immediately above, which already records a SqlException, stack frames, and the observed retry-login symptom (lines 86-95). Keep the tracking link, but remove the inaccurate claim that no exception details were retained.

Use canonical lowercase flaky quarantine trait

src/Microsoft.Data.SqlClient/tests/UnitTests/SimulatedServerTests/ConnectionTests.cs:192

Use the repository's canonical quarantine trait spelling, [Trait("category", "flaky")], documented in .github/instructions/testing.instructions.md:84-102. Keeping the lowercase key consistent avoids divergent test metadata.

🧠 Review effort: Balanced

mdaigle added 4 commits June 4, 2026 10:54

Minor fixes.

9006463

Move flaky manual test to unit test.

0458023

Mark flaky test

db12467

Mark flaky test

4be07ad

Copilot AI review requested due to automatic review settings June 5, 2026 23:11

mdaigle requested a review from a team as a code owner June 5, 2026 23:11

github-project-automation Bot added this to SqlClient Board Jun 5, 2026

github-project-automation Bot moved this to To triage in SqlClient Board Jun 5, 2026

Copilot started reviewing on behalf of mdaigle June 5, 2026 23:11 View session

mdaigle changed the title ~~Dev/mdaigle/flaky tests~~ Flaky tests / lighter test queries Jun 5, 2026

mdaigle added this to the 7.1.0-preview2 milestone Jun 5, 2026

github-actions Bot assigned apoorvdeshmukh and priyankatiwari08 Jun 5, 2026

Copilot AI reviewed Jun 5, 2026

View reviewed changes

paulmedynski moved this from To triage to In review in SqlClient Board Jun 6, 2026

paulmedynski added the Area\Tests Issues that are targeted to tests or test projects label Jun 6, 2026

paulmedynski previously approved these changes Jun 6, 2026

View reviewed changes

apoorvdeshmukh previously approved these changes Jun 8, 2026

View reviewed changes

cheenamalhotra added the Author attention needed PRs that require author to respond or make updates to PR. label Jun 23, 2026

priyankatiwari08 modified the milestones: 7.1.0-preview2, 7.1.0-preview3 Jul 2, 2026

Copilot AI review requested due to automatic review settings July 17, 2026 16:44

mdaigle dismissed apoorvdeshmukh’s stale review via 104122a July 17, 2026 16:44

Copilot started reviewing on behalf of mdaigle July 17, 2026 16:44 View session

Copilot AI reviewed Jul 17, 2026

View reviewed changes

Comment thread ...oft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/WaitHandleDbConnectionPool.cs Outdated

paulmedynski previously approved these changes Jul 17, 2026

View reviewed changes

Merge remote-tracking branch 'origin/main' into dev/mdaigle/flaky-tests

95a6922

# Conflicts: # src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/WaitHandleDbConnectionPool.cs

Copilot AI review requested due to automatic review settings July 17, 2026 17:39

mdaigle dismissed paulmedynski’s stale review via 95a6922 July 17, 2026 17:39

Copilot started reviewing on behalf of mdaigle July 17, 2026 17:40 View session

Copilot AI reviewed Jul 17, 2026

View reviewed changes

Comment thread src/Microsoft.Data.SqlClient/tests/UnitTests/SimulatedServerTests/ConnectionTests.cs

paulmedynski previously approved these changes Jul 17, 2026

View reviewed changes

priyankatiwari08 previously approved these changes Jul 23, 2026

View reviewed changes

apoorvdeshmukh previously approved these changes Jul 23, 2026

View reviewed changes

mdaigle modified the milestones: 7.1.0-preview3, 7.1.0 Aug 18, 2026

benrr101 previously approved these changes Sep 3, 2026

View reviewed changes

mdaigle modified the milestones: 7.1.0, 8.0.0-preview1 Sep 14, 2026

Merge main and address flaky-test review feedback

1921d17

Preserve the removed manual cleanup test, make callback state nullable, and exercise enabled pruning with an idle control connection. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI balanced review requested due to automatic review settings October 8, 2026 23:10

mdaigle dismissed stale reviews from benrr101, apoorvdeshmukh, priyankatiwari08, and paulmedynski via 1921d17 October 8, 2026 23:10

Copilot started reviewing on behalf of mdaigle October 8, 2026 23:11 View session

Copilot AI reviewed Oct 8, 2026

View reviewed changes

mdaigle assigned Copilot Oct 9, 2026

Address flaky test review findings

1941b8f

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI balanced review requested due to automatic review settings October 9, 2026 22:55

Copilot started reviewing on behalf of mdaigle October 9, 2026 22:56 View session

Copilot AI reviewed Oct 9, 2026

View reviewed changes

Navigation Menu

Repository navigation

Flaky tests / lighter test queries - #4344

Flaky tests / lighter test queries#4344
mdaigle wants to merge 8 commits into
mainfrom
dev/mdaigle/flaky-tests

mdaigle commented Jun 5, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

github-actions Bot commented Jun 23, 2026

Uh oh!

cheenamalhotra commented Jun 23, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

codecov Bot commented Jul 17, 2026 •

edited

Loading

Uh oh!

benrr101 left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

Conversation

mdaigle commented Jun 5, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

github-actions Bot commented Jun 23, 2026

Uh oh!

cheenamalhotra commented Jun 23, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

codecov Bot commented Jul 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

benrr101 left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

codecov Bot commented Jul 17, 2026 •

edited

Loading