From b6875591f04bc442a01146ee915eaa9e48e3187e Mon Sep 17 00:00:00 2001 From: Allamagoosa <36551512+dmatlock171@users.noreply.github.com> Date: Fri, 21 Aug 2026 07:18:33 -0700 Subject: [PATCH] Stop the dispatch tests measuring the CI runner instead of the loop EffectiveConcurrentDownloads clamps the setting by Environment.ProcessorCount, so a test asking for three books at once starts only two on a small machine, waits out its ten second patience and fails. Reproduced with DOTNET_PROCESSOR_COUNT=2: three failures and a thirty-four second run, against five seconds and none on a sixteen core box. The CI matrix includes runners small enough to hit it. Machine capability is now overridable, and the dispatch tests pin it to the concurrency they are asking about. The clamp itself is unchanged in the app. Two tests for the seam while it is there, since neither the point-of-use clamp nor the hint had any coverage: that a machine smaller than the setting holds the loop down without rewriting what the user asked for, and that ConcurrencyHint says so and falls silent when the machine can keep up. --- .../ProcessQueue/ProcessQueueViewModel.cs | 12 ++++- .../ProcessQueueDispatchTests.cs | 44 ++++++++++++++++++- 2 files changed, 54 insertions(+), 2 deletions(-) diff --git a/Source/LibationUiBase/ProcessQueue/ProcessQueueViewModel.cs b/Source/LibationUiBase/ProcessQueue/ProcessQueueViewModel.cs index e6104ab5..3a25b428 100644 --- a/Source/LibationUiBase/ProcessQueue/ProcessQueueViewModel.cs +++ b/Source/LibationUiBase/ProcessQueue/ProcessQueueViewModel.cs @@ -88,12 +88,22 @@ public class ProcessQueueViewModel : ReactiveObject } private int _maxConcurrentDownloads; + /// + /// What this machine can usefully manage. Overridable so that tests of the dispatch loop pin it + /// instead of inheriting the host's core count - otherwise a test asking for three books at once + /// passes on a developer's machine and fails on a two-core CI runner, having tested the runner + /// rather than the loop. + /// + internal int? MachineCeilingOverride { get; set; } + + private int MachineCeiling => MachineCeilingOverride ?? Configuration.MaxAllowedConcurrentDownloads; + /// /// How many books actually run at once: what the user asked for, held down to what this machine /// can usefully manage. Applied here, at the point of use, so the stored setting is left alone. /// private int EffectiveConcurrentDownloads - => Math.Clamp(MaxConcurrentDownloads, Configuration.MinConcurrentDownloads, Configuration.MaxAllowedConcurrentDownloads); + => Math.Clamp(MaxConcurrentDownloads, Configuration.MinConcurrentDownloads, MachineCeiling); public bool AutoScrollQueue { get => field; set { RaiseAndSetIfChanged(ref field, value); Configuration.Instance.AutoScrollQueue = value; } } /// Exposed so UI controls can bind their spinner bounds rather than hardcoding them. diff --git a/Source/_Tests/LibationUiBase.Tests/ProcessQueueDispatchTests.cs b/Source/_Tests/LibationUiBase.Tests/ProcessQueueDispatchTests.cs index 09a72a0e..d12a6296 100644 --- a/Source/_Tests/LibationUiBase.Tests/ProcessQueueDispatchTests.cs +++ b/Source/_Tests/LibationUiBase.Tests/ProcessQueueDispatchTests.cs @@ -118,10 +118,16 @@ public class ProcessQueueDispatchTests } } + /// + /// The setting under test. Machine capability is pinned to the same number, because the loop + /// clamps the setting by it: left to , a test asking for + /// three books at once would quietly start two on a small CI runner, wait out its patience and + /// fail - having measured the runner rather than the loop. + /// private static (ProcessQueueViewModel Queue, FakeBooks Books) NewQueue(int atOnce) { var books = new FakeBooks(); - var queue = new ProcessQueueViewModel { MaxConcurrentDownloads = atOnce }; + var queue = new ProcessQueueViewModel { MaxConcurrentDownloads = atOnce, MachineCeilingOverride = atOnce }; queue.ProcessBookHandler = books.Handle; return (queue, books); } @@ -304,4 +310,40 @@ public class ProcessQueueDispatchTests Assert.AreEqual(0, queue.QueuedCount, "Cancel All left books queued."); Assert.AreEqual(2, books.Started.Count, "Cancel All did not stop new books from starting."); } + + [TestMethod] + public async Task a_machine_smaller_than_the_setting_holds_the_loop_down_without_changing_the_setting() + { + var books = new FakeBooks(); + var queue = new ProcessQueueViewModel { MaxConcurrentDownloads = 8, MachineCeilingOverride = 2 }; + queue.ProcessBookHandler = books.Handle; + + queue.AddToQueue([Book("A"), Book("B"), Book("C"), Book("D")]); + + await books.WaitForStarted(2); + await Task.Delay(100); + Assert.AreEqual(2, books.Started.Count, "The machine ceiling did not hold the loop down."); + + // The whole point of clamping here rather than on the way in: what the user asked for + // survives being opened on a machine that cannot deliver it. + Assert.AreEqual(8, queue.MaxConcurrentDownloads, "The stored setting was rewritten to what the machine could manage."); + + books.FinishAll("A", "B"); + await books.WaitForStarted(4); + books.FinishAll("C", "D"); + + await RunToCompletion(queue); + Assert.AreEqual(2, books.HighWaterMark, "More books ran at once than the machine allows."); + } + + [TestMethod] + public void the_hint_says_what_the_machine_will_do_and_is_silent_when_it_can_keep_up() + { + var queue = new ProcessQueueViewModel { MaxConcurrentDownloads = 8, MachineCeilingOverride = 2 }; + Assert.AreEqual("(2 on this machine)", queue.ConcurrencyHint); + + // Nothing to say once the machine can deliver what was asked for. + queue.MachineCeilingOverride = 10; + Assert.IsNull(queue.ConcurrencyHint); + } }