Skip to content

Commit 547632a

Browse files
committed
Fix flaky test with client initial connection retry.
Signed-off-by: currantw <taylor.curran@improving.com>
1 parent d3c7a5a commit 547632a

6 files changed

Lines changed: 73 additions & 14 deletions

File tree

tests/Valkey.Glide.IntegrationTests/GenericCommandTests.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -254,13 +254,13 @@ public async Task TestKeyIdleTime(BaseClient client)
254254

255255
// Verify that idle time increases.
256256
TimeSpan idleTime2 = TimeSpan.Zero;
257-
await Polling.AssertTrueAsync(
257+
await Polling.WaitForTrueAsync(
258258
async () =>
259259
{
260260
idleTime2 = Assert.NotNull(await client.ObjectIdleTimeAsync(key));
261261
return idleTime2.TotalSeconds > idleTime1.TotalSeconds;
262262
},
263-
$"Idle time expected to increase.");
263+
"Idle time expected to increase.");
264264

265265
// Verify that idle time is reset on access.
266266
_ = await client.GetAsync(key);

tests/Valkey.Glide.IntegrationTests/PubSubQueueTests.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ public static async Task Queue_Channel_ReceivesMessage(bool isCluster, PubSubCha
2727
// Verify that message is received.
2828
PubSubMessageQueue queue = subscriber.PubSubQueue!;
2929

30-
await Polling.AssertTrue(
30+
await Polling.WaitForTrue(
3131
() => queue.Count > 0,
3232
"Expected message was not received.",
3333
timeout: MaxDuration,
@@ -55,7 +55,7 @@ public static async Task Queue_WithHighVolume_HandlesAllMessages(bool isCluster)
5555
// Verify that all messages are received.
5656
PubSubMessageQueue queue = subscriber.PubSubQueue!;
5757

58-
await Polling.AssertTrue(
58+
await Polling.WaitForTrue(
5959
() => queue!.Count >= messageCount,
6060
$"Expected {messageCount} messages but only received {queue.Count}.",
6161
timeout: MaxDuration,

tests/Valkey.Glide.IntegrationTests/PubSubUtils.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -695,7 +695,7 @@ public static async Task AssertSubscribedAsync(BaseClient client, IEnumerable<Pu
695695
}
696696

697697
// For lazy mode, retry until subscribed or timeout occurs.
698-
await Polling.AssertTrueAsync(
698+
await Polling.WaitForTrueAsync(
699699
() => IsSubscribedAsync(client, expectedSubscriptions),
700700
"Expected subscriptions not found.",
701701
timeout: MaxDuration,
@@ -727,7 +727,7 @@ public static async Task AssertNotSubscribedAsync(BaseClient client, IEnumerable
727727
}
728728

729729
// For lazy mode, retry until unsubscribed or timeout occurs.
730-
await Polling.AssertTrueAsync(
730+
await Polling.WaitForTrueAsync(
731731
() => IsNotSubscribedAsync(client, notExpected),
732732
"Unexpected subscriptions found.",
733733
timeout: MaxDuration,

tests/Valkey.Glide.IntegrationTests/StackExchange/PubSubCommandTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -648,7 +648,7 @@ private static async Task AssertHandlerReceives(ConcurrentBag<MessageInfo> actua
648648
return;
649649
}
650650

651-
await Polling.AssertTrue(
651+
await Polling.WaitForTrue(
652652
() => actual.Count >= expected.Count,
653653
"Expected messages not received.",
654654
timeout: AssertTimeout,

tests/Valkey.Glide.TestUtils/Polling.cs

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,12 +32,12 @@ public static class Polling
3232
/// <param name="message">The message to report if the condition is not met within the timeout.</param>
3333
/// <param name="timeout">Maximum time to poll. Defaults to <see cref="DefaultTimeout"/>.</param>
3434
/// <param name="interval">Delay between attempts. Defaults to <see cref="DefaultInterval"/>.</param>
35-
public static Task AssertTrue(
35+
public static Task WaitForTrue(
3636
Func<bool> condition,
3737
string message,
3838
TimeSpan? timeout = null,
3939
TimeSpan? interval = null)
40-
=> AssertTrueAsync(()
40+
=> WaitForTrueAsync(()
4141
=> Task.FromResult(condition()), message, timeout, interval);
4242

4343
/// <summary>
@@ -48,22 +48,24 @@ public static Task AssertTrue(
4848
/// <param name="message">The message to report if the condition is not met within the timeout.</param>
4949
/// <param name="timeout">Maximum time to poll. Defaults to <see cref="DefaultTimeout"/>.</param>
5050
/// <param name="interval">Delay between attempts. Defaults to <see cref="DefaultInterval"/>.</param>
51-
public static async Task AssertTrueAsync(
51+
public static async Task WaitForTrueAsync(
5252
Func<Task<bool>> condition,
5353
string message,
5454
TimeSpan? timeout = null,
5555
TimeSpan? interval = null)
5656
{
57-
using CancellationTokenSource cts = new(timeout ?? DefaultTimeout);
57+
timeout ??= DefaultTimeout;
58+
interval ??= DefaultInterval;
5859

60+
using CancellationTokenSource cts = new(timeout.Value);
5961
while (!cts.Token.IsCancellationRequested)
6062
{
6163
if (await condition())
6264
{
6365
return;
6466
}
6567

66-
await Task.Delay(interval ?? DefaultInterval);
68+
await Task.Delay(interval.Value);
6769
}
6870

6971
Assert.Fail(message);

tests/Valkey.Glide.TestUtils/Server.cs

Lines changed: 59 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// Copyright Valkey GLIDE Project Contributors - SPDX Identifier: Apache-2.0
22

33
using static Valkey.Glide.ConnectionConfiguration;
4+
using static Valkey.Glide.Errors;
45

56
namespace Valkey.Glide.TestUtils;
67

@@ -16,6 +17,16 @@ public abstract class Server : IDisposable
1617
/// </summary>
1718
protected static readonly GlideString[] KillClientArgs = ["CLIENT", "KILL", "TYPE", "NORMAL"];
1819

20+
/// <summary>
21+
/// Number of attempts to establish the initial client connection.
22+
/// </summary>
23+
private const int ConnectionAttempts = 5;
24+
25+
/// <summary>
26+
/// Delay between initial connection attempts.
27+
/// </summary>
28+
private static readonly TimeSpan ConnectionRetryDelay = TimeSpan.FromMilliseconds(500);
29+
1930
#endregion
2031
#region Fields
2132

@@ -122,6 +133,44 @@ public void Dispose()
122133
/// </summary>
123134
public abstract Task KillClientsAsync();
124135

136+
#endregion
137+
#region Protected Methods
138+
139+
/// <summary>
140+
/// Creates a client using the given factory.
141+
/// </summary>
142+
/// <typeparam name="T">The client type produced by the factory.</typeparam>
143+
/// <param name="factory">Factory that builds and connects a client.</param>
144+
/// <returns>The connected client.</returns>
145+
/// <exception cref="ConnectionException">
146+
/// Thrown when a connection could not be established after all attempts.
147+
/// </exception>
148+
protected static async Task<T> CreateClientAsync<T>(Func<Task<T>> factory)
149+
where T : BaseClient
150+
{
151+
ConnectionException? lastException = null;
152+
153+
/// Retry the initial connection attempt if needed to handle flakiness in CI environment.
154+
for (int attempt = 1; attempt <= ConnectionAttempts; attempt++)
155+
{
156+
try
157+
{
158+
return await factory();
159+
}
160+
catch (ConnectionException ex)
161+
{
162+
lastException = ex;
163+
164+
if (attempt < ConnectionAttempts)
165+
{
166+
await Task.Delay(ConnectionRetryDelay);
167+
}
168+
}
169+
}
170+
171+
throw lastException!;
172+
}
173+
125174
#endregion
126175
}
127176

@@ -150,7 +199,11 @@ public override async Task<BaseClient> CreateClientAsync()
150199
/// Builds and returns a cluster client for this server.
151200
/// </summary>
152201
public async Task<GlideClusterClient> CreateClusterClientAsync()
153-
=> await GlideClusterClient.CreateClient(CreateConfigBuilder().Build());
202+
{
203+
var config = CreateConfigBuilder().Build();
204+
Task<GlideClusterClient> factory() => GlideClusterClient.CreateClient(config);
205+
return await CreateClientAsync(factory);
206+
}
154207

155208
public override async Task SetPasswordAsync(string password)
156209
{
@@ -200,7 +253,11 @@ public override async Task<BaseClient> CreateClientAsync()
200253
/// Builds and returns a standalone client for this server.
201254
/// </summary>
202255
public async Task<GlideClient> CreateStandaloneClientAsync()
203-
=> await GlideClient.CreateClient(CreateConfigBuilder().Build());
256+
{
257+
var config = CreateConfigBuilder().Build();
258+
Task<GlideClient> factory() => GlideClient.CreateClient(config);
259+
return await CreateClientAsync(factory);
260+
}
204261

205262
public override async Task SetPasswordAsync(string password)
206263
{

0 commit comments

Comments
 (0)