Make Refresh Build Data force the next poll instead of delaying it

BuildMonitor.Test/BuildSyncRefreshTests.cs

@@ -0,0 +1,99 @@
+// Copyright (c) 2023-2026 ktsu-dev contributors
+
+namespace ktsu.BuildMonitor.Test;
+
+using ktsu.Semantics.Strings;
+
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+
+/// <summary>
+/// Covers "Refresh Build Data" and the refresh after a re-run, cancel or trigger. It used to restart
+/// the build's timer, which pushed the next poll a full interval away instead of forcing it
+/// (ktsu-dev/BuildMonitor#303).
+/// </summary>
+[TestClass]
+public sealed class BuildSyncRefreshTests
+{
+	/// <summary>
+	/// A provider that records the builds it was asked to update.
+	/// </summary>
+	private sealed class RecordingProvider : BuildProvider
+	{
+		internal override BuildProviderName Name { get; } = "Recording".As<BuildProviderName>();
+
+		internal int BuildUpdates { get; private set; }
+
+		internal override Task UpdateRepositoriesAsync(Owner owner) => Task.CompletedTask;
+		internal override Task UpdateBuildsAsync(Repository repository) => Task.CompletedTask;
+		internal override Task UpdateRunAsync(Run run) => Task.CompletedTask;
+
+		internal override Task UpdateBuildAsync(Build build)
+		{
+			BuildUpdates++;
+			return Task.CompletedTask;
+		}
+	}
+
+	private static BuildSync CreateSync(RecordingProvider provider)
+	{
+		Owner owner = provider.CreateOwner("owner".As<OwnerName>());
+		Repository repository = owner.CreateRepository("repo".As<RepositoryName>());
+		Build build = repository.CreateBuild("build".As<BuildName>());
+		Assert.IsTrue(repository.Builds.TryAdd(build.Id, build));
+		return new() { Build = build };
+	}
+
+	[TestMethod]
+	public void ForceUpdateMakesTheBuildDueOnTheNextTick()
+	{
+		// Arrange
+		BuildSync sync = CreateSync(new());
+		Assert.IsFalse(sync.ShouldUpdate, "A new low-priority build should be a full interval from its first poll");
+
+		// Act
+		sync.ForceUpdate();
+
+		// Assert
+		Assert.IsTrue(sync.ShouldUpdate, "A refresh should make the build due now, not a full interval later");
+		Assert.AreEqual(TimeSpan.Zero, sync.TimeRemaining);
+		Assert.AreEqual(1, sync.UpdateProgress, "The progress bar should show the build as due");
+	}
+
+	[TestMethod]
+	public void RefreshBuildDataForcesTheTrackedBuildsUpdate()
+	{
+		// Arrange
+		BuildSync sync = CreateSync(new());
+		Assert.IsTrue(BuildMonitor.BuildSyncCollection.TryAdd(sync.Build.Id, sync));
+
+		try
+		{
+			// Act
+			BuildMonitor.RefreshBuildData(sync.Build);
+
+			// Assert
+			Assert.IsTrue(sync.ShouldUpdate, "Refresh Build Data should queue the build for its next tick");
+		}
+		finally
+		{
+			_ = BuildMonitor.BuildSyncCollection.TryRemove(sync.Build.Id, out _);
+		}
+	}
+
+	[TestMethod]
+	public async Task NormalPacingResumesAfterTheForcedUpdate()
+	{
+		// Arrange
+		RecordingProvider provider = new();
+		BuildSync sync = CreateSync(provider);
+		sync.ForceUpdate();
+
+		// Act
+		await sync.UpdateAsync().ConfigureAwait(false);
+
+		// Assert
+		Assert.AreEqual(1, provider.BuildUpdates);
+		Assert.IsFalse(sync.ShouldUpdate, "Once the forced poll has run, the build should wait out its interval again");
+		Assert.IsGreaterThan(TimeSpan.FromSeconds(100), sync.TimeRemaining);
+	}
+}

BuildMonitor/BuildMonitor.cs

@@ -28,7 +28,7 @@
 
 	internal static object SyncLock { get; } = new();
 
-	private static ConcurrentDictionary<BuildId, BuildSync> BuildSyncCollection { get; } = [];
+	internal static ConcurrentDictionary<BuildId, BuildSync> BuildSyncCollection { get; } = [];
 	internal static ConcurrentDictionary<RunId, RunSync> RunSyncCollection { get; } = [];
 
 	private static Task UpdateTask { get; set; } = Task.CompletedTask;
@@ -186,7 +186,7 @@
 	/// fields actually occupy tests the bug itself, so a fixed binding is recognised as fixed and
 	/// anything else is refused rather than guessed at.
 	/// </remarks>
 	private static unsafe int? ProbeNativeImGuiTableColumnSize()
 	{
 		int csharpSize = sizeof(ImGuiTableColumn);
 		int widthGivenOffset;
@@ -285,7 +285,7 @@
 		return narrowFieldBytes == NarrowlyBoundColumnFields.Length * 2 ? csharpSize : null;
 	}
 
 	private static unsafe void SaveColumnWidth(string columnName, int columnIndex)
 	{
 		// Null means the layout probe rejected the binding, so there is no address we can trust.
 		if (NativeImGuiTableColumnSize is not int nativeStructSize)
@@ -412,7 +412,7 @@
 		SaveSettingsIfRequired();
 	}
 
 	private static void UpdateOwnerTabs()
 	{
 		// Build expected tabs based on provider type:
 		// - GitHub: one tab per owner (org/user)
@@ -421,7 +421,7 @@
 		HashSet<string> expectedTabIds = [AllOwnersTabId, LogsTabId];
 		Dictionary<string, (string Label, Action Content)> tabsToCreate = [];
 
 		foreach ((BuildProviderName providerName, BuildProvider provider) in AppData.BuildProviders)
 		{
 			if (provider is GitHub)
 			{
@@ -628,7 +628,7 @@
 		return displayName;
 	}
 
 	private static string MakeRepositoryDisplayName(Build build)
 	{
 		string repoName = build.Repository.Name;
 		string ownerName = build.Owner.Name;
@@ -1263,12 +1263,12 @@
 
 	private static string GetRunUrl(Run run) => $"https://github.com/{run.Owner.Name}/{run.Repository.Name}/actions/runs/{run.Id}";
 
-	private static void RefreshBuildData(Build build)
+	internal static void RefreshBuildData(Build build)
 	{
 		// Queue the build for immediate update
 		if (BuildSyncCollection.TryGetValue(build.Id, out BuildSync? buildSync))
 		{
-			buildSync.ResetTimer();
+			buildSync.ForceUpdate();
 		}
 	}
 

BuildMonitor/BuildSync.cs

@@ -27,6 +27,11 @@ internal sealed class BuildSync
 	private const int UpdateIntervalMedium = 60;
 	private const int UpdateIntervalLow = 120;
 
+	/// <summary>
+	/// Set by <see cref="ForceUpdate"/> to poll the build on the next tick regardless of its interval.
+	/// </summary>
+	private volatile bool forceUpdate;
+
 	/// <summary>
 	/// Returns true if the build no longer exists in its parent repository's builds collection.
 	/// This happens when the build/workflow is deleted or the repository is removed.
@@ -68,19 +73,28 @@ internal RequestPriority Priority
 		_ => UpdateIntervalLow,
 	};
 
-	internal TimeSpan TimeRemaining => TimeSpan.FromSeconds(
-		Math.Max(0, UpdateInterval - UpdateTimer.Elapsed.TotalSeconds));
+	internal TimeSpan TimeRemaining => forceUpdate
+		? TimeSpan.Zero
+		: TimeSpan.FromSeconds(Math.Max(0, UpdateInterval - UpdateTimer.Elapsed.TotalSeconds));
 
-	internal double UpdateProgress => Math.Clamp(UpdateTimer.Elapsed.TotalSeconds / UpdateInterval, 0, 1);
+	internal double UpdateProgress => forceUpdate ? 1 : Math.Clamp(UpdateTimer.Elapsed.TotalSeconds / UpdateInterval, 0, 1);
 
-	internal bool ShouldUpdate => !IsOrphaned && UpdateTimer.Elapsed.TotalSeconds >= UpdateInterval;
+	internal bool ShouldUpdate => !IsOrphaned && (forceUpdate || UpdateTimer.Elapsed.TotalSeconds >= UpdateInterval);
 
 	internal BuildSync() => UpdateTimer.Start();
 
-	internal void ResetTimer() => UpdateTimer.Restart();
+	/// <summary>
+	/// Queues the build to be polled on the next tick, however much of its interval is left. Restarting
+	/// the timer instead pushed the next poll a full interval away (ktsu-dev/BuildMonitor#303).
+	/// </summary>
+	internal void ForceUpdate() => forceUpdate = true;
 
 	internal async Task UpdateAsync()
 	{
+		// Clear the request before polling, so a refresh asked for while this update is in flight still
+		// gets its own poll afterwards.
+		forceUpdate = false;
+
 		// Restart the timer whether or not the update succeeds. A failed update that left it running
 		// kept ShouldUpdate true, so the build was polled again back to back (ktsu-dev/BuildMonitor#299).
 		try