test(scripted-alarms): wait for the sibling run to finish before asserting InFlightCount (#911)

ScriptRunGuard counts a run on StartedRunCount and InFlightCount together the
moment it is admitted, but releases InFlightCount only in the run's finally.
The runaway test waited on StartedRunCount >= 6 and then asserted
InFlightCount == 1, so the healthy sibling's 4th-push run - admitted but still
executing - was counted alongside the runaway orphan (2). Test race, not an
engine defect: widening only the healthy run reproduced it 5/5.

- wait until StartedRunCount == 6 AND InFlightCount == 1
- healthy predicate spins ~100 ms so the window is open on every run
- positive controls: the final push must be refused once more (the in-flight
  run is the orphan) and the quality-once absence check runs only after the
  healthy Activated event from the later pass has been delivered

Under 28 busy-loops on 14 cores: before 2/60 failed, after 0/60.
This commit is contained in:
Joseph Doherty
2026-11-13 10:46:19 -05:00
parent e299dc090a
commit f1da809f3c
@@ -45,9 +45,13 @@ public sealed class ScriptedAlarmEngineRunGuardTests
eng.OnEvent += (_, e) => { lock (events) events.Add(e); }; eng.OnEvent += (_, e) => { lock (events) events.Add(e); };
// The load itself evaluates once (startup recovery) — that is the first, abandoned run. // The load itself evaluates once (startup recovery) — that is the first, abandoned run.
// The healthy predicate deliberately spins ~100 ms (well inside the 500 ms budget) so every run of it
// is observably in flight for a while. That keeps the #911 window open on EVERY run of this test
// instead of only under CPU load: the guard counts a run on StartedRunCount AND InFlightCount the
// moment it is admitted, but releases InFlightCount only when the run finishes.
await eng.LoadAsync( await eng.LoadAsync(
[Alarm("runaway", $"var t = ctx.GetTag(\"Temp\"); {SpinPredicate}"), [Alarm("runaway", $"var t = ctx.GetTag(\"Temp\"); {SpinPredicate}"),
Alarm("healthy", """return (int)ctx.GetTag("Temp").Value > 100;""")], Alarm("healthy", $$"""{{HealthySpin}} return (int)ctx.GetTag("Temp").Value > 100;""")],
TestContext.Current.CancellationToken); TestContext.Current.CancellationToken);
// N more pushes while the orphan runs: every one is refused (a refusal is not a strike — I1), and // N more pushes while the orphan runs: every one is refused (a refusal is not a strike — I1), and
@@ -59,24 +63,41 @@ public sealed class ScriptedAlarmEngineRunGuardTests
var expected = i + 1; var expected = i + 1;
await WaitForAsync(() => eng.RunGuard.RefusedWhileRunningCount >= expected); await WaitForAsync(() => eng.RunGuard.RefusedWhileRunningCount >= expected);
} }
await WaitForAsync(() => eng.RunGuard.StartedRunCount >= 1 + 1 + pushes);
// #911: wait for the sibling's last run to FINISH, not merely to start. StartedRunCount reaching 6 only
// proves the healthy alarm's 4th-push run was admitted — it is still executing (InFlightCount 2: the
// orphan plus that run) until its finally releases the slot. Waiting on the started count and then
// asserting the in-flight count was the race.
await WaitForAsync(() => eng.RunGuard.StartedRunCount >= 1 + 1 + pushes && eng.RunGuard.InFlightCount == 1);
eng.RunGuard.IsQuarantined("runaway", out _).ShouldBeFalse("refusals are not strikes"); eng.RunGuard.IsQuarantined("runaway", out _).ShouldBeFalse("refusals are not strikes");
eng.RunGuard.StartedRunCount.ShouldBe(1 + 1 + pushes, "one dispatch for the runaway alarm, one per evaluation for the healthy one"); eng.RunGuard.StartedRunCount.ShouldBe(1 + 1 + pushes, "one dispatch for the runaway alarm, one per evaluation for the healthy one");
eng.RunGuard.InFlightCount.ShouldBe(1); eng.RunGuard.InFlightCount.ShouldBe(1, "exactly one orphan, never one per push");
// The frozen condition is annotated Bad out of band — the operator sees WHY it is not moving —
// exactly once across the whole run of non-completions.
ScriptedAlarmEvent[] snapshot;
lock (events) snapshot = events.ToArray();
var quality = snapshot.Where(e => e.AlarmId == "runaway" && e.Emission == EmissionKind.QualityChanged).ToArray();
quality.Length.ShouldBe(1);
quality[0].WorstInputStatusCode.ShouldBe(StatusBadInternalError);
eng.GetState("runaway")!.Active.ShouldBe(AlarmActiveState.Inactive, "state is held, never invented"); eng.GetState("runaway")!.Active.ShouldBe(AlarmActiveState.Inactive, "state is held, never invented");
// Positive control: the healthy sibling still transitions on the same engine. // Positive control: the healthy sibling still transitions on the same engine. The same push re-evaluates
// the runaway alarm too, and it must be refused once more — proving the single in-flight run counted
// above is the runaway's orphan (still holding its slot), not a leftover healthy run.
up.Push("Temp", 150); up.Push("Temp", 150);
await WaitForAsync(() => eng.GetState("healthy")!.Active == AlarmActiveState.Active); await WaitForAsync(() => SnapshotOf(events).Any(e => e.AlarmId == "healthy" && e.Emission == EmissionKind.Activated));
eng.RunGuard.RefusedWhileRunningCount.ShouldBe(pushes + 1);
eng.RunGuard.StartedRunCount.ShouldBe(1 + 1 + pushes + 1, "only the healthy alarm was dispatched");
// The frozen condition is annotated Bad out of band — the operator sees WHY it is not moving —
// exactly once across the whole run of non-completions. Checked only AFTER the healthy Activated event
// arrived: the single drainer fires each pass's events in push order, so every earlier refusal's
// emissions have already been delivered and a per-push re-annotation could not still be in transit.
var quality = SnapshotOf(events).Where(e => e.AlarmId == "runaway" && e.Emission == EmissionKind.QualityChanged).ToArray();
quality.Length.ShouldBe(1);
quality[0].WorstInputStatusCode.ShouldBe(StatusBadInternalError);
}
private const string HealthySpin =
"var spinUntil = System.DateTime.UtcNow.AddMilliseconds(100); while (System.DateTime.UtcNow < spinUntil) { }";
private static ScriptedAlarmEvent[] SnapshotOf(List<ScriptedAlarmEvent> events)
{
lock (events) return events.ToArray();
} }
/// <summary>I3 — a QUARANTINED predicate is re-attempted on every push and refused before dispatch; the /// <summary>I3 — a QUARANTINED predicate is re-attempted on every push and refused before dispatch; the