From ddafc5c811b1216f279afb7d8685929cbd50f6aa Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Fri, 19 Jun 2026 00:33:39 -0400 Subject: [PATCH] test(commons): realign enum-count guards (AuditKind/AuditChannel/DataType) + derace StaleTagMonitor timer tests (#228, pre-existing) --- .../Types/EnumTests.cs | 2 +- .../Types/Enums/AuditEnumTests.cs | 7 ++- .../Types/StaleTagMonitorCollection.cs | 10 ++++ .../Types/StaleTagMonitorRaceTests.cs | 3 + .../Types/StaleTagMonitorTests.cs | 60 +++++++++++++++---- 5 files changed, 66 insertions(+), 16 deletions(-) create mode 100644 tests/ZB.MOM.WW.ScadaBridge.Commons.Tests/Types/StaleTagMonitorCollection.cs diff --git a/tests/ZB.MOM.WW.ScadaBridge.Commons.Tests/Types/EnumTests.cs b/tests/ZB.MOM.WW.ScadaBridge.Commons.Tests/Types/EnumTests.cs index 2db01f51..ded4e95b 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.Commons.Tests/Types/EnumTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.Commons.Tests/Types/EnumTests.cs @@ -5,7 +5,7 @@ namespace ZB.MOM.WW.ScadaBridge.Commons.Tests.Types; public class EnumTests { [Theory] - [InlineData(typeof(DataType), new[] { "Boolean", "Int32", "Float", "Double", "String", "DateTime", "Binary" })] + [InlineData(typeof(DataType), new[] { "Boolean", "Int32", "Float", "Double", "String", "DateTime", "Binary", "List" })] [InlineData(typeof(InstanceState), new[] { "NotDeployed", "Enabled", "Disabled" })] [InlineData(typeof(DeploymentStatus), new[] { "Pending", "InProgress", "Success", "Failed" })] [InlineData(typeof(AlarmState), new[] { "Active", "Normal" })] diff --git a/tests/ZB.MOM.WW.ScadaBridge.Commons.Tests/Types/Enums/AuditEnumTests.cs b/tests/ZB.MOM.WW.ScadaBridge.Commons.Tests/Types/Enums/AuditEnumTests.cs index a94ae3fa..0db2087b 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.Commons.Tests/Types/Enums/AuditEnumTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.Commons.Tests/Types/Enums/AuditEnumTests.cs @@ -12,7 +12,7 @@ public class AuditEnumTests [Fact] public void AuditChannel_HasExactlyExpectedMembers() { - var expected = new[] { "ApiOutbound", "DbOutbound", "Notification", "ApiInbound" }; + var expected = new[] { "ApiOutbound", "DbOutbound", "Notification", "ApiInbound", "SecuredWrite" }; var actual = Enum.GetValues(typeof(AuditChannel)) .Cast() .Select(x => x.ToString()) @@ -23,20 +23,21 @@ public class AuditEnumTests } [Fact] - public void AuditKind_HasExactlyTenExpectedMembers() + public void AuditKind_HasExactlyFourteenExpectedMembers() { var expected = new[] { "ApiCall", "ApiCallCached", "DbWrite", "DbWriteCached", "NotifySend", "NotifyDeliver", "InboundRequest", "InboundAuthFailure", "CachedSubmit", "CachedResolve", + "SecuredWriteSubmit", "SecuredWriteApprove", "SecuredWriteReject", "SecuredWriteExecute", }; var actual = Enum.GetValues(typeof(AuditKind)) .Cast() .Select(x => x.ToString()) .ToArray(); - Assert.Equal(10, actual.Length); + Assert.Equal(14, actual.Length); Assert.Equal(expected, actual); } diff --git a/tests/ZB.MOM.WW.ScadaBridge.Commons.Tests/Types/StaleTagMonitorCollection.cs b/tests/ZB.MOM.WW.ScadaBridge.Commons.Tests/Types/StaleTagMonitorCollection.cs new file mode 100644 index 00000000..0dd11ab9 --- /dev/null +++ b/tests/ZB.MOM.WW.ScadaBridge.Commons.Tests/Types/StaleTagMonitorCollection.cs @@ -0,0 +1,10 @@ +namespace ZB.MOM.WW.ScadaBridge.Commons.Tests.Types; + +/// +/// Marks both StaleTagMonitor test classes as non-parallel with each other and with the +/// rest of the test suite. This prevents CPU-contention-induced timing flakiness in tests +/// that rely on wall-clock timer deadlines (MaxSilence values in the 30–200ms range). +/// xUnit runs all tests in the same [Collection] sequentially. +/// +[CollectionDefinition("StaleTagMonitor", DisableParallelization = true)] +public class StaleTagMonitorCollection { } diff --git a/tests/ZB.MOM.WW.ScadaBridge.Commons.Tests/Types/StaleTagMonitorRaceTests.cs b/tests/ZB.MOM.WW.ScadaBridge.Commons.Tests/Types/StaleTagMonitorRaceTests.cs index 2bcf3e33..06ce9d59 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.Commons.Tests/Types/StaleTagMonitorRaceTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.Commons.Tests/Types/StaleTagMonitorRaceTests.cs @@ -5,6 +5,8 @@ namespace ZB.MOM.WW.ScadaBridge.Commons.Tests.Types; /// /// Regression tests for Commons-001: the check-then-act race between the timer /// callback (OnTimerElapsed) and OnValueReceived / Stop / Start. +/// Placed in the same non-parallel collection as to +/// avoid CPU contention from other timer-based tests running concurrently. /// /// The original implementation guarded firing with a single volatile bool that /// was both read by the callback and reset by the caller threads. Because the @@ -16,6 +18,7 @@ namespace ZB.MOM.WW.ScadaBridge.Commons.Tests.Types; /// These tests use the internal CallbackEnteredHook seam to deterministically /// interleave a caller-thread operation with an in-flight callback. /// +[Collection("StaleTagMonitor")] public class StaleTagMonitorRaceTests { /// diff --git a/tests/ZB.MOM.WW.ScadaBridge.Commons.Tests/Types/StaleTagMonitorTests.cs b/tests/ZB.MOM.WW.ScadaBridge.Commons.Tests/Types/StaleTagMonitorTests.cs index 2ac7a171..2f4f359b 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.Commons.Tests/Types/StaleTagMonitorTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.Commons.Tests/Types/StaleTagMonitorTests.cs @@ -2,8 +2,33 @@ using ZB.MOM.WW.ScadaBridge.Commons.Types; namespace ZB.MOM.WW.ScadaBridge.Commons.Tests.Types; +/// +/// Both StaleTagMonitor test classes are placed in the same non-parallel collection +/// to prevent wall-clock contention when the full suite runs under load. +/// Within each test, fixed-delay assertions are replaced with poll-until-condition +/// loops that tolerate CPU jitter on a busy machine. +/// +[Collection("StaleTagMonitor")] public class StaleTagMonitorTests { + // Generous poll timeout: tests use MaxSilence values of 50–200ms; we allow up to + // 5 seconds of total wait before declaring failure. This absorbs any scheduler lag + // that would previously cause a tight Task.Delay window to expire before the timer + // had a chance to fire. + private static readonly TimeSpan PollTimeout = TimeSpan.FromSeconds(5); + private static readonly TimeSpan PollInterval = TimeSpan.FromMilliseconds(10); + + private static async Task PollUntilAsync(Func condition, TimeSpan timeout) + { + var deadline = DateTime.UtcNow + timeout; + while (DateTime.UtcNow < deadline) + { + if (condition()) return true; + await Task.Delay(PollInterval); + } + return condition(); // final check + } + [Fact] public void Constructor_ZeroTimeSpan_Throws() { @@ -24,7 +49,8 @@ public class StaleTagMonitorTests monitor.Stale += () => Interlocked.Increment(ref staleCount); monitor.Start(); - await Task.Delay(300); + var fired = await PollUntilAsync(() => staleCount >= 1, PollTimeout); + Assert.True(fired, "Stale event did not fire within the generous timeout."); Assert.Equal(1, staleCount); } @@ -36,7 +62,13 @@ public class StaleTagMonitorTests monitor.Stale += () => Interlocked.Increment(ref staleCount); monitor.Start(); - await Task.Delay(300); + // Wait for the first (and only) fire, then hold for a bit to confirm no second fire. + var fired = await PollUntilAsync(() => staleCount >= 1, PollTimeout); + Assert.True(fired, "Stale event did not fire within the generous timeout."); + + // Hold long enough to confirm no second fire — the one-shot timer design means + // a second fire can only happen if OnValueReceived is called; we do not call it. + await Task.Delay(200); Assert.Equal(1, staleCount); } @@ -48,14 +80,16 @@ public class StaleTagMonitorTests monitor.Stale += () => Interlocked.Increment(ref staleCount); monitor.Start(); - // Keep resetting before the 200ms deadline + // Keep resetting before the 200ms deadline — use the monitor's own MaxSilence + // divided by 2 as the reset cadence so timing margin is relative, not absolute. + var resetInterval = (int)(monitor.MaxSilence.TotalMilliseconds / 2); for (int i = 0; i < 5; i++) { - await Task.Delay(100); + await Task.Delay(resetInterval); monitor.OnValueReceived(); } - // Should not have gone stale + // Should not have gone stale during the reset loop. Assert.Equal(0, staleCount); } @@ -67,12 +101,12 @@ public class StaleTagMonitorTests monitor.Stale += () => Interlocked.Increment(ref staleCount); monitor.Start(); - // Reset once + // Reset once before stale fires, then go silent and poll for the eventual fire. await Task.Delay(50); monitor.OnValueReceived(); - // Then go silent - await Task.Delay(250); + var fired = await PollUntilAsync(() => staleCount >= 1, PollTimeout); + Assert.True(fired, "Stale event did not fire after silence following a reset."); Assert.Equal(1, staleCount); } @@ -84,13 +118,15 @@ public class StaleTagMonitorTests monitor.Stale += () => Interlocked.Increment(ref staleCount); monitor.Start(); - // Wait for first stale - await Task.Delay(250); + // Wait for first stale via poll. + var firstFired = await PollUntilAsync(() => staleCount >= 1, PollTimeout); + Assert.True(firstFired, "First Stale event did not fire."); Assert.Equal(1, staleCount); - // Reset — should allow second stale fire + // Reset — should allow second stale fire. monitor.OnValueReceived(); - await Task.Delay(250); + var secondFired = await PollUntilAsync(() => staleCount >= 2, PollTimeout); + Assert.True(secondFired, "Second Stale event did not fire after reset."); Assert.Equal(2, staleCount); }