Files
DodoSSH/tests/DodoSSH.Client.App.Tests/UpdateFlowTests.cs
T
jaap-jan e02491a6ca
ci / build and test (pull_request) Failing after 8s
ci / desktop nightly (pull_request) Skipped
ci / api image (pull_request) Skipped
ci / android head (pull_request) Failing after 21s
Look for a newer build the moment the application starts
The first pass of the update loop waited two minutes. Every pass after it came
six hours apart, which is the right interval for a product that ships rarely —
but the delay in front of the first one quietly excluded a whole way of using
this application.

A client opened to reach one host and closed again is over before the two
minutes are. Used that way, it never checks at all: not once, not slowly, never.
That is precisely the machine ADR 0011 names as the real cost of distributing
outside a store — quietly a year behind — and the galling part is that the
mechanism to fix it was switched on the whole time and simply never reached.

The delay's own argument is recorded in the diff it is being removed from, and
it was not a bad one: nothing anybody does in their first two minutes depends on
an update, and launch is already contending for the network with a schema
migration, a resumed sign-in and a first sync, at the one moment somebody is
watching the window. What it weighed was the cost of checking early against the
benefit of checking early. It never weighed the cost of not checking at all.

◆ THE YIELD IS WHAT KEEPS THIS OFF THE LAUNCH PATH, AND IT IS NOT DECORATION.
Start() is called from MainWindowViewModel.StartAsync ahead of the migration, so
an inline first pass would run whatever the channel does before its own first
await — Velopack reads the install layout from disk — between the user and their
window. Yielding hands the rest of launch back and puts the check in a later
turn, which is the same moment in every sense anybody can perceive and none of
the cost. So the answer to the delay's argument is not that it was wrong; it is
that a yield buys most of what two minutes bought.

Task.Yield takes no token where Task.Delay did, so the loop body now observes
cancellation at its head. Without that, an application closed during launch
spends its last moment asking a release channel about a build it will not run.

Two things deliberately not changed. The AUTOMATIC UPDATE CHECKS preference
still gates the pass — "on start" means every start, not regardless of what the
user asked for, and that setting is already on by default. And the data cost is
unchanged rather than merely acceptable: a check is a few hundred bytes and the
download only follows if something newer exists, so this moves the same traffic
earlier without adding any. That matters most on the phone, where the same loop
runs against AndroidUpdateChannel.

TheFirstPassRunsAtStart_RatherThanOnADelay drives the real loop rather than
CheckOnceAsync, which is the one thing that file otherwise avoids — and here it
is the point, because the claim is about when the pass happens rather than what
it does. It waits on the pass and not on a clock, so there is nothing to be
flaky about: a regression that puts a delay back does not fail on a margin, it
spins until the suite's own cancellation ends it. DisposingStopsTheLoop keeps
its assertion and gains a note that it is now a race rather than a formality.

§16.7 of the manual checks gains the sentence that reopening the application
does what CHECK NOW does. It is the step somebody following that section would
otherwise discover by accident.

447 App tests and 153 layout tests pass.
2026-08-12 10:34:36 +02:00

457 lines
15 KiB
C#

using DodoSSH.Client.Session;
using DodoSSH.Client.Shell.ViewModels;
namespace DodoSSH.Client.App.Tests;
/// <summary>
/// Finding a newer build, fetching it, and not installing it until somebody says so.
/// </summary>
/// <remarks>
/// <para>
/// The view model is driven a pass at a time through <c>CheckOnceAsync</c> rather than through its timer,
/// which is the same split <c>VaultViewModel</c> makes between its sync pass and its sync loop and for the
/// same reason: a suite that waited on a <c>PeriodicTimer</c> would be testing the clock.
/// </para>
/// <para>
/// The load-bearing test here is <see cref="AReadyUpdate_IsNeverAppliedOnItsOwn"/>. Everything else is
/// about how the feature behaves; that one is about the promise it makes.
/// </para>
/// </remarks>
public sealed class UpdateFlowTests : IDisposable
{
private static CancellationToken Token => TestContext.Current.CancellationToken;
private readonly string directory =
Path.Combine(Path.GetTempPath(), $"dodossh-updates-{Guid.CreateVersion7():N}");
private readonly FakeUpdateChannel channel = new();
private int liveSessions;
private int restarts;
private ClientPaths Paths => new(directory);
/// <remarks>
/// The real settings store over a real temporary directory, not a stand-in. Persistence is one of the
/// things being asserted, and the file is the thing that persists.
/// </remarks>
private UpdateViewModel Build() =>
new(
channel,
new ClientSettingsStore(Paths),
TimeProvider.System,
() => liveSessions,
restart: update =>
{
restarts++;
channel.ApplyAndRestart(update);
return Task.CompletedTask;
});
/// <inheritdoc />
public void Dispose()
{
if (Directory.Exists(directory))
{
Directory.Delete(directory, recursive: true);
}
}
[Fact]
public void ABuildThatCannotReplaceItself_OffersNothing()
{
channel.IsSupported = false;
var updates = Build();
updates.State.ShouldBe(UpdateState.Unsupported);
updates.IsUnsupported.ShouldBeTrue();
updates.CanCheckNow.ShouldBeFalse();
updates.IsBannerShowing.ShouldBeFalse();
}
[Fact]
public void ABuildThatCannotReplaceItself_StillKnowsItsOwnVersion()
{
channel.IsSupported = false;
channel.CurrentVersion = "0.4.2";
Build().CurrentVersion.ShouldBe("0.4.2");
}
[Fact]
public async Task ABackgroundPassThatFindsNothing_SaysNothing()
{
var updates = Build();
await updates.CheckOnceAsync(Token);
channel.Checks.ShouldBe(1);
updates.State.ShouldBe(UpdateState.Idle);
updates.Status.ShouldBeEmpty();
}
[Fact]
public async Task APressedCheckThatFindsNothing_SaysSo()
{
channel.CurrentVersion = "1.2.0";
var updates = Build();
await updates.CheckNowCommand.ExecuteAsync(null);
updates.State.ShouldBe(UpdateState.Idle);
updates.Status.ShouldContain("1.2.0");
}
[Fact]
public async Task AnUpdateThatIsFound_IsFetchedWithoutBeingAskedAbout()
{
channel.Available = new AvailableUpdate("1.3.0");
var updates = Build();
await updates.CheckOnceAsync(Token);
channel.Downloads.ShouldBe(1);
updates.State.ShouldBe(UpdateState.Ready);
updates.ReadyVersion.ShouldBe("1.3.0");
updates.DownloadPercent.ShouldBe(100);
updates.IsBannerShowing.ShouldBeTrue();
}
/// <remarks>
/// The whole policy in one assertion. A fetched update sits until a person presses the button, or until
/// the application is next started for their own reasons — because a restart ends every shell, and this
/// application has spent a lot of design effort on shells surviving a lock.
/// </remarks>
[Fact]
public async Task AReadyUpdate_IsNeverAppliedOnItsOwn()
{
channel.Available = new AvailableUpdate("1.3.0");
var updates = Build();
await updates.CheckOnceAsync(Token);
updates.State.ShouldBe(UpdateState.Ready);
channel.Restarts.ShouldBe(0);
restarts.ShouldBe(0);
}
[Fact]
public async Task RestartingApplies_TheUpdateThatWasFound()
{
channel.Available = new AvailableUpdate("1.3.0");
var updates = Build();
await updates.CheckOnceAsync(Token);
await updates.RestartNowCommand.ExecuteAsync(null);
channel.Restarts.ShouldBe(1);
channel.RestartedWith!.Version.ShouldBe("1.3.0");
}
/// <remarks>
/// ◆ The regression for an install that closed the application instead of installing anything. On the
/// phone applying is a request that returns, so a failure in it throws on the spot rather than being
/// cut short by the process ending — and with nothing catching it, it left a command handler, passed
/// the dispatcher and took the process with it. Pressing INSTALL closed the application, said nothing,
/// and installed nothing, which reads as anything but a bug in the updater.
/// </remarks>
[Fact]
public async Task AnInstallThatFails_IsReportedRatherThanThrown()
{
channel.Available = new AvailableUpdate("1.3.0");
channel.ApplyingEndsTheProcess = false;
channel.RestartFailure = new InvalidOperationException("The installer refused the session.");
var updates = Build();
await updates.CheckOnceAsync(Token);
// Not Should.ThrowAsync: the assertion is that this does not throw at all.
await updates.RestartNowCommand.ExecuteAsync(null);
updates.State.ShouldBe(UpdateState.Failed);
updates.Status.ShouldBe("The installer refused the session.");
}
[Fact]
public async Task RestartingWithNothingReady_DoesNothing()
{
var updates = Build();
await updates.RestartNowCommand.ExecuteAsync(null);
channel.Restarts.ShouldBe(0);
}
[Fact]
public async Task DismissingTheBanner_KeepsTheOfferOnPreferences()
{
channel.Available = new AvailableUpdate("1.3.0");
var updates = Build();
await updates.CheckOnceAsync(Token);
updates.DismissBannerCommand.Execute(null);
updates.IsBannerShowing.ShouldBeFalse();
updates.IsReady.ShouldBeTrue();
}
/// <remarks>
/// A forge that cannot be reached is a laptop on a train. Nothing asked, so nothing is said, and the
/// state goes back to Idle rather than to Failed — Failed is reserved for something a person is waiting
/// on an answer to.
/// </remarks>
[Fact]
public async Task ABackgroundCheckThatFails_IsNotAnnounced()
{
channel.CheckFailure = new HttpRequestException("no such host");
var updates = Build();
await updates.CheckOnceAsync(Token);
updates.State.ShouldBe(UpdateState.Idle);
updates.Status.ShouldBeEmpty();
}
[Fact]
public async Task APressedCheckThatFails_SaysWhy()
{
channel.CheckFailure = new HttpRequestException("no such host");
var updates = Build();
await updates.CheckNowCommand.ExecuteAsync(null);
updates.State.ShouldBe(UpdateState.Failed);
updates.Status.ShouldBe("no such host");
}
[Fact]
public async Task TurningOffAutomaticChecks_StopsThePassDoingAnything()
{
channel.Available = new AvailableUpdate("1.3.0");
var updates = Build();
updates.IsAutomatic = false;
await updates.CheckOnceAsync(Token);
channel.Checks.ShouldBe(0);
}
/// <remarks>
/// A second view model over the same <see cref="ClientPaths"/>, which is how this repository tests that
/// a preference reached the disk — the same shape as the terminal font size's own persistence test.
/// </remarks>
[Fact]
public void TurningOffAutomaticChecks_IsStillOffOnTheNextLaunch()
{
var first = Build();
first.IsAutomatic.ShouldBeTrue();
first.IsAutomatic = false;
Build().IsAutomatic.ShouldBeFalse();
}
/// <summary>Storing one preference does not discard the others.</summary>
/// <remarks>
/// <para>
/// Pins the read-modify-write in <c>OnIsAutomaticChanged</c>, which nothing else covered: writing this
/// setting reads the file first, so a font size chosen earlier is still there afterwards. Writing the
/// view model's own state instead would silently reset every preference it does not hold.
/// </para>
/// <para>
/// Note the limit of what read-modify-write buys here, because the comment it guards is easy to read
/// as promising more. It preserves settings <em>this build knows about</em>. A key written by a newer
/// build is dropped, because the store deserialises with
/// <c>JsonUnmappedMemberHandling.Skip</c> — unmapped members are skipped rather than carried, so they
/// do not survive a round trip. That is asserted below rather than left as an assumption, so that
/// anybody who needs forward-compatibility discovers the cost here instead of in the field.
/// </para>
/// </remarks>
[Fact]
public void StoringOnePreference_KeepsTheOthersThisBuildKnows()
{
Directory.CreateDirectory(directory);
File.WriteAllText(
Paths.SettingsFile,
"""{"terminalFontSize":19,"somethingOnlyANewerBuildKnows":"keep me"}""");
Build().IsAutomatic = false;
var reread = new ClientSettingsStore(Paths).Read();
reread.TerminalFontSize.ShouldBe(19);
reread.AutomaticUpdateChecks.ShouldBeFalse();
// The honest limit, stated as an assertion: an unmapped key does not survive.
File.ReadAllText(Paths.SettingsFile).ShouldNotContain("somethingOnlyANewerBuildKnows");
}
/// <summary>
/// A setting missing from the file reads back as the default the record declares.
/// </summary>
/// <remarks>
/// <para>
/// This is the regression test for a defect that was already in the tree and could not bite until a
/// second preference existed. The JSON source generator builds a record through a synthesised
/// parameterised constructor and assigns <em>every</em> property from its argument array, so a member
/// absent from the file arrived as the CLR default and overwrote whatever a property initializer had
/// set. A settings file of <c>{}</c> read back a font size of 0 — clamped to the 8px floor rather than
/// the 13px the renderer draws at — and, once it existed, update checks off.
/// </para>
/// <para>
/// It is asserted on both settings, not just the new one, because the mechanism has nothing to do with
/// either: it is a property of how this record is deserialised, and the next preference somebody adds
/// inherits it. The fix is that the defaults live on the constructor parameters, which is the only
/// place the generator reads them from.
/// </para>
/// <para>
/// A file that exists and lacks the key is the case that matters, and it is not the same as no file at
/// all — <c>Read</c> short-circuits to a fresh record when the file is missing, which is why the bug
/// hid. Every machine that has ever run this application has a settings.json without the newer key.
/// </para>
/// </remarks>
[Fact]
public void ASettingAbsentFromTheFile_ComesBackAsItsDeclaredDefault()
{
Directory.CreateDirectory(directory);
var store = new ClientSettingsStore(Paths);
File.WriteAllText(Paths.SettingsFile, "{}");
var fromEmptyObject = store.Read();
fromEmptyObject.AutomaticUpdateChecks.ShouldBeTrue();
fromEmptyObject.TerminalFontSize.ShouldBe(ClientSettings.DefaultTerminalFontSize);
// The realistic shape: a file written by the build before this feature existed.
File.WriteAllText(Paths.SettingsFile, """{"terminalFontSize":19}""");
var fromOlderBuild = store.Read();
fromOlderBuild.TerminalFontSize.ShouldBe(19);
fromOlderBuild.AutomaticUpdateChecks.ShouldBeTrue();
}
[Fact]
public async Task TheRestartWarning_CountsTheShellsItWouldClose()
{
channel.Available = new AvailableUpdate("1.3.0");
var updates = Build();
await updates.CheckOnceAsync(Token);
liveSessions = 0;
updates.RestartWarning.ShouldContain("Nothing is connected");
liveSessions = 1;
updates.RestartWarning.ShouldContain("the shell you have open");
liveSessions = 3;
updates.RestartWarning.ShouldContain("the 3 shells you have open");
}
[Fact]
public async Task ADownloadInFlight_IsAStateTheScreenCanShow()
{
channel.Available = new AvailableUpdate("1.3.0");
channel.HoldDownload = new TaskCompletionSource();
var updates = Build();
var pass = updates.CheckOnceAsync(Token);
// The gate is what makes this observable at all; without it the download would be over before
// anything could look.
while (updates.State is not UpdateState.Downloading)
{
await Task.Yield();
}
updates.IsDownloading.ShouldBeTrue();
updates.CanCheckNow.ShouldBeFalse();
channel.HoldDownload.SetResult();
await pass;
updates.State.ShouldBe(UpdateState.Ready);
}
[Fact]
public async Task APassWhileOneIsAlreadyReady_DoesNotAskAgain()
{
channel.Available = new AvailableUpdate("1.3.0");
var updates = Build();
await updates.CheckOnceAsync(Token);
await updates.CheckOnceAsync(Token);
channel.Checks.ShouldBe(1);
}
/// <remarks>
/// <para>
/// The loop rather than <c>CheckOnceAsync</c>, which is the one thing the rest of this file avoids
/// driving — and here it is the whole point, because the claim is about when the first pass happens
/// rather than about what it does. The first pass used to wait two minutes, which meant a client opened
/// to reach one host and closed again never asked at all.
/// </para>
/// <para>
/// It waits on the pass and not on a clock, so there is nothing here to be flaky about: a regression
/// that puts a delay back in front of the loop does not fail on a margin, it spins until the suite's own
/// cancellation ends it.
/// </para>
/// </remarks>
[Fact]
public async Task TheFirstPassRunsAtStart_RatherThanOnADelay()
{
channel.Available = new AvailableUpdate("1.3.0");
var updates = Build();
await using var _ = updates.ConfigureAwait(false);
updates.Start();
while (updates.State is not UpdateState.Ready)
{
Token.ThrowIfCancellationRequested();
await Task.Yield();
}
channel.Checks.ShouldBe(1);
updates.ReadyVersion.ShouldBe("1.3.0");
}
/// <remarks>
/// Started and disposed with nothing in between, which since the first pass stopped waiting two minutes
/// is a race rather than a formality: the loop may be anywhere between its yield and a finished check
/// when the cancellation lands. What is asserted is what matters either way — that disposing returns,
/// rather than waiting on a pass that will never be allowed to finish.
/// </remarks>
[Fact]
public async Task DisposingStopsTheLoop()
{
var updates = Build();
updates.Start();
await updates.DisposeAsync();
await updates.DisposeAsync();
}
}