From 8ef5adc63ad9d5beb1e0373c3a3ba4d3927a1d8d Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 9 Nov 2026 12:06:36 -0500 Subject: [PATCH] test(modbus): guarantee cleanup in the cache-reset fixture's failure paths (#727) Reinitialize_clears_lastPublished_and_lastWritten_caches now registers ShutdownAsync through the shared FixtureCleanup helper before initialize, so an initialize, seeding/polling-observation or assertion failure still stops the probe loop and poll engine and disposes the transport; the primary failure is preserved and a secondary cleanup failure is reported. The in-body shutdown still precedes the cache-clearing assertions; the trailing cleanup is safe because shutdown is idempotent. Default probe settings, the real subscription path and the bounded deadline are unchanged. New Cache_reset_fixture_failure_after_subscribe_still_shuts_the_driver_down fails the body after the subscription is live and asserts the sentinel is kept, the transport is disposed exactly once, caches are cleared and no further poll or probe sends land (fails if the cleanup is removed). No production change. --- .../ModbusLifecycleHygieneTests.cs | 105 ++++++++++++++---- 1 file changed, 82 insertions(+), 23 deletions(-) diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusLifecycleHygieneTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusLifecycleHygieneTests.cs index 35417060..6bd7f9a8 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusLifecycleHygieneTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusLifecycleHygieneTests.cs @@ -108,28 +108,28 @@ public sealed class ModbusLifecycleHygieneTests(ITestOutputHelper output) GetTagsByName(drv).Count.ShouldBe(0, "Shutdown must clear the tag cache so the next Initialize starts clean"); } - /// Verifies that reinitialization clears the deadband and write-suppression caches. - [Fact] - public async Task Reinitialize_clears_lastPublished_and_lastWritten_caches() + private static ModbusDriverOptions CacheResetOptions() => new() { - var fake = new FakeTransport(); - var opts = new ModbusDriverOptions - { - Host = "fake", - WriteOnChangeOnly = true, - RawTags = ModbusRawTags.Entries([new ModbusTagDefinition("A", ModbusRegion.HoldingRegisters, 0, ModbusDataType.Int16, - Deadband: 1.0)]), - }; - var drv = new ModbusDriver(opts, "modbus-1", _ => fake); - await drv.InitializeAsync("{}", CancellationToken.None); + Host = "fake", + WriteOnChangeOnly = true, + RawTags = ModbusRawTags.Entries([new ModbusTagDefinition("A", ModbusRegion.HoldingRegisters, 0, ModbusDataType.Int16, + Deadband: 1.0)]), + }; - var lastPublished = (System.Collections.IDictionary)typeof(ModbusDriver) - .GetField("_lastPublishedByRef", BindingFlags.NonPublic | BindingFlags.Instance)! - .GetValue(drv)!; - var lastWritten = (System.Collections.IDictionary)typeof(ModbusDriver) - .GetField("_lastWrittenByRef", BindingFlags.NonPublic | BindingFlags.Instance)! + private static System.Collections.IDictionary GetCache(ModbusDriver drv, string field) => + (System.Collections.IDictionary)typeof(ModbusDriver) + .GetField(field, BindingFlags.NonPublic | BindingFlags.Instance)! .GetValue(drv)!; + /// + /// Seeds the write-suppression cache with a write and the deadband cache through a real subscription, + /// waiting (bounded) for the poll engine to publish. Returns the live subscription handle. + /// + private static async Task SeedCachesAsync(ModbusDriver drv, FakeTransport fake) + { + var lastPublished = GetCache(drv, "_lastPublishedByRef"); + var lastWritten = GetCache(drv, "_lastWrittenByRef"); + // Seed both caches via a write (lastWritten) and a publish through ShouldPublish (lastPublished). await drv.WriteAsync([new WriteRequest("A", (short)5)], CancellationToken.None); lastWritten.Count.ShouldBe(1); @@ -138,13 +138,72 @@ public sealed class ModbusLifecycleHygieneTests(ITestOutputHelper output) fake.HoldingRegisters[0] = 5; var handle = await drv.SubscribeAsync(["A"], TimeSpan.FromMilliseconds(100), CancellationToken.None); var deadline = DateTime.UtcNow.AddSeconds(2); - while (lastPublished.Count == 0 && DateTime.UtcNow < deadline) await Task.Delay(25); + while (lastPublished.Count == 0 && DateTime.UtcNow < deadline) await Task.Delay(25, TestContext.Current.CancellationToken); lastPublished.Count.ShouldBe(1); - await drv.UnsubscribeAsync(handle, CancellationToken.None); + return handle; + } - await drv.ShutdownAsync(CancellationToken.None); - lastPublished.Count.ShouldBe(0, "Shutdown must clear the deadband cache"); - lastWritten.Count.ShouldBe(0, "Shutdown must clear the write-suppression cache"); + /// Verifies that reinitialization clears the deadband and write-suppression caches. + /// + /// #727: cleanup is registered before initialize, so an initialize, seeding or assertion failure still + /// shuts the driver down (probe loop, poll engine, transport). The in-body ShutdownAsync stays ahead of + /// the cache-clearing assertions it is the oracle for; the registered cleanup runs after it and is safe + /// because shutdown is idempotent. + /// + [Fact] + public async Task Reinitialize_clears_lastPublished_and_lastWritten_caches() + { + var fake = new FakeTransport(); + var drv = new ModbusDriver(CacheResetOptions(), "modbus-1", _ => fake); + await FixtureCleanup.RunAsync(async () => + { + await drv.InitializeAsync("{}", CancellationToken.None); + var lastPublished = GetCache(drv, "_lastPublishedByRef"); + var lastWritten = GetCache(drv, "_lastWrittenByRef"); + + var handle = await SeedCachesAsync(drv, fake); + await drv.UnsubscribeAsync(handle, CancellationToken.None); + + await drv.ShutdownAsync(CancellationToken.None); + lastPublished.Count.ShouldBe(0, "Shutdown must clear the deadband cache"); + lastWritten.Count.ShouldBe(0, "Shutdown must clear the write-suppression cache"); + }, () => drv.ShutdownAsync(CancellationToken.None), + failure => output.WriteLine($"Modbus cache-reset fixture cleanup failed: {failure}")); + } + + /// + /// #727 failure path: a body that fails after the subscription and probe loop are live keeps its + /// primary exception, and the registered cleanup still releases the fixture — the transport is + /// disposed once, the caches are cleared, and no further polling or probe sends land. + /// + [Fact] + public async Task Cache_reset_fixture_failure_after_subscribe_still_shuts_the_driver_down() + { + var fake = new FakeTransport(); + var drv = new ModbusDriver(CacheResetOptions(), "modbus-1", _ => fake); + var sentinel = new InvalidOperationException("cache-reset body sentinel"); + + var actual = await Should.ThrowAsync(() => FixtureCleanup.RunAsync(async () => + { + await drv.InitializeAsync("{}", CancellationToken.None); + _ = await SeedCachesAsync(drv, fake); + fake.DisposeCount.ShouldBe(0); + throw sentinel; // no Unsubscribe, no Shutdown — the leak shape the fixture used to have + }, () => drv.ShutdownAsync(CancellationToken.None), + failure => output.WriteLine($"Modbus cache-reset fixture cleanup failed: {failure}"))); + + actual.ShouldBeSameAs(sentinel); + actual.Data.Contains(FixtureCleanup.SecondaryExceptionKey).ShouldBeFalse("cleanup itself must succeed"); + fake.DisposeCount.ShouldBe(1, "cleanup must dispose the transport exactly once"); + GetCache(drv, "_lastPublishedByRef").Count.ShouldBe(0); + GetCache(drv, "_lastWrittenByRef").Count.ShouldBe(0); + drv.GetHealth().State.ShouldNotBe(DriverState.Healthy); + + // Poll engine and probe loop are both stopped: the send count is flat across several poll intervals. + var sendsAfterCleanup = Interlocked.CompareExchange(ref fake.SendCount, 0, 0); + await Task.Delay(300, TestContext.Current.CancellationToken); + Interlocked.CompareExchange(ref fake.SendCount, 0, 0).ShouldBe(sendsAfterCleanup, + "no subscription poll or probe may run after the failure-path cleanup"); } // -------------------- Finding -004 / -012 (4) --------------------