Public Access
Merge branch 'claude/gallant-brahmagupta-1f8244'
Writes down that locking the vault leaves shells running, and shows the count on the unlock screen rather than leaving it to be inferred. Conflict resolution: - ShellFlowTests' fixture keeps main's FakeSshConnectionFactory. The branch added an IdleSshConnectionFactory for exactly what main's fake already does — a shell that is open, silent and never closes on its own — so FakeSshConnections.cs is dropped rather than merged, leaving one fake SSH stack in the suite instead of two that would drift apart. - MainWindowViewModel and TerminalWorkspace: both sides added their own members, so both are kept. - TerminalWorkspaceTests was added by both branches, with the renderer gate on one side and session lifetime on the other. Merged into one class over one set of helpers; the gate tests now use FakeConnectionFactory rather than an NSubstitute stub, since the suite already has the fake. gallant's polling Timeout constant is PollTimeout, which no longer reads as the renderer's. - platform-flags.md keeps main's measured focus section and drops the short "nothing hands the terminal keyboard focus" entry the branch still carried, which that section supersedes. One genuine disagreement between the branches, left visible rather than flattened: this branch measured that a collapsed WebView cannot be typed into and attributed it to a hidden WS_CHILD window being ineligible for keyboard focus, while main's focus work measured Win32 focus still held by that hidden window and added a lock path that moves the keyboard off it. Both results stand; the mechanism sentence now defers to the focus entry, which makes the input barrier something the lock path maintains rather than something the platform guarantees. Full suite green, including the container-backed SSH tests.
This commit is contained in:
@@ -101,6 +101,65 @@ internal sealed class FakeShellSession : ISshShellSession
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Hands out <see cref="FakeShellSession"/>s, so a workspace can be driven with no network.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// Exists for the session-lifetime tests. Everything else in this suite works on a pump directly; the
|
||||
/// workspace is the layer that decides when a session is over, and that decision is what needs a
|
||||
/// connection whose shell can be made to end on cue.
|
||||
/// </remarks>
|
||||
internal sealed class FakeConnectionFactory(long bytesPerShell = long.MaxValue) : ISshConnectionFactory
|
||||
{
|
||||
/// <summary>Connections handed out, in order.</summary>
|
||||
internal List<FakeConnection> Connections { get; } = [];
|
||||
|
||||
/// <inheritdoc />
|
||||
public Task<ISshConnection> ConnectAsync(
|
||||
SshConnectionRequest request,
|
||||
CancellationToken cancellationToken)
|
||||
{
|
||||
var connection = new FakeConnection(request, bytesPerShell);
|
||||
Connections.Add(connection);
|
||||
|
||||
return Task.FromResult<ISshConnection>(connection);
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>A connection that opens fake shells and records its own disposal.</summary>
|
||||
internal sealed class FakeConnection(SshConnectionRequest request, long bytesPerShell) : ISshConnection
|
||||
{
|
||||
/// <inheritdoc />
|
||||
public bool IsConnected { get; private set; } = true;
|
||||
|
||||
/// <inheritdoc />
|
||||
public HostKeyPresentation HostKey { get; } =
|
||||
new(request.Host, request.Port, "ssh-ed25519", "SHA256:fake");
|
||||
|
||||
/// <summary>The shell this connection opened, if it opened one.</summary>
|
||||
internal FakeShellSession? Shell { get; private set; }
|
||||
|
||||
/// <summary>Whether the connection was disposed, which is what closing a session must do.</summary>
|
||||
internal bool IsDisposed { get; private set; }
|
||||
|
||||
/// <inheritdoc />
|
||||
public Task<ISshShellSession> OpenShellAsync(TerminalSize size, CancellationToken cancellationToken)
|
||||
{
|
||||
Shell = new FakeShellSession(bytesPerShell);
|
||||
|
||||
return Task.FromResult<ISshShellSession>(Shell);
|
||||
}
|
||||
|
||||
/// <inheritdoc />
|
||||
public ValueTask DisposeAsync()
|
||||
{
|
||||
IsDisposed = true;
|
||||
IsConnected = false;
|
||||
|
||||
return ValueTask.CompletedTask;
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>Records frames, and can acknowledge them to keep credit flowing.</summary>
|
||||
internal sealed class RecordingTransport : ITerminalTransport
|
||||
{
|
||||
|
||||
@@ -1,21 +1,29 @@
|
||||
using System.Text;
|
||||
using DodoSSH.Client.Ssh;
|
||||
using NSubstitute;
|
||||
|
||||
namespace DodoSSH.Client.Terminal.Tests;
|
||||
|
||||
/// <summary>
|
||||
/// The renderer gate: the one place the workspace waits on something outside the process.
|
||||
/// What the workspace waits for outside the process, and how long a session lives once it has one.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// The gate itself is not in question. <see cref="TerminalDataPlane.SendAsync"/> drops frames when nothing
|
||||
/// is attached, so a session opened before the renderer arrives loses its <c>SessionOpened</c> frame and
|
||||
/// streams output at a terminal that was never created — and that it opens when a renderer does attach is
|
||||
/// covered by <see cref="TerminalDataPlaneTests.TheRenderer_Attaches"/>. What is worth a test here is the
|
||||
/// <para>
|
||||
/// The renderer gate itself is not in question. <see cref="TerminalDataPlane.SendAsync"/> drops frames when
|
||||
/// nothing is attached, so a session opened before the renderer arrives loses its <c>SessionOpened</c> frame
|
||||
/// and streams output at a terminal that was never created — and that it opens when a renderer does attach
|
||||
/// is covered by <see cref="TerminalDataPlaneTests.TheRenderer_Attaches"/>. What is worth a test here is the
|
||||
/// half that used to be missing: waiting for a renderer that never arrives has to end.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// The session count is here for a different reason. It is shown to a user on the unlock screen, as the
|
||||
/// disclosure that locking the vault leaves shells running, and a wrong number there is not a cosmetic bug:
|
||||
/// it either hides a live connection or invents one.
|
||||
/// </para>
|
||||
/// </remarks>
|
||||
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);
|
||||
|
||||
[Fact]
|
||||
public async Task WaitingForARendererThatNeverAttaches_GivesUp()
|
||||
{
|
||||
@@ -23,7 +31,9 @@ public sealed class TerminalWorkspaceTests
|
||||
// runtime, an install blocked by policy, an AppContainer that cannot reach loopback. From this
|
||||
// side they are identical and all look like the listener being up with nothing ever connecting
|
||||
// to it. Before the wait was bounded this test would have hung instead of failing.
|
||||
await using var workspace = CreateWorkspace(TimeSpan.FromMilliseconds(250));
|
||||
await using var workspace = CreateWorkspace(
|
||||
new FakeConnectionFactory(), rendererTimeout: TimeSpan.FromMilliseconds(250));
|
||||
|
||||
workspace.Start();
|
||||
|
||||
await Should.ThrowAsync<TimeoutException>(async () =>
|
||||
@@ -36,7 +46,9 @@ public sealed class TerminalWorkspaceTests
|
||||
// The timeout is the backstop; the caller's token is what makes a Connect the user gave up on
|
||||
// return at once rather than sitting out the rest of the wait. The timeout here is long enough
|
||||
// that only cancellation can end this.
|
||||
await using var workspace = CreateWorkspace(TimeSpan.FromMinutes(5));
|
||||
await using var workspace = CreateWorkspace(
|
||||
new FakeConnectionFactory(), rendererTimeout: TimeSpan.FromMinutes(5));
|
||||
|
||||
workspace.Start();
|
||||
|
||||
using var cancellation = new CancellationTokenSource();
|
||||
@@ -47,20 +59,132 @@ public sealed class TerminalWorkspaceTests
|
||||
await Should.ThrowAsync<OperationCanceledException>(async () => await wait);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task AnOpenSessionIsReportedAsLive()
|
||||
{
|
||||
var connections = new FakeConnectionFactory();
|
||||
|
||||
await using var workspace = CreateWorkspace(connections);
|
||||
|
||||
workspace.LiveSessionCount.ShouldBe(0);
|
||||
|
||||
await workspace.OpenSessionAsync(
|
||||
Request(), TerminalSize.Default, TestContext.Current.CancellationToken);
|
||||
|
||||
workspace.LiveSessionCount.ShouldBe(1);
|
||||
|
||||
await workspace.OpenSessionAsync(
|
||||
Request(), TerminalSize.Default, TestContext.Current.CancellationToken);
|
||||
|
||||
workspace.LiveSessionCount.ShouldBe(2);
|
||||
}
|
||||
|
||||
/// <remarks>
|
||||
/// The connection factory is never reached: every test here stops at the gate, and reaching a real
|
||||
/// host would make this a network test.
|
||||
/// The case a naive <c>sessions.Count</c> gets wrong. Nothing removes the entry when the remote
|
||||
/// closes the channel by itself — the session is still in the dictionary, holding a connection that
|
||||
/// is finished — so counting entries would report a shell that exited as still running. A user
|
||||
/// deciding whether it is safe to walk away is the person that lie is told to.
|
||||
/// </remarks>
|
||||
private static TerminalWorkspace CreateWorkspace(TimeSpan rendererTimeout) =>
|
||||
[Fact]
|
||||
public async Task ASessionWhoseRemoteHasExitedIsNotReportedAsLive()
|
||||
{
|
||||
// A shell with no output to give: its first read returns 0, which is a remote closing the
|
||||
// channel, so the pump finishes on its own with nobody asking it to.
|
||||
var connections = new FakeConnectionFactory(bytesPerShell: 0);
|
||||
|
||||
await using var workspace = CreateWorkspace(connections);
|
||||
|
||||
await workspace.OpenSessionAsync(
|
||||
Request(), TerminalSize.Default, TestContext.Current.CancellationToken);
|
||||
|
||||
await WaitUntilAsync(() => workspace.LiveSessionCount == 0);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task ClosingASessionEndsItAndDisposesItsConnection()
|
||||
{
|
||||
var connections = new FakeConnectionFactory();
|
||||
|
||||
await using var workspace = CreateWorkspace(connections);
|
||||
|
||||
var sessionId = await workspace.OpenSessionAsync(
|
||||
Request(), TerminalSize.Default, TestContext.Current.CancellationToken);
|
||||
|
||||
await workspace.CloseSessionAsync(sessionId);
|
||||
|
||||
workspace.LiveSessionCount.ShouldBe(0);
|
||||
connections.Connections.ShouldHaveSingleItem().IsDisposed.ShouldBeTrue();
|
||||
}
|
||||
|
||||
/// <remarks>
|
||||
/// Disposal is the one path that does close sessions, because it is process shutdown. Asserted so
|
||||
/// that the SSH connections are known to be released rather than assumed to be.
|
||||
/// </remarks>
|
||||
[Fact]
|
||||
public async Task DisposingTheWorkspaceClosesEverySession()
|
||||
{
|
||||
var connections = new FakeConnectionFactory();
|
||||
|
||||
var workspace = CreateWorkspace(connections);
|
||||
|
||||
await workspace.OpenSessionAsync(
|
||||
Request(), TerminalSize.Default, TestContext.Current.CancellationToken);
|
||||
await workspace.OpenSessionAsync(
|
||||
Request(), TerminalSize.Default, TestContext.Current.CancellationToken);
|
||||
|
||||
await workspace.DisposeAsync();
|
||||
|
||||
workspace.LiveSessionCount.ShouldBe(0);
|
||||
connections.Connections.Count.ShouldBe(2);
|
||||
connections.Connections.ShouldAllBe(connection => connection.IsDisposed);
|
||||
}
|
||||
|
||||
// ---- Helpers ----
|
||||
|
||||
/// <param name="connections">
|
||||
/// How connections are made. The renderer-gate tests never reach it — they stop at the gate — but they
|
||||
/// take a fake anyway, because reaching a real host from here would make this a network test.
|
||||
/// </param>
|
||||
/// <param name="rendererTimeout">The gate's bound, or null for the shipped default.</param>
|
||||
private static TerminalWorkspace CreateWorkspace(
|
||||
ISshConnectionFactory connections,
|
||||
TimeSpan? rendererTimeout = null) =>
|
||||
new(
|
||||
new InMemoryTerminalAssetProvider(
|
||||
new Dictionary<string, TerminalAsset>(StringComparer.Ordinal)
|
||||
{
|
||||
[TerminalDataPlane.PagePath] = new(
|
||||
"text/html; charset=utf-8",
|
||||
Encoding.UTF8.GetBytes("<html><body></body></html>")),
|
||||
}),
|
||||
Substitute.For<ISshConnectionFactory>(),
|
||||
StubAssets(),
|
||||
connections,
|
||||
TimeProvider.System,
|
||||
new TerminalWorkspaceOptions { RendererTimeout = rendererTimeout });
|
||||
rendererTimeout is { } timeout
|
||||
? new TerminalWorkspaceOptions { RendererTimeout = timeout }
|
||||
: null);
|
||||
|
||||
private static InMemoryTerminalAssetProvider StubAssets() =>
|
||||
new(new Dictionary<string, TerminalAsset>(StringComparer.Ordinal)
|
||||
{
|
||||
[TerminalDataPlane.PagePath] = new("text/html; charset=utf-8", "<!doctype html>"u8.ToArray()),
|
||||
});
|
||||
|
||||
private static SshConnectionRequest Request() =>
|
||||
new("host.invalid", 22, "dodo", new SshPasswordCredential("irrelevant"));
|
||||
|
||||
/// <remarks>
|
||||
/// Polled rather than awaited on a task, because the point is what an observer of the property
|
||||
/// sees: the pump ends on a thread of its own, and the count has to catch up without anyone
|
||||
/// telling it to.
|
||||
/// </remarks>
|
||||
private static async Task WaitUntilAsync(Func<bool> condition)
|
||||
{
|
||||
var deadline = TimeProvider.System.GetUtcNow() + PollTimeout;
|
||||
|
||||
while (TimeProvider.System.GetUtcNow() < deadline)
|
||||
{
|
||||
if (condition())
|
||||
{
|
||||
return;
|
||||
}
|
||||
|
||||
await Task.Delay(20, TestContext.Current.CancellationToken);
|
||||
}
|
||||
|
||||
throw new TimeoutException($"The condition was still false after {PollTimeout}.");
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user