From bb5603b7ec978667a4e79a3cd2cde3da512f2c67 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 18 May 2026 23:14:47 -0400 Subject: [PATCH] Fix flaky GalaxyHierarchyRefreshServiceTests timing race ExecuteAsync_WhenFirstRefreshThrowsNonCancellationException_DoesNotFault BackgroundService cancelled the service immediately after StartAsync, so under parallel load the first RefreshAsync could be skipped (RefreshCallCount 0) and `await executeTask` rethrew TaskCanceledException before the IsFaulted assertion. The test now waits for a TaskCompletionSource signal that the first refresh was attempted before cancelling, and uses Task.WhenAny so a Canceled ExecuteTask does not rethrow. Confirmed stable across full-suite runs (408/408). Co-Authored-By: Claude Opus 4.7 (1M context) --- .../GalaxyHierarchyRefreshServiceTests.cs | 23 ++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/src/MxGateway.Tests/Galaxy/GalaxyHierarchyRefreshServiceTests.cs b/src/MxGateway.Tests/Galaxy/GalaxyHierarchyRefreshServiceTests.cs index f0759f2..bd23b37 100644 --- a/src/MxGateway.Tests/Galaxy/GalaxyHierarchyRefreshServiceTests.cs +++ b/src/MxGateway.Tests/Galaxy/GalaxyHierarchyRefreshServiceTests.cs @@ -22,13 +22,23 @@ public sealed class GalaxyHierarchyRefreshServiceTests using CancellationTokenSource cts = new(); await service.StartAsync(cts.Token); + + // Wait until the first RefreshAsync has actually been attempted (and + // thrown) before cancelling, so cancellation cannot race ahead of the + // first-load path under test — this is what made the test flaky under + // parallel load. + await cache.FirstRefreshAttempted.WaitAsync(TimeSpan.FromSeconds(10)); + await cts.CancelAsync(); - // The background loop must have stopped cleanly: ExecuteTask completes - // (RanToCompletion or Canceled) rather than faulting on the first refresh. + // The background loop must have stopped cleanly: ExecuteTask reaches a + // terminal state that is not Faulted (RanToCompletion or Canceled) + // rather than faulting on the first refresh. WhenAny is used so a + // Canceled task does not rethrow before the IsFaulted assertion. Task? executeTask = service.ExecuteTask; Assert.NotNull(executeTask); - await executeTask; + Task completed = await Task.WhenAny(executeTask, Task.Delay(TimeSpan.FromSeconds(10))); + Assert.Same(executeTask, completed); Assert.False(executeTask.IsFaulted); Assert.Equal(1, cache.RefreshCallCount); @@ -49,13 +59,20 @@ public sealed class GalaxyHierarchyRefreshServiceTests private sealed class ThrowingCache(Exception toThrow) : IGalaxyHierarchyCache { + private readonly TaskCompletionSource firstRefreshAttempted = + new(TaskCreationOptions.RunContinuationsAsynchronously); + public int RefreshCallCount { get; private set; } + /// Completes once has been invoked at least once. + public Task FirstRefreshAttempted => firstRefreshAttempted.Task; + public GalaxyHierarchyCacheEntry Current => GalaxyHierarchyCacheEntry.Empty; public Task RefreshAsync(CancellationToken cancellationToken) { RefreshCallCount++; + firstRefreshAttempted.TrySetResult(); throw toThrow; }