Files
DodoSSH/tests/DodoSSH.Client.App.Layout.Tests/TerminalTabsTests.cs
T
jaap-jan 7b7fd7b2ef Make a vault the thing you create, and let a window set one aside
Everything a shared vault needs was already here and arranged the wrong way
round. A vault has to belong to a team, so creating one meant going to the teams
screen, founding an organisation, and only then adding a vault to it — which the
NEW VAULT button named after the team, so a team with three of them held three
vaults called the same thing and nothing told them apart. Somebody who wants to
share four servers with two colleagues is not asking to found anything.

So the form asks for a name and nothing else. The team is derived from it, slug
included, and created with this account as its owner; the vault goes inside; and
the members, roles, invitations and key holders that hang off a team are all on
screen the moment it exists. The tab strip's New vault entry lands there with the
new vault selected, which is where the next thing anybody wants to do already is.

That is two calls, and the first can succeed alone. When it does the team is
kept: the id is minted once into pendingVaultTeamId, so pressing CREATE again
resends the identical create — which the server treats as the same team — and
retries the vault, and the message says all of that rather than "creating the
vault failed". Archiving the orphan instead would be a client deleting something
on the user's behalf because a later step failed, which is the kind of tidying
that eventually archives a team somebody has just been added to. A slug taken by
somebody else is retried once with a disambiguated one and never in a loop; a
name with no a-z or 0-9 anywhere in it falls back to the team's own id rather
than to a refusal pointing at a field nobody was shown.

The other half is the caret beside Vaults. Being in four teams means four teams'
machines in front of you all day, and the answer is a switch per vault rather
than four sign-ins. Switching one off takes its hosts, groups, keys and pins off
the screens that list them and does nothing else: it still syncs, its key stays
in the keyring, it stays choosable as somewhere to file a new item, and a shown
host that authenticates with a key filed in it still connects. That last one is
what shaped the design. TryBuildAuthentication resolves a binding out of the
keychain's typed list and a cross-vault binding is legal, so filtering the reload
loops — the obvious implementation — would have turned a preference about reading
into an outage. Only the projections a person reads consult IsVaultShown; every
Reload*Async stays whole, including the dialled-endpoint set that decides which
pins are described as unused, because that is a hint which invites deleting
trust.

Snippets, logs and buckets needed no code and the comment says so out loud: all
three read ActiveVaultId alone, and the personal vault is drawn in the menu
ticked and cannot be switched off — it is the active vault, the group and tag
editors' target, and the save picker's fallback, so hiding it would empty half
the application rather than filter it.

The preference is a column on the cache's vault row, which is what makes it
survive both a relaunch and the /me refresh that runs every minute: Apply does
not touch it, deliberately, because the server has never been told which vaults
this machine is showing. It is in the encrypted cache rather than settings.json
because it is a list of vault ids and that file's own doc comment says what may
go in it. VaultSession cannot see the type at all — ReadableVaults is what the
sync loop walks, and a filter reaching it would be a vault that quietly stopped
syncing, found out weeks later from a host that was never there.

The strip's note refusing a MenuFlyout stands and is unchanged. This flyout
sidesteps the question rather than answering it: the handler selects the Vaults
tab first, which collapses the renderer, so nothing native is under the popup by
the time it opens — the move QuickConnect already makes. A headless test asserts
that ordering, which is as far as headless can go with no native window, and
manual check 1.6 is the other half.

The phone is out of scope on purpose: it has no tab strip and its teams screen's
vault section is read-only. The plumbing is in Client.Shell, so it can adopt this
later; until then nothing there is ever hidden, which is today's behaviour.

1514 tests pass. Fifteen are new in VaultVisibilityTests, and the ones worth
naming are the guards: a hidden vault still syncs, still holds keys that
authenticate hosts on screen, still appears in the save picker, and still counts
towards which pins nothing dials.

Not fixed, and noted here because it is next door: VaultGrantService's team-vault
create refuses a taken vault id rather than returning the existing vault, while
VaultSharing's own remark claims a create whose response was lost is safe to
resend. A lost 200 therefore leaves a vault whose key the client's catch already
zeroed, openable by nobody.
2026-08-03 21:52:27 +02:00

508 lines
21 KiB
C#

using Avalonia;
using Avalonia.Controls;
using Avalonia.Controls.Presenters;
using Avalonia.Controls.Primitives;
using Avalonia.Headless;
using Avalonia.Input;
using Avalonia.Media;
using Avalonia.VisualTree;
using DodoSSH.Client.App.Views;
using DodoSSH.Client.Session;
using DodoSSH.Client.Shell.ViewModels;
using DodoSSH.Client.Ssh;
using DodoSSH.Client.Storage;
using DodoSSH.Client.Terminal;
using NSubstitute;
namespace DodoSSH.Client.App.Layout.Tests;
/// <summary>
/// How the tab strip answers a pointer.
/// </summary>
/// <remarks>
/// <para>
/// The strip spans every screen now, so it is chrome a user is in contact with all day rather than one
/// column of the hosts screen. What that earns it is the gestures every other tabbed application has — a
/// middle click that closes, a cross inside the tab rather than beside it, a button that opens another — and
/// what those need is a suite, because all three are pointer behaviour and none of it is expressible as a
/// binding.
/// </para>
/// <para>
/// A <c>UserControl</c> in a bare window, for the reason the palette's suite is one:
/// <see cref="LayoutHarnessTests.WhyTheWindowItselfIsNeverShown"/>. No vault and no session — the strip
/// binds only to the shell's tab list, and tabs are shell state that outlives the vault that opened them, so
/// they can be put there directly. Closing one asks the workspace to end a session it has never heard of,
/// which the workspace answers by returning: that is the same path a real close takes, minus a shell.
/// </para>
/// </remarks>
public sealed class TerminalTabsTests : IAsyncLifetime
{
private ClientCacheFactory caches = null!;
private TerminalWorkspace workspace = null!;
private MainWindowViewModel shell = null!;
private static CancellationToken Token => TestContext.Current.CancellationToken;
/// <inheritdoc />
public ValueTask InitializeAsync()
{
caches = ClientCacheFactory.ForMemory($"tabs-{Guid.CreateVersion7():N}");
workspace = new TerminalWorkspace(
new InMemoryTerminalAssetProvider(new Dictionary<string, TerminalAsset>(StringComparer.Ordinal)),
Substitute.For<ISshConnectionFactory>(),
TimeProvider.System);
shell = new MainWindowViewModel(
ClientPaths.Default,
caches,
workspace,
new VaultKnownHostStore(),
Substitute.For<IDeviceKeyStore>(),
(_, _) => throw new NotSupportedException("nothing here signs in"),
TimeProvider.System,
Substitute.For<ISftpSessionFactory>())
{
// The only state the strip is ever interactive in. Assigned rather than reached through an
// enrollment, which would be an Argon2 pass for no extra coverage — nothing here reads the vault.
State = ShellState.Unlocked,
};
return ValueTask.CompletedTask;
}
/// <inheritdoc />
public async ValueTask DisposeAsync()
{
await shell.DisposeAsync();
await workspace.DisposeAsync();
caches.Dispose();
}
/// <remarks>
/// The gesture this rework is for. Middle-clicking a tab is how every browser and every terminal closes
/// one, and the strip answered nothing but a left click before.
/// </remarks>
[Fact]
public async Task AMiddleClickOnATabClosesThatTab()
{
await OnTheStripAsync((strip, window) =>
{
var doomed = shell.Tabs[0];
var survivor = shell.Tabs[1];
window.MouseDown(Centre(TabButton(strip, doomed), window), MouseButton.Middle);
shell.Tabs.ShouldHaveSingleItem().ShouldBe(survivor);
});
}
/// <remarks>
/// The other half of the rule, and the reason the handler is on the tab's own template root rather than
/// on the strip: a middle click on the chrome between the last tab and the edge of the window must not
/// close anything. Wiring it on the strip and testing what was underneath the pointer would have been
/// the same feature with a way to get it wrong.
/// </remarks>
[Fact]
public async Task AMiddleClickOnTheStripBackgroundClosesNothing()
{
await OnTheStripAsync((strip, window) =>
{
// Well right of two short tabs and the button after them, and inside the strip's own height.
window.MouseDown(new Point(700, 17), MouseButton.Middle);
shell.Tabs.Count.ShouldBe(2);
});
}
[Fact]
public async Task AMiddleClickOnTheButtonThatOpensAConnectionClosesNothing()
{
await OnTheStripAsync((strip, window) =>
{
window.MouseDown(Centre(PlusButton(strip), window), MouseButton.Middle);
shell.Tabs.Count.ShouldBe(2);
shell.IsSearching.ShouldBeFalse("a middle click is not how the palette opens either");
});
}
/// <remarks>
/// The cross is inside the tab, so a middle click on it bubbles out to the tab's handler as well. One
/// close, not two: the second would take the neighbour, which is the tab the user was aiming to keep.
/// </remarks>
[Fact]
public async Task AMiddleClickOnTheCrossClosesExactlyOneTab()
{
await OnTheStripAsync((strip, window) =>
{
var survivor = shell.Tabs[1];
window.MouseDown(Centre(CloseButton(strip, shell.Tabs[0]), window), MouseButton.Middle);
shell.Tabs.ShouldHaveSingleItem().ShouldBe(survivor);
});
}
/// <remarks>
/// The one assumption the nested-button template makes, stated as a test. Avalonia's
/// <c>Button.OnPointerPressed</c> takes the capture and marks a left press handled, so the cross does
/// not also reach the tab underneath it — which would select a tab on its way out and leave the
/// terminal switching to something that is about to disappear.
/// </remarks>
[Fact]
public async Task ALeftClickOnTheCrossClosesTheTabAndDoesNotSelectIt()
{
await OnTheStripAsync((strip, window) =>
{
var doomed = shell.Tabs[0];
var survivor = shell.Tabs[1];
shell.SelectTabCommand.Execute(survivor);
var cross = CloseButton(strip, doomed);
window.MouseDown(Centre(cross, window), MouseButton.Left);
window.MouseUp(Centre(cross, window), MouseButton.Left);
shell.Tabs.ShouldHaveSingleItem().ShouldBe(survivor);
shell.SelectedTab.ShouldBe(survivor);
});
}
[Fact]
public async Task ALeftClickOnATabSelectsItAndShowsTheTerminal()
{
await OnTheStripAsync((strip, window) =>
{
var wanted = shell.Tabs[1];
shell.ShowScreenCommand.Execute(ShellScreen.Preferences);
shell.IsTerminalShowing.ShouldBeFalse();
var button = TabButton(strip, wanted);
window.MouseDown(Centre(button, window), MouseButton.Left);
window.MouseUp(Centre(button, window), MouseButton.Left);
shell.Tabs.Count.ShouldBe(2, "selecting is not closing");
shell.SelectedTab.ShouldBe(wanted);
shell.IsTerminalShowing.ShouldBeTrue();
});
}
/// <remarks>
/// It opens the palette rather than a menu, so that the strip and Ctrl+K are one way of doing one thing.
/// See the note in <c>TerminalTabs.axaml</c> for why a flyout over the terminal's rectangle is not a
/// claim this project is willing to make without a screenshot.
/// </remarks>
[Fact]
public async Task TheButtonThatOpensAConnectionOpensThePalette()
{
await OnTheStripAsync((strip, window) =>
{
var plus = PlusButton(strip);
window.MouseDown(Centre(plus, window), MouseButton.Left);
window.MouseUp(Centre(plus, window), MouseButton.Left);
shell.IsSearching.ShouldBeTrue();
});
}
/// <summary>
/// The three fixed tabs select what they name, and Vaults comes back to the page it was left on.
/// </summary>
/// <remarks>
/// <para>
/// The memory is the part worth a gesture rather than a property assertion. Vaults is the one tab with
/// sub-navigation, so it is the one that can come back to the wrong place — and the failure is silent:
/// a Vaults tab that always landed on Hosts looks like a working tab to anybody who was already on
/// Hosts, which is most of the time.
/// </para>
/// <para>
/// Driven through the strip rather than through the commands, because what is being checked is that
/// three buttons in the markup are wired to three different things. Three commands called directly
/// would pass on a strip whose SFTP tab was bound to the S3 one.
/// </para>
/// </remarks>
[Fact]
public async Task TheFixedTabsSelectTheirSurface_AndVaultsRemembersItsPage()
{
await OnTheStripAsync((strip, window) =>
{
shell.ShowScreenCommand.Execute(ShellScreen.Snippets);
shell.IsVaultsTab.ShouldBeTrue("a rail screen is under the Vaults tab");
Click(FixedTab(strip, "SFTP"), window);
shell.IsTransfersShowing.ShouldBeTrue();
shell.IsVaultsTab.ShouldBeFalse("exactly one tab is lit at a time");
Click(FixedTab(strip, "S3"), window);
shell.IsBucketsShowing.ShouldBeTrue();
shell.IsTransfersShowing.ShouldBeFalse();
Click(FixedTab(strip, "Vaults"), window);
shell.IsVaultsTab.ShouldBeTrue();
shell.Screen.ShouldBe(
ShellScreen.Snippets,
"the Vaults tab comes back to the page it was left on, not to Hosts");
});
}
/// <remarks>
/// The caret is the second half of the Vaults pill and the only control in this strip that opens a
/// popup. Asserted as behaviour rather than as markup, because what makes it correct is the order in
/// the handler rather than the flyout being attached — see the test below.
/// </remarks>
[Fact]
public async Task TheCaretBesideVaults_OpensTheVaultMenu()
{
await OnTheStripAsync((strip, window) =>
{
var caret = CaretButton(strip);
FlyoutBase.GetAttachedFlyout(caret)!.IsOpen.ShouldBeFalse("nothing has been pressed yet");
Click(caret, window);
FlyoutBase.GetAttachedFlyout(caret)!.IsOpen.ShouldBeTrue();
});
}
/// <summary>
/// Opening the vault menu selects the Vaults tab first, so the renderer is collapsed under it.
/// </summary>
/// <remarks>
/// <para>
/// <b>The occlusion guard, and the reason this strip may have a flyout at all.</b> The comment on the
/// <c>+</c> button refuses one because a popup dropping into the terminal's rectangle would have to
/// composite above a native child window, which this project does not claim without a screenshot. The
/// caret sidesteps the question rather than answering it: it goes to the Vaults tab before it opens,
/// and a page surface is one where the renderer is not drawn.
/// </para>
/// <para>
/// So the assertion is on <see cref="MainWindowViewModel.IsTerminalShowing"/> rather than on anything
/// about the popup. A change that opened the flyout without moving the surface first would still show a
/// menu in every screenshot anybody took on a machine where it happened to work.
/// </para>
/// </remarks>
[Fact]
public async Task OpeningTheVaultMenu_SelectsTheVaultsTabSoTheTerminalIsNotUnderIt()
{
await OnTheStripAsync((strip, window) =>
{
shell.SelectTabCommand.Execute(shell.Tabs[0]);
shell.IsTerminalShowing.ShouldBeTrue("this test is meaningless without one in the way");
Click(CaretButton(strip), window);
shell.IsVaultsTab.ShouldBeTrue();
shell.IsTerminalShowing.ShouldBeFalse(
"the flyout must never have to composite over the renderer's native child window");
});
}
/// <remarks>
/// None of the three owns a shell, so none of them may offer to end one. The cross is what tells a
/// destination from a machine in this strip, and a fixed tab that grew one would be offering to close
/// SFTP.
/// <para>
/// The Vaults caret is a sibling of its tab rather than a child, which is what keeps this assertion
/// meaning what it says: a button inside a fixed tab would still be a close box.
/// </para>
/// </remarks>
[Fact]
public async Task TheFixedTabsCarryNoCloseBox()
{
await OnTheStripAsync((strip, _) =>
{
foreach (var label in new[] { "Vaults", "SFTP", "S3" })
{
FixedTab(strip, label)
.GetVisualDescendants()
.OfType<Button>()
.ShouldBeEmpty($"{label} is a destination, not a session");
}
});
}
/// <remarks>
/// The strip is the one row of chrome every screen pays for, so its height is part of the layout budget
/// and this is what stops the budget drifting from the markup. See
/// <see cref="LayoutHarness.TerminalTabsHeight"/>.
/// </remarks>
[Fact]
public async Task TheStripIsTheHeightTheBudgetAssumes_AndDoesNotGrowWithTabs()
{
await LayoutHarness.OnTheUiThreadAsync(
() =>
{
for (var i = 0; i < 12; i++)
{
shell.Tabs.Add(new TerminalTabViewModel((uint)i, $"host-{i}", $"deploy@host-{i}:22"));
}
var strip = new TerminalTabs { DataContext = shell };
var window = LayoutHarness.HostAtMinimumSize(
strip, LayoutHarness.MinimumWidth, LayoutHarness.MinimumHeight);
try
{
// What it asks for, not what this host window gave it. Hosting it at 34 and then
// asserting it is 34 would pass on a strip that wanted 300 and got clipped, which is
// exactly the regression the budget needs catching.
strip.DesiredSize.Height.ShouldBe(LayoutHarness.TerminalTabsHeight);
LayoutHarness.Unreachable(window).ShouldBeEmpty();
}
finally
{
window.Close();
}
},
Token);
}
/// <summary>
/// A tab lights under the pointer, and the button that opens one is not drawn as a tab.
/// </summary>
/// <remarks>
/// <para>
/// The only test in this suite that reads a brush rather than a rectangle, and it is here because that
/// was the gap a real regression went through. Everything else measures heights and reachability, so a
/// strip whose tabs had silently stopped answering the pointer passed all of it.
/// </para>
/// <para>
/// What went wrong is worth stating, because the shape of it will recur. Avalonia has no specificity —
/// the later declaration wins — and when the tab became a pill that paints its own background, that
/// background was declared *after* the hover rule it relied on and after the exceptions the <c>+</c>
/// is made of. So every tab lost its pointer feedback and the <c>+</c> gained a fill and an outline it
/// is specifically not supposed to have. Both are one assertion each below.
/// </para>
/// </remarks>
[Fact]
public async Task ATabLightsUnderThePointer_AndThePlusIsNotDrawnAsATab()
{
await OnTheStripAsync(
(strip, window) =>
{
var tab = TabButton(strip, shell.Tabs[0]);
var resting = Fill(tab);
window.MouseMove(Centre(tab, window));
LayoutHarness.Settle(window, 900, 600);
tab.IsPointerOver.ShouldBeTrue("the pointer was moved onto it");
Fill(tab).ShouldNotBe(
resting,
"a tab that does not change under the pointer is one nobody can tell is clickable");
// Off the strip again, so the plus is measured at rest rather than under the pointer.
window.MouseMove(new Point(0, 0));
LayoutHarness.Settle(window, 900, 600);
var plus = PlusButton(strip);
Fill(plus).ShouldNotBe(
Fill(TabButton(strip, shell.Tabs[0])),
"the button that opens a connection is not one of the connections");
Presenter(plus).BorderThickness.ShouldBe(
default(Thickness),
"it carries no outline, because it is not a thing being chosen between");
});
}
// ---- Helpers ----
/// <summary>The presenter the Fluent theme actually paints, which is where every button style lands.</summary>
private static ContentPresenter Presenter(Visual button) =>
button.GetVisualDescendants()
.OfType<ContentPresenter>()
.First(presenter => presenter.Name is "PART_ContentPresenter");
/// <remarks>
/// The colour rather than the brush. Two <see cref="ISolidColorBrush"/> instances holding the same
/// colour are not equal, and it is the colour a user sees.
/// </remarks>
private static Color? Fill(Visual button) =>
Presenter(button).Background is ISolidColorBrush brush ? brush.Color : null;
/// <summary>Two open tabs, laid out in a window the width the application's is.</summary>
private Task OnTheStripAsync(Action<TerminalTabs, Window> body) =>
LayoutHarness.OnTheUiThreadAsync(
() =>
{
shell.Tabs.Add(new TerminalTabViewModel(1, "prod-db", "deploy@db.internal:22"));
shell.Tabs.Add(new TerminalTabViewModel(2, "web-01", "deploy@web-01.internal:22"));
var strip = new TerminalTabs { DataContext = shell };
var window = new Window { Content = strip };
LayoutHarness.Settle(window, 900, 600);
try
{
body(strip, window);
}
finally
{
window.Close();
}
},
Token);
/// <remarks>
/// Found by the class the style system already keys on, rather than by position in the visual tree: the
/// template puts the cross inside the tab, so both buttons carry the same data context and only the
/// classes tell them apart.
/// </remarks>
private static Button TabButton(Visual strip, TerminalTabViewModel tab) =>
strip.GetVisualDescendants()
.OfType<Button>()
.First(button => ReferenceEquals(button.DataContext, tab) && button.Classes.Contains("tab"));
/// <inheritdoc cref="TabButton" />
private static Button CloseButton(Visual strip, TerminalTabViewModel tab) =>
strip.GetVisualDescendants()
.OfType<Button>()
.First(button => ReferenceEquals(button.DataContext, tab) && button.Classes.Contains("close"));
/// <summary>The half of the Vaults pill that opens the vault menu.</summary>
/// <inheritdoc cref="TabButton" path="/remarks" />
private static Button CaretButton(Visual strip) =>
strip.GetVisualDescendants().OfType<Button>().First(button => button.Classes.Contains("caret"));
/// <inheritdoc cref="TabButton" />
private static Button PlusButton(Visual strip) =>
strip.GetVisualDescendants().OfType<Button>().First(button => button.Classes.Contains("plus"));
/// <summary>One of the three tabs that are always there, found by the word on it.</summary>
/// <remarks>
/// By its label rather than by its position in the strip, so that adding a fourth or reordering the
/// three does not silently point these tests at the wrong one. The class narrows it to a fixed tab
/// first, because a terminal tab could be opened on a host called SFTP.
/// </remarks>
private static Button FixedTab(Visual strip, string label) =>
strip.GetVisualDescendants()
.OfType<Button>()
.First(button => button.Classes.Contains("fixed")
&& button.GetVisualDescendants()
.OfType<TextBlock>()
.Any(text => string.Equals(text.Text, label, StringComparison.Ordinal)));
private static void Click(Visual control, Window window)
{
var at = Centre(control, window);
window.MouseDown(at, MouseButton.Left);
window.MouseUp(at, MouseButton.Left);
}
private static Point Centre(Visual control, Visual window) =>
control.TranslatePoint(new Point(control.Bounds.Width / 2, control.Bounds.Height / 2), window)
?? throw new InvalidOperationException("the control is not in this window's tree");
}