Address flaky DEBUG assertions

[paulmedynski]

Description

Refactors the DataTestUtility.AssertThrowsWrapper assertion helpers and fixes race condition test failures across the manual test suite.

Problem

1. Flaky assertion regression: A DEBUG assertion started failing on Ubuntu .NET 8 in PR pipelines, causing test failures in DataStreamTest (see SqlDataReader async read throws inconsistent exception types on disposal (IOException vs InvalidOperationException) #4088, Fix commented-out Debug.Assert()s that are failing #3604).
2. Race conditions in async disposal: When PendAsyncReadsScope is disposed while an async ReadAsync is in-flight, the inner exception type varies depending on timing — it may be IOException, InvalidOperationException, ObjectDisposedException, or TaskCanceledException. The existing assertion helpers only accepted a single expected type and would fail intermittently.
3. Misleading assertion helpers: AssertThrowsWrapper accepted generic type parameters for inner/inner-inner exceptions but never validated them with actual type checks. It also had unused parameters (innerExceptionMustBeNull, customExceptionVerifier, innerInnerInnerExceptionMustBeNull) that no callers were passing.
4. Duplicated exception-handling code: Six identical try { t.Wait(); } catch (AggregateException) { ... } blocks were scattered throughout DataStreamTest.cs.

Changes

DataTestUtility.cs — Assertion helper redesign

Replaced the three AssertThrowsWrapper overloads with four properly-typed, well-documented helpers:

Old Method	New Method	What Changed
AssertThrowsWrapper<T>(...)	AssertThrows<T>(...)	Removed unused params (innerExceptionMustBeNull, customExceptionVerifier)
AssertThrowsWrapper<T, TInner>(...)	AssertThrowsInner<T, TInner>(...)	Now actually validates the inner exception type via Assert.IsAssignableFrom<TInnerException>
AssertThrowsWrapper<T, TInner, TInnerInner>(...)	AssertThrowsInnerWithAlternate<T, TInner, TAlt>(...)	Repurposed: accepts two possible inner exception types for race conditions instead of checking inner-inner exceptions

All helpers are wrapped in a #nullable enable block with XML documentation.

DataStreamTest.cs — Race condition fixes

• Extracted WaitIgnoringFlakyException(Task): Replaces 6 duplicated try/catch blocks. Calls task.Wait() and ignores AggregateException only for faulted tasks (with a comment explaining that a faulted Task permanently stores its exception, so re-waiting always rethrows).
• Fixed race-prone assertion sites using AssertThrowsInnerWithAlternate:
  • Lines ~1407, ~1797, ~1813: IOException ↔ InvalidOperationException depending on reader disposal timing
  • Line ~1828: TaskCanceledException ↔ InvalidOperationException depending on PendAsyncReadsScope disposal timing
  • Lines ~1841, ~1969: Inline assertions handling IOException → SqlException or ObjectDisposedException alternate path
• Removed all TODO(GH-3604) comments — replaced with proper alternate-type assertions that handle the race correctly.
• Added explanatory comments at each race-prone site referencing SqlDataReader async read throws inconsistent exception types on disposal (IOException vs InvalidOperationException) #4088.

22 other test files — Assertion renames

All call sites across the manual test suite updated from AssertThrowsWrapper to the new names:

• AssertThrowsWrapper<T>(...) → AssertThrows<T>(...)
• AssertThrowsWrapper<T, TInner>(...) → AssertThrowsInner<T, TInner>(...)

Files: AdapterTest, LocalDBTest, MARSTest, ParallelTransactionsTest, ParametersTest, 10 SqlBulkCopy tests, SqlCommandCancelTest, SqlNotificationTest, TransactionTest, UdtTest2, CopyAllFromReaderConnectionCloseOnEventAsync.

Minor cleanups

• Fixed #pragma warning indentation in DataTestUtility.cs
• Changed #nullable disable → #nullable restore for proper nullable context scoping

Testing

• Verified all assertion call sites compile and pass locally
• CI pipeline validates across all configurations (Windows/Linux/macOS, net462/net8.0/net9.0, ManagedSNI/NativeSNI, SQL Server 2019/2022/Azure SQL/Named Instance/ARM64)
• Confirmed the only remaining CI failure (ReadStream_ReadsStreamDataCorrectly line 1828) is fixed by the AssertThrowsInnerWithAlternate change

[Copilot]

Pull request overview

This PR updates DEBUG-only sections of the manual DataStreamTest to stop asserting that certain Task.Wait() calls must throw, because (per GH-3604) the exception behavior is race-dependent on some platforms (notably Ubuntu/.NET 8 in PR pipelines).

Changes:

• Replaces several AssertThrowsWrapper<AggregateException, IOException>(() => t.Wait()) assertions with a try { t.Wait(); } catch (AggregateException ...) { ... } pattern that tolerates “no throw” outcomes.
• Keeps validation of the expected failure mode when an exception does occur (expects an IO-related failure).
• Applies minor whitespace cleanup in a few places.

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

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

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 66.43%. Comparing base (60d4b92) to head (b57c81b).
⚠️ Report is 13 commits behind head on main.

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

HEAD has 1 upload less than BASE

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

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4085      +/-   ##
==========================================
- Coverage   73.22%   66.43%   -6.80%     
==========================================
  Files         280      275       -5     
  Lines       43000    65804   +22804     
==========================================
+ Hits        31486    43715   +12229     
- Misses      11514    22089   +10575     

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

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 24 out of 24 changed files in this pull request and generated 2 comments.

[paulmedynski]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 2 pipeline(s).