Merge pull request 'Stop an attach's own replay landing in the middle of a test's frames' (#18) from claude/fix-failed-test-3e1719 into main
ci / build and test (push) Successful in 3m23s
ci / android head (push) Successful in 3m48s
ci / api image (push) Failing after 15s
ci / desktop nightly (push) Successful in 1m29s

Reviewed-on: #18
This commit was merged in pull request #18.
This commit is contained in:
jaap-jan committed 2026-09-15 12:46:46 +00:00
commit 54bc142dce
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;
}