Public Access
Stop an attach's own replay landing in the middle of a test's frames
TheClipboard_ReachesThePaneThatAskedForIt failed in CI on an opcode of 2 where it wanted 8: a second SessionOpened frame where the clipboard's should have been. Only one thing sends a session a second SessionOpened, and that is ReplayAfterAttachAsync, the replay a reattaching page gets. It arrives there because the replay is fired and forgotten from the socket-accept thread immediately after the attach is signalled, and the test helper returned on that signal. Deschedule the accept thread for a timeslice between the two -- which a loaded CI box does -- and the test opens its session first, so the replay finds that session live and sends it a replay-flagged SessionOpened after its real one. Nothing is wrong with the workspace. The page treats a replay onto a pane it still has as idempotent, and ReplayAfterAttachAsync's own remarks already say racing a fresh page's first frames is harmless. What was wrong is a helper that waited for the attach and then let its callers count frames as though the attach were finished, so ConnectRendererAsync now waits for RendererReattached as well -- raised as the last thing the replay does. The remark there argued a replay could not be sent early; it missed that it can be sent late, and now says so. The same late replay could reset the credit window out from under ANewRenderer_ReplaysTheLiveSessionAndResetsItsCredits, which shares the helper, so that latent flake goes with it. Verified by delaying the replay's snapshot by a millisecond: the old test fails under that, and the fixed class passes it eight runs running.
This commit is contained in:
1 parent
8a4058ce3a
commit
8e61fc72b4
1 file changed
+32
-5
@@ -27,6 +27,12 @@ public sealed class TerminalWorkspaceTests
|
||||
/// <summary>How long <see cref="WaitUntilAsync"/> polls before calling it a failure.</summary>
|
||||
private static readonly TimeSpan PollTimeout = TimeSpan.FromSeconds(10);
|
||||
|
||||
/// <summary>
|
||||
/// How long <see cref="ConnectRendererAsync"/> waits for the attach's replay before calling it a
|
||||
/// failure. Generous, because it bounds a hang rather than timing anything.
|
||||
/// </summary>
|
||||
private static readonly TimeSpan ReplayTimeout = TimeSpan.FromSeconds(10);
|
||||
|
||||
[Fact]
|
||||
public async Task WaitingForARendererThatNeverAttaches_GivesUp()
|
||||
{
|
||||
@@ -773,11 +779,20 @@ public sealed class TerminalWorkspaceTests
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// <c>WaitForRendererAsync</c> answers only for the first renderer ever to attach, so a second call
|
||||
/// returns immediately without proving anything about the second socket. That is enough here and is
|
||||
/// not luck: the only frames a second socket is given are the replay, and the replay is *caused* by the
|
||||
/// attach — <see cref="TerminalWorkspace.RendererReattached"/> and the frames before it cannot be sent
|
||||
/// early. The day a test sends something else down a reattached socket, this needs the data plane's own
|
||||
/// <c>SocketAttached</c>, which the workspace does not forward today.
|
||||
/// returns immediately without proving anything about the second socket — and even for the first it
|
||||
/// answers too early for what the callers here need. It completes when the socket is attached, which
|
||||
/// is before <see cref="TerminalWorkspace.ReplayAfterAttachAsync"/> has run: that is fired and
|
||||
/// forgotten from the accept thread, and a thread-pool that deschedules it for a timeslice lets the
|
||||
/// test open a session in the meantime. The replay then finds that session live and sends it a second,
|
||||
/// replay-flagged <c>SessionOpened</c> — harmless to the page, which treats it as idempotent, but it
|
||||
/// lands in the middle of the frames the test is counting on.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// ◆ SO WAIT FOR THE REPLAY TO BE OVER, NOT ONLY FOR THE ATTACH. <c>RendererReattached</c> is raised
|
||||
/// as the last thing the replay does, so a socket this returns has had every frame the attach itself
|
||||
/// causes, and nothing but what the test does next can put another on it. Subscribed before the
|
||||
/// upgrade, because the replay for a socket that attaches quickly can be over before this method gets
|
||||
/// to look.
|
||||
/// </para>
|
||||
/// </remarks>
|
||||
private static async Task<ClientWebSocket> ConnectRendererAsync(TerminalWorkspace workspace)
|
||||
@@ -795,17 +810,29 @@ public sealed class TerminalWorkspaceTests
|
||||
"Origin",
|
||||
string.Create(CultureInfo.InvariantCulture, $"http://127.0.0.1:{workspace.PageUrl.Port}"));
|
||||
|
||||
var replayed = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
|
||||
|
||||
void OnReattached(object? sender, EventArgs e) => replayed.TrySetResult();
|
||||
|
||||
workspace.RendererReattached += OnReattached;
|
||||
|
||||
try
|
||||
{
|
||||
await client.ConnectAsync(new Uri(socketUrl), TestContext.Current.CancellationToken);
|
||||
|
||||
await workspace.WaitForRendererAsync(TestContext.Current.CancellationToken);
|
||||
|
||||
await replayed.Task.WaitAsync(ReplayTimeout, TestContext.Current.CancellationToken);
|
||||
}
|
||||
catch
|
||||
{
|
||||
client.Dispose();
|
||||
throw;
|
||||
}
|
||||
finally
|
||||
{
|
||||
workspace.RendererReattached -= OnReattached;
|
||||
}
|
||||
|
||||
return client;
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user