diff --git a/BuildMonitor.Test/BuildSyncRefreshTests.cs b/BuildMonitor.Test/BuildSyncRefreshTests.cs new file mode 100644 index 0000000..c62f04c --- /dev/null +++ b/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; + +/// +/// 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). +/// +[TestClass] +public sealed class BuildSyncRefreshTests +{ + /// + /// A provider that records the builds it was asked to update. + /// + private sealed class RecordingProvider : BuildProvider + { + internal override BuildProviderName Name { get; } = "Recording".As(); + + 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()); + Repository repository = owner.CreateRepository("repo".As()); + Build build = repository.CreateBuild("build".As()); + 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); + } +} diff --git a/BuildMonitor/BuildMonitor.cs b/BuildMonitor/BuildMonitor.cs index 1431aa2..34b2fd7 100644 --- a/BuildMonitor/BuildMonitor.cs +++ b/BuildMonitor/BuildMonitor.cs @@ -28,7 +28,7 @@ internal static class BuildMonitor internal static object SyncLock { get; } = new(); - private static ConcurrentDictionary BuildSyncCollection { get; } = []; + internal static ConcurrentDictionary BuildSyncCollection { get; } = []; internal static ConcurrentDictionary RunSyncCollection { get; } = []; private static Task UpdateTask { get; set; } = Task.CompletedTask; @@ -1263,12 +1263,12 @@ private static void CopyRunUrl(Run run) 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(); } } diff --git a/BuildMonitor/BuildSync.cs b/BuildMonitor/BuildSync.cs index 5a0d8a2..b079b82 100644 --- a/BuildMonitor/BuildSync.cs +++ b/BuildMonitor/BuildSync.cs @@ -27,6 +27,11 @@ internal sealed class BuildSync private const int UpdateIntervalMedium = 60; private const int UpdateIntervalLow = 120; + /// + /// Set by to poll the build on the next tick regardless of its interval. + /// + private volatile bool forceUpdate; + /// /// 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(); + /// + /// 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). + /// + 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