Skip to content

Commit fb50e48

Browse files
committed
tests(Spanner): Fix flakes by using NSubstitute better.
There's still a chance of a flake here but there's not much we can do about it except adjust the time we wait for a background task to complete.
1 parent 583d3b9 commit fb50e48

File tree

1 file changed

+38
-40
lines changed

1 file changed

+38
-40
lines changed

apis/Google.Cloud.Spanner.Data/Google.Cloud.Spanner.Data.Tests/V1/ManagedSessionTests.cs

Lines changed: 38 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -87,34 +87,31 @@ public async Task GetFreshSession_SoftExpiry_BackgroundRefresh()
8787
var session1 = new Session { Name = "session1", CreateTime = clock.GetCurrentDateTimeUtc().ToTimestamp() };
8888
var session2 = new Session { Name = "session2" }; // CreateTime will be set when it's created
8989

90-
var refreshStartedTsc = new TaskCompletionSource<bool>();
90+
var createSessionCalledTsc = new TaskCompletionSource<bool>();
9191
var freshSessionTsc = new TaskCompletionSource<Session>();
9292
client.Configure()
9393
.CreateSessionAsync(Arg.Any<CreateSessionRequest>(), Arg.Any<CallSettings>())
94-
.Returns(Task.FromResult(session1), CreateSessionMock(refreshStartedTsc, freshSessionTsc.Task));
94+
.Returns(Task.FromResult(session1), freshSessionTsc.Task);
95+
client.Configure()
96+
.When(client => client.CreateSessionAsync(Arg.Any<CreateSessionRequest>(), Arg.Any<CallSettings>()))
97+
.Do(Callback.First(_ => { }).Then(_ => createSessionCalledTsc.SetResult(true)));
9598

99+
// This calls CreateSessionAsync for the first time and does not return until the session is acquired.
96100
await managedSession.EnsureFreshAsync(CancellationToken.None);
97101

98102
// Advance past soft expiry
99103
clock.Advance(SoftExpiry + TimeSpan.FromMinutes(1));
100104
session2.CreateTime = clock.GetCurrentDateTimeUtc().ToTimestamp();
101105

102106
// This should trigger background refresh but return immediately because session1 is only soft expired.
107+
// The triggered background refresh will eventually call CreateSessionAsync for a second time.
103108
await managedSession.EnsureFreshAsync(CancellationToken.None);
104109

105-
// Wait for the background task to be triggered
106-
await refreshStartedTsc.Task;
107-
108-
await client.Received(2).CreateSessionAsync(Arg.Any<CreateSessionRequest>(), Arg.Any<CallSettings>());
109-
110-
// Make the mock return so that the background refresh can complete.
110+
// Allow CreateSessionAsync to return.
111111
freshSessionTsc.SetResult(session2);
112112

113-
// Wait a little in real time to allow the background refresh to complete.
114-
await Task.Delay(100);
115-
116-
// Next call should return session2 (indirectly verified by no more calls to CreateSessionAsync)
117-
await managedSession.EnsureFreshAsync(CancellationToken.None);
113+
// And now we can wait for CreateSessionAsync to be done.
114+
await createSessionCalledTsc.Task;
118115
await client.Received(2).CreateSessionAsync(Arg.Any<CreateSessionRequest>(), Arg.Any<CallSettings>());
119116
}
120117

@@ -167,43 +164,51 @@ public async Task GetFreshSession_SoftExpiry_RefreshFailure_Swallowed()
167164
var session1 = new Session { Name = "session1", CreateTime = clock.GetCurrentDateTimeUtc().ToTimestamp() };
168165
var session2 = new Session { Name = "session2" };
169166

170-
var firstRefreshStartedTcs = new TaskCompletionSource<bool>();
171-
var firstFreshSessionTcs = new TaskCompletionSource<Session>();
172-
var secondRefreshStartedTcs = new TaskCompletionSource<bool>();
167+
var throwingCreateSessionCalledTcs = new TaskCompletionSource<bool>();
168+
var throwingFreshSessionTcs = new TaskCompletionSource<Session>();
169+
var createSessionCalledTcs = new TaskCompletionSource<bool>();
170+
var freshSessionTcs = new TaskCompletionSource<Session>();
173171
client.Configure()
174172
.CreateSessionAsync(Arg.Any<CreateSessionRequest>(), Arg.Any<CallSettings>())
175-
.Returns(
176-
Task.FromResult(session1),
177-
CreateSessionMock(firstRefreshStartedTcs, firstFreshSessionTcs.Task),
178-
CreateSessionMock(secondRefreshStartedTcs, Task.FromResult(session2)));
173+
.Returns(Task.FromResult(session1), throwingFreshSessionTcs.Task, freshSessionTcs.Task);
174+
client.Configure()
175+
.When(client => client.CreateSessionAsync(Arg.Any<CreateSessionRequest>(), Arg.Any<CallSettings>()))
176+
.Do(Callback.First(_ => { }).Then(_ => throwingCreateSessionCalledTcs.SetResult(true)).Then(_ => createSessionCalledTcs.SetResult(true)));
179177

178+
// This calls CreateSessionAsync for the first time and does not return until the session is acquired.
180179
await managedSession.EnsureFreshAsync(CancellationToken.None);
181180

182181
// Advance past soft expiry
183182
clock.Advance(SoftExpiry + TimeSpan.FromMinutes(1));
184183
session2.CreateTime = clock.GetCurrentDateTimeUtc().ToTimestamp();
185184

186-
// Should return session1 and not throw
185+
// This should trigger background refresh but return immediately because session1 is only soft expired.
186+
// The triggered background refresh will eventually call CreateSessionAsync for a second time, which fails.
187187
await managedSession.EnsureFreshAsync(CancellationToken.None);
188-
189-
// Wait for the background task to be triggered
190-
await firstRefreshStartedTcs.Task;
191188

192-
await client.Received(2).CreateSessionAsync(Arg.Any<CreateSessionRequest>(), Arg.Any<CallSettings>());
189+
// Allow CreateSessionAsync to fail.
190+
throwingFreshSessionTcs.SetException(new Exception("Soft failure"));
193191

194-
// Complete with failure
195-
firstFreshSessionTcs.SetException(new Exception("Soft failure"));
192+
// And now we can wait for CreateSessionAsync to be called.
193+
// It has failed but that failure is not bubbled up because the session was only soft expired.
194+
await throwingCreateSessionCalledTcs.Task;
195+
await client.Received(2).CreateSessionAsync(Arg.Any<CreateSessionRequest>(), Arg.Any<CallSettings>());
196196

197197
// Wait a little in real time to allow the failed background refresh to complete.
198-
await Task.Delay(100);
198+
// There's a little bit of work that's done in the backgroun refresh after CreateSessinAsync has returned,
199+
// but we don't have a way to wait on that.
200+
// Note: Adust this number up if this is flaky.
201+
await Task.Delay(500);
199202

200-
// Still should not throw.
201-
// Note that since the previous background refresh failed, this call will trigger a new background refresh
202-
// because the session is still soft-expired.
203+
// This should still return inmediately and without throwing because session1 is only soft expired.
204+
// Because the previous background refresh failed, this call will trigger a new background refresh.
203205
await managedSession.EnsureFreshAsync(CancellationToken.None);
204206

205-
// Wait for the background task to be triggered
206-
await secondRefreshStartedTcs.Task;
207+
// Allow CreateSessionAsync to return
208+
freshSessionTcs.SetResult(session2);
209+
210+
// And now we can wait for CreateSessionAsync to be called for the third time.
211+
await createSessionCalledTcs.Task;
207212
await client.Received(3).CreateSessionAsync(Arg.Any<CreateSessionRequest>(), Arg.Any<CallSettings>());
208213
}
209214

@@ -335,11 +340,4 @@ public async Task BatchWriteAsync_EnsureFreshnessAndSetsSession()
335340
// Verify BatchWrite was called with the correct session
336341
client.Received(1).BatchWrite(Arg.Is<BatchWriteRequest>(r => r.Session == session1.Name), Arg.Any<CallSettings>());
337342
}
338-
339-
private static Task<Session> CreateSessionMock(TaskCompletionSource<bool> methodStartedTsc, Task<Session> result) =>
340-
Task.Run(() =>
341-
{
342-
methodStartedTsc.SetResult(true);
343-
return result;
344-
});
345343
}

0 commit comments

Comments
 (0)