diff --git a/docs/manual-checks.md b/docs/manual-checks.md index 6886b5f..97a9c47 100644 --- a/docs/manual-checks.md +++ b/docs/manual-checks.md @@ -2706,6 +2706,32 @@ having been broken, and the fallback is to move the offer into the titlebar inst the swap having been blocked, usually by a process still holding a file under `current\`. Being asked to enrol again means the profile directory did not survive, which is 16.1's failure arriving late. +### 16.7a Two releases published while it is running cost one update · **needs two releases** + +The nightly channel is the easier place to do this — every push to `main` publishes one — but it works the +same way on the release channel with two tags. With the client running and the banner already up for one +build, publish a second. Then press **RESTART NOW** on the banner. + +**Pass:** one download and one restart, landing on the **second** build. The banner says +*"A newer build appeared. Fetching it before restarting"* with a percentage while it does, and it stays on +screen throughout rather than vanishing. Starting up afterwards offers nothing, because there is nothing +left to offer. + +**And the timer's half, if you have six hours or a debugger.** Leave the client running past a check +interval with a build already fetched and a newer one published. The banner's version changes on its own, +and LATER having been pressed does not stop it coming back — a different version is a different question. + +**Failure means:** two downloads and two restarts is the regression this check exists for, and it was the +shipped behaviour: the background pass skipped itself entirely once anything was fetched, so a running +client was pinned to whichever build it first saw and installed that, then found the next one at the launch +straight afterwards. On a rolling channel — where the release is deleted and remade on every push, so the +feed holds exactly one build — that was the normal case rather than a corner. + +A banner that disappears when RESTART NOW is pressed is `IsRestarting` not reaching it; everything the strip +draws keys off `IsReady`, and the last look moves the state off that. A restart that does not happen at all +when the forge is unreachable is the last look having become a gate: it is a courtesy, and the build already +on disk is still newer than the one running. + ### 16.8 The first connect after an update is not a cold start Immediately after 16.7, connect to a host. diff --git a/src/DodoSSH.Client.App/Platform/VelopackUpdateChannel.cs b/src/DodoSSH.Client.App/Platform/VelopackUpdateChannel.cs index 346be69..aa48656 100644 --- a/src/DodoSSH.Client.App/Platform/VelopackUpdateChannel.cs +++ b/src/DodoSSH.Client.App/Platform/VelopackUpdateChannel.cs @@ -204,6 +204,25 @@ internal sealed class VelopackUpdateChannel : IUpdateChannel /// private UpdateInfo? found; + /// + /// The last thing actually fetched, kept separately from the last thing found. + /// + /// + /// + /// ◆ Because the two can disagree, and when they do it is an apply that pays for it. A check + /// overwrites , and the shell now checks while a build is already downloaded and + /// waiting — that is how it replaces a pending build when the feed has moved on, and how it takes one + /// last look on the way to a restart. Either of those can land a newer answer in found and then + /// fail to fetch it, or be given up on halfway; the shell's answer is to restart with the build it + /// already has, and resolving only against found would have thrown at it. + /// + /// + /// Which turned a slow forge into a refused restart: the offer on screen was real, the package on disk + /// was real, and the only thing wrong was that this class had forgotten which one it was. + /// + /// + private UpdateInfo? fetched; + internal VelopackUpdateChannel(UpdateManager manager) => this.manager = manager; /// @@ -321,7 +340,23 @@ internal sealed class VelopackUpdateChannel : IUpdateChannel // Velopack reports progress as an Action and the rest of this codebase speaks IProgress, // so the adaptation happens here rather than leaking the older shape into the view models. - return manager.DownloadUpdatesAsync(Matched(update), progress.Report, cancellationToken); + return DownloadAndRememberAsync(Matched(update), progress, cancellationToken); + } + + /// + /// Recorded after the await and not before, so that a download which threw leaves the previous fetch as + /// the one this channel can still apply. See . + /// + private async Task DownloadAndRememberAsync( + UpdateInfo info, + IProgress progress, + CancellationToken cancellationToken) + { + await manager + .DownloadUpdatesAsync(info, progress.Report, cancellationToken) + .ConfigureAwait(false); + + fetched = info; } /// @@ -335,21 +370,39 @@ internal sealed class VelopackUpdateChannel : IUpdateChannel } /// + /// /// The guard exists because the seam narrows UpdateInfo down to a version string, so nothing in /// the type system stops a caller inventing one. Every legitimate caller passes back exactly what - /// returned; a mismatch is a bug in this application rather than anything a - /// user did, which is why it throws rather than resolving to some safe-looking default. + /// returned, or what a download of that then fetched; a mismatch is a bug in + /// this application rather than anything a user did, which is why it throws rather than resolving to + /// some safe-looking default. + /// + /// + /// What was fetched is tried before what was found, because that is the one an apply names and the one a + /// later check cannot take away. See for how the two come to disagree. + /// /// private UpdateInfo Matched(AvailableUpdate update) { - if (found is not { } info - || !string.Equals(info.TargetFullRelease.Version.ToString(), update.Version, StringComparison.Ordinal)) + if (Names(fetched, update)) { - throw new InvalidOperationException( - $"No update matching {update.Version} has been found by this channel. " - + "Call CheckAsync and pass back what it returned."); + return fetched!; } - return info; + if (Names(found, update)) + { + return found!; + } + + throw new InvalidOperationException( + $"No update matching {update.Version} has been found or fetched by this channel. " + + "Call CheckAsync and pass back what it returned."); } + + private static bool Names(UpdateInfo? info, AvailableUpdate update) => + info is not null + && string.Equals( + info.TargetFullRelease.Version.ToString(), + update.Version, + StringComparison.Ordinal); } diff --git a/src/DodoSSH.Client.App/Views/SettingsGeneralPage.axaml b/src/DodoSSH.Client.App/Views/SettingsGeneralPage.axaml index d07c6ad..3c77313 100644 --- a/src/DodoSSH.Client.App/Views/SettingsGeneralPage.axaml +++ b/src/DodoSSH.Client.App/Views/SettingsGeneralPage.axaml @@ -47,7 +47,7 @@ + Text="Every six hours while DodoSSH is running, starting at launch. It keeps checking while the keychain is locked — where builds come from has nothing to do with your vault — and while an update is already waiting, so what is offered is the newest build rather than the first one this run happened to see." /> private static readonly TimeSpan CheckInterval = TimeSpan.FromHours(6); + /// How long the look taken on the way to a restart may take before it is given up on. + /// + /// + /// Ten seconds. It bounds one HTTPS request for a small JSON document, which ordinarily answers in well + /// under one — and the case it exists for is not a slow answer but no answer at all: a captive portal or + /// a firewall that drops packets rather than refusing them, where the underlying handler's own timeout is + /// a hundred seconds. A refused connection fails in milliseconds and never reaches this. + /// + /// + /// Bounded because somebody is waiting on it with a finger on RESTART NOW, which is the whole difference + /// from the timer's own pass. Giving up is not a failure here: the build already on disk is still newer + /// than the one running, so the restart goes ahead with it. + /// + /// + private static readonly TimeSpan LastLookTimeout = TimeSpan.FromSeconds(10); + private readonly IUpdateChannel updates; private readonly ClientSettingsStore settings; private readonly TimeProvider clock; @@ -143,6 +159,18 @@ internal sealed partial class UpdateViewModel : ObservableObject, IAsyncDisposab [ObservableProperty] private DateTimeOffset? lastChecked; + /// + /// Whether a restart is under way, including the look it takes before replacing anything. + /// + /// + /// It exists to keep the banner up. Everything the banner shows keys off , and the + /// last look moves the state to Checking and possibly Downloading — so without this, pressing RESTART NOW + /// makes the strip vanish and leaves nothing on screen saying why, for however long a package takes. See + /// , which is what it says instead. + /// + [ObservableProperty] + private bool isRestarting; + internal bool IsSupported => State is not UpdateState.Unsupported; internal bool IsUnsupported => State is UpdateState.Unsupported; @@ -155,12 +183,37 @@ internal sealed partial class UpdateViewModel : ObservableObject, IAsyncDisposab internal bool CanCheckNow => IsSupported && State is not (UpdateState.Checking or UpdateState.Downloading); - internal bool IsBannerShowing => IsReady && !IsBannerDismissed; + internal bool IsBannerShowing => (IsReady || IsRestarting) && !IsBannerDismissed; /// What the banner says. - internal string ReadyHeadline => ReadyVersion is { Length: > 0 } version - ? $"DodoSSH {version} is ready to install." - : "An update is ready to install."; + /// + /// Three sentences rather than one, because a restart no longer necessarily starts with a build in hand: + /// it takes one last look first, and may find something newer than what was fetched six hours ago. That + /// look is short and the fetch behind it is not, so both say what they are doing — a strip that read + /// "ready to install" while it downloaded would be describing the wrong build, and a blank one would be + /// describing nothing. + /// + internal string ReadyHeadline + { + get + { + if (IsRestarting && State is UpdateState.Checking) + { + return "Checking for a newer build before restarting…"; + } + + if (IsRestarting && State is UpdateState.Downloading) + { + return string.Create( + CultureInfo.CurrentCulture, + $"A newer build appeared. Fetching it before restarting — {DownloadPercent}%"); + } + + return ReadyVersion is { Length: > 0 } version + ? $"DodoSSH {version} is ready to install." + : "An update is ready to install."; + } + } /// What restarting costs, in the terms this application has already taught. /// @@ -201,10 +254,24 @@ internal sealed partial class UpdateViewModel : ObservableObject, IAsyncDisposab OnPropertyChanged(nameof(CanCheckNow)); OnPropertyChanged(nameof(IsBannerShowing)); OnPropertyChanged(nameof(RestartWarning)); + OnPropertyChanged(nameof(ReadyHeadline)); } partial void OnIsBannerDismissedChanged(bool value) => OnPropertyChanged(nameof(IsBannerShowing)); + partial void OnIsRestartingChanged(bool value) + { + OnPropertyChanged(nameof(IsBannerShowing)); + OnPropertyChanged(nameof(ReadyHeadline)); + } + + /// + /// The headline carries the percentage while a restart is fetching, so it has to be re-read as the bar + /// moves. Raised unconditionally rather than only while restarting, because a condition here would be a + /// second place that has to agree with the headline's own about when the percentage is in it. + /// + partial void OnDownloadPercentChanged(int value) => OnPropertyChanged(nameof(ReadyHeadline)); + partial void OnReadyVersionChanged(string? value) => OnPropertyChanged(nameof(ReadyHeadline)); partial void OnLastCheckedChanged(DateTimeOffset? value) => OnPropertyChanged(nameof(LastCheckedSummary)); @@ -278,14 +345,32 @@ internal sealed partial class UpdateViewModel : ObservableObject, IAsyncDisposab /// One pass of the loop. /// + /// /// Quiet by construction. A pass that finds nothing writes nothing, and a pass that cannot reach /// the forge writes nothing either — an unreachable release page is a laptop on a train, it is not news, /// and it heals itself in six hours. The same discipline as VaultViewModel.AutoSyncAsync: what /// nobody asked for may only speak when it has something to say. + /// + /// + /// ◆ A pass still runs when one is already fetched, and this used to be the one state that stopped it + /// for good. The guard read Downloading or Ready, on the reasonable-looking argument that + /// there is no point fetching what has already been fetched. What it actually did was stop asking + /// — so the moment anything was downloaded, the banner was pinned to that build for the rest of the + /// process's life, however many newer ones were published afterwards. On the nightly channel, where the + /// release is deleted and remade on every push to main, that is most of the time: the client offered the + /// build it happened to see first, installed it, restarted, and immediately offered the next one. Two + /// updates and two restarts to move one step. + /// + /// + /// Not fetching what is already fetched is still the rule, and is where it now + /// lives — by comparing versions rather than by refusing to look. Downloading is still excluded, because + /// a fetch in flight must not be restarted underneath itself; Checking is excluded for the same reason, + /// and it is a pressed CHECK NOW that a tick would otherwise land on top of. + /// /// internal async Task CheckOnceAsync(CancellationToken cancellationToken) { - if (!IsAutomatic || State is UpdateState.Downloading or UpdateState.Ready) + if (!IsAutomatic || State is UpdateState.Checking or UpdateState.Downloading) { return; } @@ -339,27 +424,91 @@ internal sealed partial class UpdateViewModel : ObservableObject, IAsyncDisposab } } - /// Checks, and fetches whatever it finds. - /// The update found, or null. - private async Task FetchAsync(CancellationToken cancellationToken) + /// Checks, and fetches whatever it finds that is not already fetched. + /// Cancellation. + /// + /// A bound on the check alone, or null for none. See . Deliberately not a + /// bound on the whole method: a package legitimately takes minutes, and a caller that wants a short + /// answer wants it about the question, not about the download the answer may lead to. + /// + /// What is waiting to be installed once this has run, or null if nothing is. + /// + /// + /// ◆ It answers with what is pending rather than with what the check returned, and the difference is + /// what makes a second pass safe. A client running 1.0 that has fetched 1.1 is told 1.1 by every + /// later check, because 1.1 is still the newest thing on the feed — so "the check found something" is + /// not the same question as "there is something new". Comparing the two is what lets the loop keep + /// asking without fetching the same package every six hours, and it is what replaces the pending build + /// when the feed has moved on underneath it. + /// + /// + /// A build already fetched is never thrown away, including when a check answers null. Null means the + /// feed's newest is no newer than the build that is running, which says nothing about the one + /// on disk; that one is still newer than what is running, and withdrawing an offer somebody has already + /// been shown would be the worse answer. + /// + /// + private async Task FetchAsync( + CancellationToken cancellationToken, + TimeSpan? checkWithin = null) { State = UpdateState.Checking; Status = string.Empty; - var found = await updates.CheckAsync(cancellationToken).ConfigureAwait(true); + var found = await AskAsync(checkWithin, cancellationToken).ConfigureAwait(true); LastChecked = clock.GetUtcNow(); if (found is null) { - State = UpdateState.Idle; + // Answered with what is still pending rather than with nothing, which is what keeps a pressed + // CHECK NOW honest: "you are on the latest build" is true of the build that is running and false + // of the machine, if there is one downloaded and waiting to be it. + State = ready is null ? UpdateState.Idle : UpdateState.Ready; - return null; + if (ready is { } waiting) + { + Status = Waiting(waiting); + } + + return ready; } + if (ready is { } already + && string.Equals(already.Version, found.Version, StringComparison.Ordinal)) + { + // The ordinary answer for every pass after the first one that found something. Not fetched + // again, which is the rule the old Ready guard was reaching for and got by not asking at all. + State = UpdateState.Ready; + Status = Waiting(already); + + return already; + } + + return await FetchIntoReadyAsync(found, cancellationToken).ConfigureAwait(true); + } + + /// Downloads one update and offers it. + /// + /// Split from for length rather than for reuse — it has one caller — and the + /// division is a real one: above is which build to fetch, below is fetching it. + /// + private async Task FetchIntoReadyAsync( + AvailableUpdate found, + CancellationToken cancellationToken) + { + var superseded = ready; + State = UpdateState.Downloading; DownloadPercent = 0; + // Said while it downloads, and only when this displaces something already fetched. Both places that + // shows are places somebody is looking: a banner whose version changes under them, and a RESTART NOW + // that starts a download instead of a restart. + Status = superseded is { } stale + ? $"DodoSSH {found.Version} appeared while {stale.Version} was waiting. Fetching it instead." + : string.Empty; + // Monotonic, and it has to be. Progress delivers its callbacks by posting them to the captured // synchronisation context rather than invoking them inline, so a report can arrive after the // download has already returned — and an unguarded assignment then puts a stale smaller number @@ -382,13 +531,43 @@ internal sealed partial class UpdateViewModel : ObservableObject, IAsyncDisposab DownloadPercent = 100; ready = found; ReadyVersion = found.Version; + + // Undismissed, because this is a different build from the one that was dismissed. LATER was an + // answer about a version, and there is a new question. IsBannerDismissed = false; State = UpdateState.Ready; - Status = $"DodoSSH {found.Version} is downloaded and will run after a restart."; + Status = Waiting(found); return found; } + /// Asks the channel, giving up after if one was given. + /// + /// + /// The wait is bounded rather than the request cancelled, and that is not the same thing: the desktop + /// channel's own remark records that Velopack's check takes no token, so cancelling would be observed + /// only once the call had already returned. Waiting on it with a timeout is what actually gives an + /// answer back in ten seconds. + /// + /// + /// Which leaves the request running, and it is the reason the channel had to learn to remember what it + /// downloaded rather than only what it last found: an orphaned check can land afterwards and + /// move the channel's idea of the newest build, and the restart that gave up on it is about to name the + /// older one. See VelopackUpdateChannel.Matched. + /// + /// + private async Task AskAsync(TimeSpan? within, CancellationToken cancellationToken) + { + var asking = updates.CheckAsync(cancellationToken); + + return within is { } bound + ? await asking.WaitAsync(bound, clock, cancellationToken).ConfigureAwait(true) + : await asking.ConfigureAwait(true); + } + + private static string Waiting(AvailableUpdate update) => + $"DodoSSH {update.Version} is downloaded and will run after a restart."; + /// /// /// Hands the update to whoever was given the job at composition rather than applying it here, and the @@ -406,26 +585,89 @@ internal sealed partial class UpdateViewModel : ObservableObject, IAsyncDisposab [RelayCommand] private async Task RestartNowAsync() { - if (ready is not { } update) + if (ready is null) { return; } - // ◆ Reported rather than allowed to escape, and the phone is why. Where applying ends the process - // a throw here is nearly unreachable; where it does not — see ApplyingEndsTheProcess — the whole - // point is that this returns and the application carries on, so an exception has somewhere to go: - // out of a command handler, past the dispatcher, and into the process. What that looks like to - // somebody holding the phone is pressing INSTALL and having the application close, with nothing - // installed and nothing said. Failed and the message is the same answer CheckNowAsync gives, and - // it is the difference between a bug that describes itself and one that presents as a crash. + IsRestarting = true; + try { - await restart(update).ConfigureAwait(true); + await TakeOneLastLookAsync().ConfigureAwait(true); + + if (ready is not { } update) + { + return; + } + + // ◆ Reported rather than allowed to escape, and the phone is why. Where applying ends the + // process a throw here is nearly unreachable; where it does not — see ApplyingEndsTheProcess — + // the whole point is that this returns and the application carries on, so an exception has + // somewhere to go: out of a command handler, past the dispatcher, and into the process. What + // that looks like to somebody holding the phone is pressing INSTALL and having the application + // close, with nothing installed and nothing said. Failed and the message is the same answer + // CheckNowAsync gives, and it is the difference between a bug that describes itself and one + // that presents as a crash. + try + { + await restart(update).ConfigureAwait(true); + } + catch (Exception exception) when (exception is not OutOfMemoryException) + { + State = UpdateState.Failed; + Status = exception.Message; + } + } + finally + { + // Never reached on a desktop, where restart() replaces the process from inside the try. It is + // for the phone, where INSTALL is a request the platform may refuse, and for every failure + // path: a flag that keeps the banner up has to come down again. + IsRestarting = false; + } + } + + /// + /// Asks the feed once more, on the way to replacing this process. + /// + /// + /// + /// ◆ Because what was fetched can be hours old, and a restart is the one moment being one build + /// behind is expensive. The loop asks every six hours; a build published between the last pass and + /// this press would otherwise be installed on the next restart, which is to say: update, + /// restart, be told at once that another update is waiting, update, restart again. Two of everything to + /// move one step, which is exactly what a rolling nightly channel produces. + /// + /// + /// It costs one small request in the ordinary case, because the feed answers with the build already on + /// disk and compares rather than downloads. It costs a download only when there + /// genuinely is something newer, which is the case where the alternative was two downloads anyway. + /// + /// + /// Nothing here refuses a restart. A forge that cannot be reached, a look that ran out of time, a + /// fetch of the newer build that failed halfway — all of them leave the build already on disk untouched + /// and still newer than the one running, so the restart goes ahead with it. That is why this swallows + /// rather than reporting: is for something a person is waiting on an + /// answer to, and this person is waiting on a restart they are about to get. + /// + /// + private async Task TakeOneLastLookAsync() + { + try + { + await FetchAsync(lifetime.Token, checkWithin: LastLookTimeout).ConfigureAwait(true); + } + catch (OperationCanceledException) when (lifetime.IsCancellationRequested) + { + // Shutting down, which is the one case that must not go on to restart: whatever is fetched is + // applied at the next ordinary launch anyway, and that is the policy rather than a fallback. + ready = null; } catch (Exception exception) when (exception is not OutOfMemoryException) { - State = UpdateState.Failed; - Status = exception.Message; + // Including the bound running out. Back to the offer that was already on screen. + State = UpdateState.Ready; } } diff --git a/tests/DodoSSH.Client.App.Tests/UpdateFlowTests.cs b/tests/DodoSSH.Client.App.Tests/UpdateFlowTests.cs index cee898d..417a724 100644 --- a/tests/DodoSSH.Client.App.Tests/UpdateFlowTests.cs +++ b/tests/DodoSSH.Client.App.Tests/UpdateFlowTests.cs @@ -390,8 +390,14 @@ public sealed class UpdateFlowTests : IDisposable updates.State.ShouldBe(UpdateState.Ready); } + /// + /// Asks, and does not fetch again. The distinction is the whole of the fix for a client that installed + /// two updates to move one step: the old guard skipped the pass once anything was ready, so the + /// banner was pinned to whatever build the process happened to see first. Not re-fetching is still the + /// rule; it is enforced by comparing versions now rather than by refusing to look. + /// [Fact] - public async Task APassWhileOneIsAlreadyReady_DoesNotAskAgain() + public async Task APassWhileOneIsAlreadyReady_AsksAgainAndDoesNotFetchTwice() { channel.Available = new AvailableUpdate("1.3.0"); @@ -400,7 +406,181 @@ public sealed class UpdateFlowTests : IDisposable await updates.CheckOnceAsync(Token); await updates.CheckOnceAsync(Token); - channel.Checks.ShouldBe(1); + channel.Checks.ShouldBe(2); + channel.Downloads.ShouldBe(1, "the second pass found the build it had already fetched"); + updates.State.ShouldBe(UpdateState.Ready); + updates.ReadyVersion.ShouldBe("1.3.0"); + } + + /// + /// ◆ The regression this whole change is about, in the case the timer can reach. A rolling nightly + /// channel keeps exactly one build, so a client left running does not accumulate a queue of updates — it + /// is offered whichever one the feed holds, and the pending build has to be replaced when that moves on. + /// + [Fact] + public async Task APassWhileOneIsAlreadyReady_ReplacesItWhenTheFeedHasMovedOn() + { + channel.Available = new AvailableUpdate("1.3.0"); + + var updates = Build(); + + await updates.CheckOnceAsync(Token); + updates.ReadyVersion.ShouldBe("1.3.0"); + + channel.Available = new AvailableUpdate("1.4.0"); + + await updates.CheckOnceAsync(Token); + + channel.Downloads.ShouldBe(2); + updates.State.ShouldBe(UpdateState.Ready); + updates.ReadyVersion.ShouldBe("1.4.0"); + } + + /// + /// A build superseded after LATER was pressed brings the banner back, because LATER was an answer about + /// a version and this is a different one. + /// + [Fact] + public async Task ASupersededOffer_IsShownAgainEvenIfTheOldOneWasDismissed() + { + channel.Available = new AvailableUpdate("1.3.0"); + + var updates = Build(); + + await updates.CheckOnceAsync(Token); + updates.DismissBannerCommand.Execute(null); + updates.IsBannerShowing.ShouldBeFalse(); + + channel.Available = new AvailableUpdate("1.4.0"); + await updates.CheckOnceAsync(Token); + + updates.IsBannerShowing.ShouldBeTrue(); + updates.ReadyHeadline.ShouldContain("1.4.0"); + } + + /// + /// ◆ The half the timer cannot reach, and the one a person actually hit: a build published between the + /// last pass and the press. Without this the press installs a build that was already superseded, and the + /// next launch offers the newer one straight away — two downloads, two installs and two restarts to move + /// one step. + /// + [Fact] + public async Task Restarting_TakesOneLastLookAndInstallsWhatItFinds() + { + channel.Available = new AvailableUpdate("1.3.0"); + + var updates = Build(); + + await updates.CheckOnceAsync(Token); + + // Published while the banner was up, which on a rolling channel replaces the release rather than + // adding to it — so the feed now holds only this one. + channel.Available = new AvailableUpdate("1.4.0"); + + await updates.RestartNowCommand.ExecuteAsync(null); + + channel.Restarts.ShouldBe(1); + channel.RestartedWith!.Version.ShouldBe("1.4.0"); + restarts.ShouldBe(1, "one restart, not one per build"); + } + + /// + /// The ordinary case, and what it costs: one question and no download. A press that re-fetched what was + /// already on disk would have made the fix worse than the bug. + /// + [Fact] + public async Task Restarting_AsksOnceMoreAndFetchesNothingWhenNothingIsNewer() + { + channel.Available = new AvailableUpdate("1.3.0"); + + var updates = Build(); + + await updates.CheckOnceAsync(Token); + await updates.RestartNowCommand.ExecuteAsync(null); + + channel.Checks.ShouldBe(2); + channel.Downloads.ShouldBe(1); + channel.RestartedWith!.Version.ShouldBe("1.3.0"); + } + + /// + /// The look is a courtesy and never a gate. Somebody pressing RESTART NOW on a train has a build on disk + /// that is newer than the one running, and a forge that cannot be reached must not stand between them. + /// + [Fact] + public async Task RestartingWhenTheLastLookFails_StillInstallsWhatWasAlreadyFetched() + { + channel.Available = new AvailableUpdate("1.3.0"); + + var updates = Build(); + + await updates.CheckOnceAsync(Token); + + channel.CheckFailure = new HttpRequestException("no such host"); + + await updates.RestartNowCommand.ExecuteAsync(null); + + channel.Restarts.ShouldBe(1); + channel.RestartedWith!.Version.ShouldBe("1.3.0"); + updates.State.ShouldNotBe(UpdateState.Failed, "nothing the user asked for failed"); + } + + /// + /// + /// The same rule one step further in: the newer build was found and then could not be fetched. What is + /// on disk is untouched, so that is what gets installed. + /// + [Fact] + public async Task RestartingWhenTheNewerFetchFails_StillInstallsWhatWasAlreadyFetched() + { + channel.Available = new AvailableUpdate("1.3.0"); + + var updates = Build(); + + await updates.CheckOnceAsync(Token); + + channel.Available = new AvailableUpdate("1.4.0"); + channel.DownloadFailure = new HttpRequestException("the connection was reset"); + + await updates.RestartNowCommand.ExecuteAsync(null); + + channel.Restarts.ShouldBe(1); + channel.RestartedWith!.Version.ShouldBe("1.3.0"); + } + + /// + /// Everything the banner draws keys off IsReady, and the last look moves the state off it — so + /// without IsRestarting the strip vanished the moment RESTART NOW was pressed and left nothing on + /// screen saying why, for as long as a package takes to fetch. + /// + [Fact] + public async Task TheBannerStaysUpWhileARestartFetchesANewerBuild() + { + channel.Available = new AvailableUpdate("1.3.0"); + + var updates = Build(); + + await updates.CheckOnceAsync(Token); + + channel.Available = new AvailableUpdate("1.4.0"); + channel.HoldDownload = new TaskCompletionSource(); + + var pressed = updates.RestartNowCommand.ExecuteAsync(null); + + // The gate is what makes the intermediate state observable; without it the fetch is over before + // anything could look at it. + while (updates.State is not UpdateState.Downloading) + { + await Task.Yield(); + } + + updates.IsBannerShowing.ShouldBeTrue(); + updates.ReadyHeadline.ShouldContain("before restarting"); + + channel.HoldDownload.SetResult(); + await pressed; + + channel.RestartedWith!.Version.ShouldBe("1.4.0"); } ///