Skip to content

[7.0] Test | Fix Transient Fault handling and other flaky unit tests #4114

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
paulmedynski wants to merge 1 commit into from
Closed

[7.0] Test | Fix Transient Fault handling and other flaky unit tests #4114

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions ...icrosoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/Common/MultipartIdentifierTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ public static TheoryData<string, string[]> ValidSinglePartIdentifierVariations
{
ReadOnlySpan<string> part1Words = ["word1", "word 1"];
TheoryData<string, string[]> data = [];
HashSet<string> seen = [];

// Combination 1: embedded and non-embedded whitespace.
// Combination 2: leading and/or trailing whitespace, and no whitespace
Expand All @@ -36,6 +37,13 @@ public static TheoryData<string, string[]> ValidSinglePartIdentifierVariations
{
foreach ((string p1Combination, string p1Expected) in GeneratePartCombinations(part1))
{
// Skip duplicates — different generation paths can produce
// identical (input, expected) pairs, which xUnit rejects.
if (!seen.Add(p1Combination))
{
continue;
}

string onePartCombination = p1Combination;
string[] onePartExpected = [p1Expected];

Expand Down
6 changes: 3 additions & 3 deletions ...crosoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/SqlTypes/SqlTypeWorkaroundsTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ public void ByteArrayToSqlBinary_NullInput()
};

[Theory]
[MemberData(nameof(SqlDecimalWriteTdsValue_NonNullInput_Data))]
[MemberData(nameof(SqlDecimalWriteTdsValue_NonNullInput_Data), DisableDiscoveryEnumeration = true)]
public void SqlDecimalWriteTdsValue_NonNullInput(SqlDecimal input)
{
// Arrange
Expand Down Expand Up @@ -155,7 +155,7 @@ public void ByteArrayToSqlGuid_ValidInput(byte[] input)
};

[Theory]
[MemberData(nameof(LongToSqlMoney_Data))]
[MemberData(nameof(LongToSqlMoney_Data), DisableDiscoveryEnumeration = true)]
public void LongToSqlMoney(long input, SqlMoney expected)
{
// Act
Expand All @@ -176,7 +176,7 @@ public void LongToSqlMoney(long input, SqlMoney expected)
};

[Theory]
[MemberData(nameof(SqlMoneyToLong_NonNullInput_Data))]
[MemberData(nameof(SqlMoneyToLong_NonNullInput_Data), DisableDiscoveryEnumeration = true)]
public void SqlMoneyToLong_NonNullInput(SqlMoney input, long expected)
{
// Act
Expand Down
29 changes: 19 additions & 10 deletions src/Microsoft.Data.SqlClient/tests/UnitTests/SimulatedServerTests/ConnectionFailoverTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@

namespace Microsoft.Data.SqlClient.UnitTests.SimulatedServerTests
{
[Trait("Category", "flaky")]
[Collection("SimulatedServerTests")]
public class ConnectionFailoverTests
{
Expand Down Expand Up @@ -70,7 +69,7 @@ public void TransientFault_NoFailover_DoesNotClearPool(uint errorCode)
Assert.Equal($"localhost,{initialServer.EndPoint.Port}", secondConnection.DataSource);

// 1 for the initial connection, 2 for the second connection
Assert.Equal(3, initialServer.PreLoginCount);
Assert.Equal(3, initialServer.PreLoginCount - initialServer.AbandonedPreLoginCount);
// A failover should not be triggered, so prelogin count to the failover server should be 0
Assert.Equal(0, failoverServer.PreLoginCount);
}
Expand Down Expand Up @@ -219,6 +218,7 @@ public void NetworkDelay_ShouldConnectToPrimary()
InitialCatalog = "master", // Required for failover partner to work
ConnectTimeout = 5,
Encrypt = false,
Pooling = false, // Disable pooling to ensure a fresh connection attempt is made
MultiSubnetFailover = false,
#if NETFRAMEWORK
TransparentNetworkIPResolution = false,
Expand Down Expand Up @@ -268,6 +268,7 @@ public void NetworkError_WithUserProvidedPartner_RetryDisabled_ShouldConnectToFa
ConnectRetryCount = 0, // Disable retry
FailoverPartner = $"localhost,{failoverServer.EndPoint.Port}", // User provided failover partner
Encrypt = false,
Pooling = false, // Disable pooling to ensure a fresh connection attempt is made on failover
};
using SqlConnection connection = new(builder.ConnectionString);

Expand Down Expand Up @@ -313,6 +314,9 @@ public void NetworkError_WithUserProvidedPartner_RetryEnabled_ShouldConnectToFai
ConnectRetryInterval = 1,
FailoverPartner = $"localhost,{failoverServer.EndPoint.Port}", // User provided failover partner
Encrypt = false,
#if NETFRAMEWORK
TransparentNetworkIPResolution = false,
#endif
};
using SqlConnection connection = new(builder.ConnectionString);
// Act
Expand All @@ -324,7 +328,8 @@ public void NetworkError_WithUserProvidedPartner_RetryEnabled_ShouldConnectToFai
Assert.Equal(ConnectionState.Open, connection.State);
Assert.Equal($"localhost,{failoverServer.EndPoint.Port}", connection.DataSource);
Assert.Equal(1, server.PreLoginCount);
Assert.Equal(1, failoverServer.PreLoginCount);
Assert.Equal(0, server.Login7Count);
Assert.Equal(1, failoverServer.PreLoginCount - failoverServer.AbandonedPreLoginCount);
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

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

This test still asserts an exact server.PreLoginCount == 1, but the flakiness being addressed elsewhere in this PR comes from extra (abandoned) PreLogin attempts caused by interval-timer/TNIR behavior. To keep this deterministic, assert using AbandonedPreLoginCount/Login7Count (or relax the PreLogin assertion) instead of requiring an exact PreLoginCount.

Suggested change
Assert.Equal(1, failoverServer.PreLoginCount - failoverServer.AbandonedPreLoginCount);
Assert.Equal(1, failoverServer.Login7Count);

Copilot uses AI. Check for mistakes.
}

[Theory]
Expand Down Expand Up @@ -357,7 +362,8 @@ public void TransientFault_ShouldConnectToPrimary(uint errorCode)
InitialCatalog = "master",
ConnectTimeout = 30,
ConnectRetryInterval = 1,
Encrypt = false
Encrypt = false,
Pooling = false, // Disable pooling to ensure a fresh connection attempt is made
};
using SqlConnection connection = new(builder.ConnectionString);

Expand All @@ -369,7 +375,7 @@ public void TransientFault_ShouldConnectToPrimary(uint errorCode)
Assert.Equal($"localhost,{server.EndPoint.Port}", connection.DataSource);

// Failures should prompt the client to return to the original server, resulting in a login count of 2
Assert.Equal(2, server.PreLoginCount);
Assert.Equal(2, server.PreLoginCount - server.AbandonedPreLoginCount);
}

[Theory]
Expand Down Expand Up @@ -454,7 +460,7 @@ public void TransientFault_WithUserProvidedPartner_ShouldConnectToPrimary(uint e
FailoverPartner = $"localhost:{failoverServer.EndPoint.Port}", // User provided failover partner
};
using SqlConnection connection = new(builder.ConnectionString);

// Act
connection.Open();

Expand All @@ -463,7 +469,7 @@ public void TransientFault_WithUserProvidedPartner_ShouldConnectToPrimary(uint e
Assert.Equal($"localhost,{server.EndPoint.Port}", connection.DataSource);

// Failures should prompt the client to return to the original server, resulting in a login count of 2
Assert.Equal(2, server.PreLoginCount);
Assert.Equal(2, server.PreLoginCount - server.AbandonedPreLoginCount);
}

[Theory]
Expand Down Expand Up @@ -559,11 +565,14 @@ public void TransientFault_IgnoreServerProvidedFailoverPartner_ShouldConnectToUs
// Close the connection to return it to the pool
connection.Close();


// Act
// Dispose of the server to trigger a failover
server.Dispose();

// Clear the pool to ensure the next connection attempt doesn't reuse
// the pooled connection to the now-disposed primary server.
SqlConnection.ClearAllPools();
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

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

SqlConnection.ClearAllPools() clears pools globally and can introduce cross-test side effects when other tests run in parallel. Since this test only needs to ensure the pooled connection for this specific connection string isn't reused, prefer clearing just this pool (e.g., SqlConnection.ClearPool(connection) / clearing by key) rather than clearing all pools.

Suggested change
SqlConnection.ClearAllPools();
SqlConnection.ClearPool(connection);

Copilot uses AI. Check for mistakes.

// Opening a new connection will use the failover partner stored in the pool group.
// This will fail if the server provided failover partner was stored to the pool group.
using SqlConnection failoverConnection = new(builder.ConnectionString);
Expand All @@ -573,9 +582,9 @@ public void TransientFault_IgnoreServerProvidedFailoverPartner_ShouldConnectToUs
Assert.Equal(ConnectionState.Open, failoverConnection.State);
Assert.Equal($"localhost,{failoverServer.EndPoint.Port}", failoverConnection.DataSource);
// 1 for the initial connection
Assert.Equal(1, server.PreLoginCount);
Assert.Equal(1, server.PreLoginCount - server.AbandonedPreLoginCount);
// 1 for the failover connection
Assert.Equal(1, failoverServer.PreLoginCount);
Assert.Equal(1, failoverServer.PreLoginCount - failoverServer.AbandonedPreLoginCount);
}
}
}
5 changes: 2 additions & 3 deletions src/Microsoft.Data.SqlClient/tests/UnitTests/SimulatedServerTests/ConnectionRoutingTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@

namespace Microsoft.Data.SqlClient.UnitTests.SimulatedServerTests
{
[Trait("Category", "flaky")]
[Collection("SimulatedServerTests")]
public class ConnectionRoutingTests
{
Expand Down Expand Up @@ -58,8 +57,8 @@ public void TransientFaultAtRoutedLocation_ShouldReturnToGateway(uint errorCode)
Assert.Equal($"localhost,{router.EndPoint.Port}", connection.DataSource);

// Failures should prompt the client to return to the original server, resulting in a login count of 2
Assert.Equal(2, router.PreLoginCount);
Assert.Equal(2, server.PreLoginCount);
Assert.Equal(2, router.PreLoginCount - router.AbandonedPreLoginCount);
Assert.Equal(2, server.PreLoginCount - server.AbandonedPreLoginCount);
}

[Theory]
Expand Down
5 changes: 2 additions & 3 deletions ...rosoft.Data.SqlClient/tests/UnitTests/SimulatedServerTests/ConnectionRoutingTestsAzure.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@

namespace Microsoft.Data.SqlClient.UnitTests.SimulatedServerTests
{
[Trait("Category", "flaky")]
[Collection("SimulatedServerTests")]
public class ConnectionRoutingTestsAzure : IDisposable
{
Expand Down Expand Up @@ -76,8 +75,8 @@ public void TransientFaultAtRoutedLocation_ShouldReturnToGateway(uint errorCode)
Assert.Equal($"localhost,{router.EndPoint.Port}", connection.DataSource);

// Failures should prompt the client to return to the original server, resulting in a login count of 2
Assert.Equal(2, router.PreLoginCount);
Assert.Equal(2, server.PreLoginCount);
Assert.Equal(2, router.PreLoginCount - router.AbandonedPreLoginCount);
Assert.Equal(2, server.PreLoginCount - server.AbandonedPreLoginCount);
}

[Theory]
Expand Down
47 changes: 24 additions & 23 deletions src/Microsoft.Data.SqlClient/tests/UnitTests/SimulatedServerTests/ConnectionTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ public void ConnectionTest()
{
using TdsServer server = new(new TdsServerArguments() { });
server.Start();
var connStr = new SqlConnectionStringBuilder() {
var connStr = new SqlConnectionStringBuilder()
{
DataSource = $"localhost,{server.EndPoint.Port}",
Encrypt = SqlConnectionEncryptOption.Optional,
}.ConnectionString;
Expand All @@ -43,7 +44,8 @@ public void IntegratedAuthConnectionTest()
{
using TdsServer server = new(new TdsServerArguments() { });
server.Start();
var connStr = new SqlConnectionStringBuilder() {
var connStr = new SqlConnectionStringBuilder()
{
DataSource = $"localhost,{server.EndPoint.Port}",
Encrypt = SqlConnectionEncryptOption.Optional,
}.ConnectionString;
Expand All @@ -61,9 +63,10 @@ public void IntegratedAuthConnectionTest()
[Fact]
public async Task RequestEncryption_ServerDoesNotSupportEncryption_ShouldFail()
{
using TdsServer server = new(new TdsServerArguments() {Encryption = TDSPreLoginTokenEncryptionType.None });
using TdsServer server = new(new TdsServerArguments() { Encryption = TDSPreLoginTokenEncryptionType.None });
server.Start();
var connStr = new SqlConnectionStringBuilder() {
var connStr = new SqlConnectionStringBuilder()
{
DataSource = $"localhost,{server.EndPoint.Port}"
}.ConnectionString;

Expand All @@ -72,34 +75,35 @@ public async Task RequestEncryption_ServerDoesNotSupportEncryption_ShouldFail()
Assert.Contains("The instance of SQL Server you attempted to connect to does not support encryption.", ex.Message, StringComparison.OrdinalIgnoreCase);
}

[Trait("Category", "flaky")]
[Theory]
[InlineData(40613)]
[InlineData(42108)]
[InlineData(42109)]
public async Task TransientFault_RetryEnabled_ShouldSucceed_Async(uint errorCode)
{
using TransientTdsErrorTdsServer server = new(
new TransientTdsErrorTdsServerArguments()
new TransientTdsErrorTdsServerArguments()
{
IsEnabledTransientError = true,
Number = errorCode,
IsEnabledTransientError = true,
Number = errorCode,
});
server.Start();
SqlConnectionStringBuilder builder = new()
{
DataSource = "localhost," + server.EndPoint.Port,
Encrypt = SqlConnectionEncryptOption.Optional
Encrypt = SqlConnectionEncryptOption.Optional,
#if NETFRAMEWORK
TransparentNetworkIPResolution = false
#endif
};

using SqlConnection connection = new(builder.ConnectionString);
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);
}

[Trait("Category", "flaky")]
[Theory]
[InlineData(40613)]
[InlineData(42108)]
Expand All @@ -123,10 +127,9 @@ public void TransientFault_RetryEnabled_ShouldSucceed(uint errorCode)
connection.Open();
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);
}

[Trait("Category", "flaky")]
[Theory]
[InlineData(40613)]
[InlineData(42108)]
Expand All @@ -151,10 +154,9 @@ public async Task TransientFault_RetryDisabled_ShouldFail_Async(uint errorCode)
SqlException e = await Assert.ThrowsAsync<SqlException>(async () => await connection.OpenAsync());
Assert.Equal((int)errorCode, e.Number);
Assert.Equal(ConnectionState.Closed, connection.State);
Assert.Equal(1, server.PreLoginCount);
Assert.Equal(1, server.PreLoginCount - server.AbandonedPreLoginCount);
}

[Trait("Category", "flaky")]
[Theory]
[InlineData(40613)]
[InlineData(42108)]
Expand All @@ -179,10 +181,9 @@ public void TransientFault_RetryDisabled_ShouldFail(uint errorCode)
SqlException e = Assert.Throws<SqlException>(() => connection.Open());
Assert.Equal((int)errorCode, e.Number);
Assert.Equal(ConnectionState.Closed, connection.State);
Assert.Equal(1, server.PreLoginCount);
Assert.Equal(1, server.PreLoginCount - server.AbandonedPreLoginCount);
}

[Trait("Category", "flaky")]
[Theory]
[InlineData(false)]
[InlineData(true)]
Expand All @@ -200,6 +201,7 @@ public async Task NetworkError_RetryEnabled_ShouldSucceed_Async(bool multiSubnet
DataSource = "localhost," + server.EndPoint.Port,
Encrypt = SqlConnectionEncryptOption.Optional,
ConnectTimeout = 5,
Pooling = false, // Disable pooling to ensure a fresh connection attempt is made
MultiSubnetFailover = multiSubnetFailoverEnabled,
#if NETFRAMEWORK
TransparentNetworkIPResolution = multiSubnetFailoverEnabled
Expand All @@ -216,11 +218,10 @@ public async Task NetworkError_RetryEnabled_ShouldSucceed_Async(bool multiSubnet
}
else
{
Assert.Equal(1, server.PreLoginCount);
Assert.Equal(1, server.PreLoginCount - server.AbandonedPreLoginCount);
}
}

[Trait("Category", "flaky")]
[Theory]
[InlineData(true)]
[InlineData(false)]
Expand Down Expand Up @@ -261,11 +262,10 @@ public async Task NetworkDelay_RetryDisabled_Async(bool multiSubnetFailoverEnabl
}
else
{
Assert.Equal(1, server.PreLoginCount);
Assert.Equal(1, server.PreLoginCount - server.AbandonedPreLoginCount);
}
}

[Trait("Category", "flaky")]
[Theory]
[InlineData(true)]
[InlineData(false)]
Expand Down Expand Up @@ -306,7 +306,7 @@ public void NetworkDelay_RetryDisabled(bool multiSubnetFailoverEnabled)
}
else
{
Assert.Equal(1, server.PreLoginCount);
Assert.Equal(1, server.PreLoginCount - server.AbandonedPreLoginCount);
}
}

Expand Down Expand Up @@ -467,7 +467,8 @@ public void ConnectionTimeoutTest(int timeout)
//TODO: do we even need a server for this test?
using TdsServer server = new();
server.Start();
var connStr = new SqlConnectionStringBuilder() {
var connStr = new SqlConnectionStringBuilder()
{
DataSource = $"localhost,{server.EndPoint.Port}",
ConnectTimeout = timeout,
Encrypt = SqlConnectionEncryptOption.Optional
Expand Down
Loading
Loading