diff --git a/tests/DodoSSH.Client.Terminal.Tests/TerminalWorkspaceTests.cs b/tests/DodoSSH.Client.Terminal.Tests/TerminalWorkspaceTests.cs index 82ece61..5f2eaa4 100644 --- a/tests/DodoSSH.Client.Terminal.Tests/TerminalWorkspaceTests.cs +++ b/tests/DodoSSH.Client.Terminal.Tests/TerminalWorkspaceTests.cs @@ -27,6 +27,12 @@ public sealed class TerminalWorkspaceTests /// How long polls before calling it a failure. private static readonly TimeSpan PollTimeout = TimeSpan.FromSeconds(10); + /// + /// How long waits for the attach's replay before calling it a + /// failure. Generous, because it bounds a hang rather than timing anything. + /// + private static readonly TimeSpan ReplayTimeout = TimeSpan.FromSeconds(10); + [Fact] public async Task WaitingForARendererThatNeverAttaches_GivesUp() { @@ -773,11 +779,20 @@ public sealed class TerminalWorkspaceTests /// /// /// WaitForRendererAsync 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 — 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 - /// SocketAttached, 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 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 SessionOpened — harmless to the page, which treats it as idempotent, but it + /// lands in the middle of the frames the test is counting on. + /// + /// + /// ◆ SO WAIT FOR THE REPLAY TO BE OVER, NOT ONLY FOR THE ATTACH. RendererReattached 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. /// /// private static async Task 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; }