From 524bdbbe9023f8c38d5669c4547e52b0b2f9800d Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 17 Aug 2026 03:48:12 -0400 Subject: [PATCH] test(siteruntime): gate ReconcileStartsInstanceTests on the DM's first instance-count report (register row 83) Row 67's suite still opened every test with a bare Task.Delay(500) after constructing the Deployment Manager. Each test then writes its deployed_configurations row -- and a row that lands before the boot-time load reads storage is picked up by that load, which means the test passes while proving the BOOT LOAD works rather than the startup-reconcile path it names. A sleep only makes that race unlikely; on a loaded machine the load can still be in flight when it expires and the test silently changes what it proves instead of failing. The row-78 fix session hit exactly this trap and closed it in InstanceSweepSelfHealTests by gating on the first ISiteHealthCollector.SetInstanceCounts report -- the edge the manager raises unconditionally the moment its load completes, including for an empty site. This applies the same gate here. The probe is now a shared TestSupport helper (DeploymentManagerStartupProbe, added with row 85) rather than a private nested class, so both suites use one implementation. Also pins InstanceSweepIntervalSeconds = 0 for this suite: with the sweep armed its periodic self-heal would independently start the same instances and the suite would stop distinguishing the row-67 mechanism from the row-78 one. The remaining Task.Delay calls in the file are deliberate and left alone -- they precede assertions that NOTHING happened (a disabled instance stayed stopped, a redundant ensure-started changed nothing), which cannot be awaited on. The one delay that was a readiness wait rather than an absence wait is now an AwaitAssertAsync on the instance actually being routable. --- .../Actors/ReconcileStartsInstanceTests.cs | 50 +++++++++++++++---- 1 file changed, 39 insertions(+), 11 deletions(-) diff --git a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/ReconcileStartsInstanceTests.cs b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/ReconcileStartsInstanceTests.cs index 13788bd7..d9cd34bc 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/ReconcileStartsInstanceTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/ReconcileStartsInstanceTests.cs @@ -68,20 +68,49 @@ public class ReconcileStartsInstanceTests : TestKit, IDisposable TestLocalDb.DeleteFiles(path); } - private IActorRef CreateDeploymentManager(IServiceProvider? serviceProvider = null) => - ActorOf(Props.Create(() => new DeploymentManagerActor( + /// + /// Starts a Deployment Manager and blocks until its boot-time config load has PROVABLY read + /// storage — its first ISiteHealthCollector.SetInstanceCounts report. + /// + /// + /// Deferred register row 83. This used to be a bare Task.Delay(500). Every test + /// here writes its deployed_configurations row AFTER the manager is constructed, and a + /// row that lands before the boot load reads storage is picked up by that load — so the test + /// would still pass, while proving the BOOT LOAD works rather than the startup-reconcile path + /// it claims to cover. A sleep only makes that race unlikely; on a loaded machine the load can + /// still be in flight when it expires, and the test silently changes what it proves instead of + /// failing. The row-78 fix session hit exactly this trap and closed it in + /// InstanceSweepSelfHealTests; this is the same gate, applied to row 67's suite. + /// + /// + /// + /// The sweep is disabled (InstanceSweepIntervalSeconds = 0) so these tests keep proving + /// the row-67 path specifically — with the sweep + /// armed, its periodic self-heal would independently start the same instances and the suite + /// would no longer distinguish the two mechanisms. + /// + /// + private IActorRef CreateDeploymentManager(IServiceProvider? serviceProvider = null) + { + var health = new DeploymentManagerStartupProbe(); + var dm = ActorOf(Props.Create(() => new DeploymentManagerActor( _storage, _compilationService, _sharedScriptLibrary, null, - new SiteRuntimeOptions(), + new SiteRuntimeOptions { InstanceSweepIntervalSeconds = 0 }, NullLogger.Instance, dclManager: null, - healthCollector: null, + healthCollector: health, serviceProvider: serviceProvider, loggerFactory: null, configFetcher: null))); + AwaitCondition(() => health.ReportCount > 0, TimeSpan.FromSeconds(20), + TimeSpan.FromMilliseconds(25)); + return dm; + } + private IActorRef CreateReconciliationActor( IActorRef siteCommunicationActor, IDeploymentConfigFetcher fetcher, @@ -143,7 +172,6 @@ public class ReconcileStartsInstanceTests : TestKit, IDisposable // Boot the Deployment Manager against EMPTY storage — this is the missing-config boot // (lost volume / first boot after a restore / rehomed data dir). var dm = CreateDeploymentManager(); - await Task.Delay(500); // Baseline: the instance is genuinely not running. Assert.False(await InstanceIsRunningAsync(dm, "HealedPump")); @@ -188,7 +216,6 @@ public class ReconcileStartsInstanceTests : TestKit, IDisposable { var siteLog = new FakeSiteEventLogger(); var dm = CreateDeploymentManager(new SingleServiceProvider(siteLog)); - await Task.Delay(500); const string instanceName = "SurfacedPump"; await _storage.StoreDeployedConfigAsync( @@ -286,18 +313,21 @@ public class ReconcileStartsInstanceTests : TestKit, IDisposable public async Task EnsureInstanceStarted_IsIdempotent_WhenAlreadyRunning() { var dm = CreateDeploymentManager(); - await Task.Delay(500); const string instanceName = "IdempotentPump"; dm.Tell(new DeployInstanceCommand( "dep-idem", instanceName, "sha256:abc", MakeConfigJson(instanceName), "admin", DateTimeOffset.UtcNow)); ExpectMsg(TimeSpan.FromSeconds(5)); - await Task.Delay(1000); + + // Wait on the instance actually being routable, not on the clock. + await AwaitAssertAsync(async () => + Assert.True(await InstanceIsRunningAsync(dm, instanceName)), TimeSpan.FromSeconds(10)); var before = await ResolveInstanceChildAsync(dm, instanceName); - // Two redundant ensure-started requests, as the two nodes would send. + // Two redundant ensure-started requests, as the two nodes would send. This one IS a + // settle wait: the assertion below is that nothing happened, which cannot be awaited on. dm.Tell(new EnsureInstanceStartedCommand(instanceName, "startup reconcile")); dm.Tell(new EnsureInstanceStartedCommand(instanceName, "startup reconcile")); await Task.Delay(1000); @@ -317,7 +347,6 @@ public class ReconcileStartsInstanceTests : TestKit, IDisposable public async Task EnsureInstanceStarted_DisabledInstance_StaysStopped() { var dm = CreateDeploymentManager(); - await Task.Delay(500); const string instanceName = "DisabledPump"; await _storage.StoreDeployedConfigAsync( @@ -338,7 +367,6 @@ public class ReconcileStartsInstanceTests : TestKit, IDisposable public async Task EnsureInstanceStarted_UnknownInstance_IsAQuietNoOp() { var dm = CreateDeploymentManager(); - await Task.Delay(500); dm.Tell(new EnsureInstanceStartedCommand("NeverHeardOf", "startup reconcile")); await Task.Delay(1500);