Skip to content

[6.1] Test | Fix Transient Fault handling and other flaky unit tests (#4080)#4269

Open
cheenamalhotra wants to merge 1 commit into
release/6.1from
dev/cheena/6.1-transient-fault-fixes
Open

[6.1] Test | Fix Transient Fault handling and other flaky unit tests (#4080)#4269
cheenamalhotra wants to merge 1 commit into
release/6.1from
dev/cheena/6.1-transient-fault-fixes

Conversation

@cheenamalhotra
Copy link
Copy Markdown
Member

Ports #4080 to release/6.1 branch.

* Test | Fix Transient Fault handling flaky tests
* Attempt to fix
* Fix the MultiPartIdentifier tests getting skipped
* Fix serialization issue of SqlTypeWorkaroundsTests
* Fix one more cases of possible error scenarios
Copilot AI review requested due to automatic review settings May 11, 2026 20:33
@cheenamalhotra cheenamalhotra requested a review from a team as a code owner May 11, 2026 20:33
@github-project-automation github-project-automation Bot moved this to To triage in SqlClient Board May 11, 2026
@cheenamalhotra cheenamalhotra changed the title Test | Fix Transient Fault handling and other flaky unit tests (#4080) [6.1] Test | Fix Transient Fault handling and other flaky unit tests (#4080) May 11, 2026
@cheenamalhotra cheenamalhotra added the Area\Tests Issues that are targeted to tests or test projects label May 11, 2026
@cheenamalhotra cheenamalhotra added this to the 6.1.6 milestone May 11, 2026
@cheenamalhotra cheenamalhotra marked this pull request as draft May 11, 2026 20:33
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Ports the fixes from #4080 onto release/6.1, primarily stabilizing simulated-server unit tests around transient fault retry behavior on .NET Framework (TNIR/interval-timer retries) and addressing a few other flaky-test/discovery issues.

Changes:

  • Make simulated-server retry assertions deterministic by tracking Login7 attempts and accounting for “abandoned” PreLogin attempts.
  • Reduce flakiness from pooling interactions in several simulated-server failover/routing tests (e.g., disabling pooling in specific cases, clearing pools in one test).
  • Fix xUnit discovery issues by setting DisableDiscoveryEnumeration = true on selected [MemberData] usages.

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/UnitTests/SimulatedServerTests/ConnectionTests.cs Updates transient/network retry assertions and adjusts connection-string settings to reduce flakiness.
src/Microsoft.Data.SqlClient/tests/UnitTests/SimulatedServerTests/ConnectionRoutingTestsAzure.cs Updates routing retry assertions to account for abandoned pre-logins.
src/Microsoft.Data.SqlClient/tests/UnitTests/SimulatedServerTests/ConnectionRoutingTests.cs Updates routing retry assertions to account for abandoned pre-logins.
src/Microsoft.Data.SqlClient/tests/UnitTests/SimulatedServerTests/ConnectionFailoverTests.cs Adjusts pooling behavior and assertions for failover scenarios; adds pool clearing in one test.
src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/SqlTypes/SqlTypeWorkaroundsTests.cs Uses DisableDiscoveryEnumeration to fix test discovery/enumeration issues.
src/Microsoft.Data.SqlClient/tests/tools/TDS/TDS.Servers/TransientTdsErrorTdsServer.cs Ensures Login7 is counted even when returning an error without calling base handler.
src/Microsoft.Data.SqlClient/tests/tools/TDS/TDS.Servers/GenericTdsServer.cs Adds Login7Count + derived abandoned-prelogin metric; refactors endpoint initialization; increments login count on login handler.

Comment on lines 243 to +249
/// <summary>
/// Handler for login request
/// </summary>
public virtual TDSMessageCollection OnLogin7Request(ITDSServerSession session, TDSMessage request)
{
Interlocked.Increment(ref _login7Count);

Comment on lines 113 to +125
public int PreLoginCount => _preLoginCount;

/// <summary>
/// Counts Login7 requests to the server.
/// </summary>
public int Login7Count => _login7Count;

/// <summary>
/// Counts pre-login requests that did not result in a Login7 request,
/// which indicates the client abandoned the connection (e.g. interval
/// timer timeout during TNIR or failover).
/// </summary>
public int AbandonedPreLoginCount => _preLoginCount - _login7Count;
Assert.Equal($"localhost,{failoverServer.EndPoint.Port}", connection.DataSource);
Assert.Equal(1, server.PreLoginCount);
Assert.Equal(1, failoverServer.PreLoginCount);
Assert.Equal(0, server.Login7Count);
Comment on lines 102 to 106
await connection.OpenAsync();
Assert.Equal(ConnectionState.Open, connection.State);
Assert.Equal($"localhost,{server.EndPoint.Port}", connection.DataSource);
Assert.Equal(2, server.PreLoginCount);
Assert.Equal(2, server.PreLoginCount - server.AbandonedPreLoginCount);
}
@cheenamalhotra cheenamalhotra marked this pull request as ready for review May 11, 2026 21:15
@cheenamalhotra cheenamalhotra moved this from To triage to In review in SqlClient Board May 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area\Tests Issues that are targeted to tests or test projects

Projects

Status: In review

Development

Successfully merging this pull request may close these issues.

4 participants