diff --git a/code-reviews/Driver.AbCip.Cli/findings.md b/code-reviews/Driver.AbCip.Cli/findings.md index 88fa810..b7cc018 100644 --- a/code-reviews/Driver.AbCip.Cli/findings.md +++ b/code-reviews/Driver.AbCip.Cli/findings.md @@ -7,7 +7,7 @@ | Review date | 2026-05-22 | | Commit reviewed | `76d35d1` | | Status | Reviewed | -| Open findings | 6 | +| Open findings | 0 | ## Checklist coverage @@ -96,7 +96,7 @@ into `AbCipCommandBase`. | Severity | Low | | Category | Concurrency & thread safety | | Location | `src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli/Commands/SubscribeCommand.cs:50-56,60-61` | -| Status | Open | +| Status | Resolved | **Description:** The `OnDataChange` handler writes change lines to `console.Output` (a `TextWriter`) from the driver's poll-engine callback thread, while the command's @@ -112,7 +112,12 @@ during the watch loop widens it. writes during the subscription with a shared lock so poll-thread and main-thread output cannot interleave. -**Resolution:** _(open)_ +**Resolution:** Resolved 2026-05-23 — moved the "Subscribed to ... Ctrl+C to stop." +banner write to BEFORE `driver.OnDataChange +=` (and therefore before +`SubscribeAsync`). With the handler not yet attached when the banner runs, the +poll thread cannot fire change events into `console.Output` concurrently with the +main-thread banner write. After `+=` the only writer to `console.Output` is the +poll-thread handler, so no interleaving is possible. ### Driver.AbCip.Cli-004 @@ -121,7 +126,7 @@ output cannot interleave. | Severity | Low | | Category | Error handling & resilience | | Location | `src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli/Commands/SubscribeCommand.cs:28,58`; `AbCipCommandBase.cs:26-34` | -| Status | Open | +| Status | Resolved | **Description:** `--interval-ms` (`IntervalMs`) is taken verbatim and passed as `TimeSpan.FromMilliseconds(IntervalMs)` to `SubscribeAsync` with no validation. A @@ -135,7 +140,16 @@ downstream component to sanitise operator input is fragile. `--timeout-ms` on `ExecuteAsync` / in `AbCipCommandBase`, throwing a `CommandException` with the accepted range when out of bounds. -**Resolution:** _(open)_ +**Resolution:** Resolved 2026-05-23 — added `SubscribeCommand.ValidateInterval(int)` +(throws `CommandException` for `IntervalMs <= 0`) called at the top of +`SubscribeCommand.ExecuteAsync`; moved `TimeoutMs > 0` validation into the +`AbCipCommandBase.Timeout` getter (throws `CommandException` for non-positive +`TimeoutMs`) and added a `_ = Timeout` touch in `SubscribeCommand.ExecuteAsync` to +fire that guard before the driver opens. Other commands trip the same guard +naturally via `BuildOptions(...).Timeout`. Regression tests +`AbCipCommandBaseTests.Timeout_get_throws_CommandException_when_TimeoutMs_is_non_positive` +and `SubscribeCommandIntervalTests.ValidateInterval_rejects_non_positive` cover +both paths. ### Driver.AbCip.Cli-005 @@ -144,7 +158,7 @@ accepted range when out of bounds. | Severity | Low | | Category | Performance & resource management | | Location | `src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.Cli.Common/DriverCommandBase.cs:51-59` | -| Status | Open | +| Status | Resolved | **Description:** `ConfigureLogging` assigns a freshly created Serilog logger to the process-global `Log.Logger` but never calls `Log.CloseAndFlush()`. For a short-lived @@ -159,7 +173,12 @@ module's review.) `AppDomain.ProcessExit` or a `finally` in the command), or have the CLI use a disposable logger scoped to `ExecuteAsync`. -**Resolution:** _(open)_ +**Resolution:** Resolved 2026-05-23 — `Driver.Cli.Common` already exposes +`DriverCommandBase.FlushLogging()` (a `Log.CloseAndFlush()` wrapper); the AB CIP +CLI was not calling it. Added `FlushLogging()` in the `finally` block of all four +commands (`ProbeCommand`, `ReadCommand`, `WriteCommand`, `SubscribeCommand`) so +buffered Serilog output is flushed before the process exits, including the +Ctrl+C-driven `subscribe` path. No edits to `Driver.Cli.Common` were needed. ### Driver.AbCip.Cli-006 @@ -168,7 +187,7 @@ disposable logger scoped to `ExecuteAsync`. | Severity | Low | | Category | Design-document adherence | | Location | `src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli/AbCipCommandBase.cs:29-34` | -| Status | Open | +| Status | Resolved | **Description:** `AbCipCommandBase` overrides the abstract `DriverCommandBase.Timeout` property with a getter derived from `TimeoutMs` and an empty `init` body @@ -183,9 +202,13 @@ bug. **Recommendation:** Either drop the `init` accessor entirely (make the override a get-only expression-bodied property) or have the empty `init` throw `NotSupportedException` to make the "driven by TimeoutMs" contract explicit and -fail-fast. +fail-fast. (Drop is not viable because the abstract base declares `{ get; init; }` +and an override must provide both accessors.) -**Resolution:** _(open)_ +**Resolution:** Resolved 2026-05-23 — the `init` accessor now throws +`NotSupportedException` with a message pointing the caller at `TimeoutMs`. A new +test `AbCipCommandBaseTests.Timeout_setter_is_inert_and_does_not_silently_swallow_assignments` +asserts that an object-initializer assignment to `Timeout` fails fast. ### Driver.AbCip.Cli-007 @@ -194,7 +217,7 @@ fail-fast. | Severity | Low | | Category | Testing coverage | | Location | `tests/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli.Tests/WriteCommandParseValueTests.cs` | -| Status | Open | +| Status | Resolved | **Description:** The only test file covers `WriteCommand.ParseValue` and `ReadCommand.SynthesiseTagName` — both pure static helpers. There is no coverage for @@ -213,7 +236,12 @@ driver CLIs. (`HostAddress`, `PlcFamily`, `DeviceName`), the supplied tag list, and the `Timeout` derived from `TimeoutMs`. -**Resolution:** _(open)_ +**Resolution:** Resolved 2026-05-23 — added +`tests/.../AbCipCommandBaseTests.cs` covering `BuildOptions` (probe disabled, +controller-browse disabled, alarm-projection disabled, single device with +`HostAddress` / `PlcFamily` / `cli-{Family}` device name, tag list passed verbatim, +`Timeout` derived from `TimeoutMs`) and `DriverInstanceId` (`abcip-cli-{Gateway}`), +plus the `RejectStructure` guard (throws for `Structure`, no-op for atomic types). ### Driver.AbCip.Cli-008 @@ -222,7 +250,7 @@ derived from `TimeoutMs`. | Severity | Low | | Category | Documentation & comments | | Location | `docs/Driver.AbCip.Cli.md:8-9` | -| Status | Open | +| Status | Resolved | **Description:** `docs/Driver.AbCip.Cli.md` opens with "Second of four driver test-client CLIs (Modbus -> AB CIP -> AB Legacy -> S7 -> TwinCAT)." The count "four" @@ -235,4 +263,6 @@ doc's "four" and the truncated chain are both stale. and complete the chain (Modbus -> AB CIP -> AB Legacy -> S7 -> TwinCAT -> FOCAS), or drop the explicit count and link `docs/DriverClis.md` as the authoritative roster. -**Resolution:** _(open)_ +**Resolution:** Resolved 2026-05-23 — rewrote the lead paragraph to "Second of six +driver test-client CLIs (Modbus -> AB CIP -> AB Legacy -> S7 -> TwinCAT -> FOCAS)" +and added a link to `docs/DriverClis.md` as the authoritative roster. diff --git a/docs/Driver.AbCip.Cli.md b/docs/Driver.AbCip.Cli.md index 5020663..d2a0581 100644 --- a/docs/Driver.AbCip.Cli.md +++ b/docs/Driver.AbCip.Cli.md @@ -4,8 +4,9 @@ Ad-hoc probe / read / write / subscribe tool for ControlLogix / CompactLogix / Micro800 / GuardLogix PLCs, talking to the **same** `AbCipDriver` the OtOpcUa server uses (libplctag under the hood). -Second of four driver test-client CLIs (Modbus → AB CIP → AB Legacy → S7 → -TwinCAT). Shares `Driver.Cli.Common` with the others. +Second of six driver test-client CLIs (Modbus → AB CIP → AB Legacy → S7 → +TwinCAT → FOCAS). Shares `Driver.Cli.Common` with the others; see +[DriverClis.md](DriverClis.md) for the authoritative roster. ## Build + run diff --git a/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli/AbCipCommandBase.cs b/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli/AbCipCommandBase.cs index d683851..e2483dd 100644 --- a/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli/AbCipCommandBase.cs +++ b/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli/AbCipCommandBase.cs @@ -27,10 +27,28 @@ public abstract class AbCipCommandBase : DriverCommandBase public int TimeoutMs { get; init; } = 5000; /// + /// + /// The getter validates (Driver.AbCip.Cli-004) — a zero or + /// negative --timeout-ms would otherwise propagate as a non-positive + /// into the driver. The init accessor is unreachable + /// because CliFx binds rather than Timeout; it throws + /// so an object-initializer assignment + /// (new ReadCommand { Timeout = ... }) fails fast instead of being silently + /// discarded (Driver.AbCip.Cli-006). + /// public override TimeSpan Timeout { - get => TimeSpan.FromMilliseconds(TimeoutMs); - init { /* driven by TimeoutMs */ } + get + { + if (TimeoutMs <= 0) + throw new CliFx.Exceptions.CommandException( + $"--timeout-ms must be > 0 (got {TimeoutMs}). " + + "Pick a positive number of milliseconds for the per-operation timeout."); + return TimeSpan.FromMilliseconds(TimeoutMs); + } + init => throw new NotSupportedException( + $"{nameof(AbCipCommandBase)}.{nameof(Timeout)} is derived from {nameof(TimeoutMs)} " + + "and cannot be assigned directly. Set TimeoutMs instead."); } /// diff --git a/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli/Commands/ProbeCommand.cs b/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli/Commands/ProbeCommand.cs index ca849cc..f6fd96a 100644 --- a/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli/Commands/ProbeCommand.cs +++ b/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli/Commands/ProbeCommand.cs @@ -54,6 +54,9 @@ public sealed class ProbeCommand : AbCipCommandBase finally { await driver.ShutdownAsync(CancellationToken.None); + // Driver.AbCip.Cli-005 — flush Serilog before process exit so buffered log + // output emitted during driver shutdown is not lost. + FlushLogging(); } } } diff --git a/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli/Commands/ReadCommand.cs b/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli/Commands/ReadCommand.cs index deeb26f..b4b1c99 100644 --- a/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli/Commands/ReadCommand.cs +++ b/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli/Commands/ReadCommand.cs @@ -49,6 +49,9 @@ public sealed class ReadCommand : AbCipCommandBase finally { await driver.ShutdownAsync(CancellationToken.None); + // Driver.AbCip.Cli-005 — flush Serilog before process exit so buffered log + // output emitted during driver shutdown is not lost. + FlushLogging(); } } diff --git a/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli/Commands/SubscribeCommand.cs b/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli/Commands/SubscribeCommand.cs index beb8a2a..2c4bd07 100644 --- a/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli/Commands/SubscribeCommand.cs +++ b/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli/Commands/SubscribeCommand.cs @@ -31,6 +31,10 @@ public sealed class SubscribeCommand : AbCipCommandBase { ConfigureLogging(); RejectStructure(DataType); + ValidateInterval(IntervalMs); + // Touch Timeout to surface the --timeout-ms guard (Driver.AbCip.Cli-004) before + // we open a driver — fast-fail with a clean CommandException on bad operator input. + _ = Timeout; var ct = console.RegisterCancellationHandler(); var tagName = ReadCommand.SynthesiseTagName(TagPath, DataType); @@ -48,6 +52,13 @@ public sealed class SubscribeCommand : AbCipCommandBase { await driver.InitializeAsync("{}", ct); + // Driver.AbCip.Cli-003 — emit the banner BEFORE wiring OnDataChange so the + // main-thread write cannot interleave with poll-thread change-event writes. + // TextWriter.WriteLine is not guaranteed thread-safe; once the handler is + // attached and SubscribeAsync starts, change events run on the poll thread. + await console.Output.WriteLineAsync( + $"Subscribed to {TagPath} @ {IntervalMs}ms. Ctrl+C to stop."); + driver.OnDataChange += (_, e) => { var line = $"[{DateTime.UtcNow:HH:mm:ss.fff}] " + @@ -58,8 +69,6 @@ public sealed class SubscribeCommand : AbCipCommandBase handle = await driver.SubscribeAsync([tagName], TimeSpan.FromMilliseconds(IntervalMs), ct); - await console.Output.WriteLineAsync( - $"Subscribed to {TagPath} @ {IntervalMs}ms. Ctrl+C to stop."); try { await Task.Delay(System.Threading.Timeout.InfiniteTimeSpan, ct); @@ -77,6 +86,23 @@ public sealed class SubscribeCommand : AbCipCommandBase catch { /* teardown best-effort */ } } await driver.ShutdownAsync(CancellationToken.None); + // Driver.AbCip.Cli-005 — flush Serilog before process exit so buffered log + // lines emitted just before Ctrl+C are not lost on abrupt termination. + FlushLogging(); } } + + /// + /// Guards --interval-ms against zero or negative values (Driver.AbCip.Cli-004). + /// A non-positive interval would produce a non-positive into + /// SubscribeAsync; the CLI should fail fast with an actionable error rather + /// than relying on the downstream PollGroupEngine to clamp the value. + /// + internal static void ValidateInterval(int intervalMs) + { + if (intervalMs <= 0) + throw new CliFx.Exceptions.CommandException( + $"--interval-ms must be > 0 (got {intervalMs}). " + + "PollGroupEngine floors sub-250ms values, but accepts any positive integer."); + } } diff --git a/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli/Commands/WriteCommand.cs b/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli/Commands/WriteCommand.cs index 8d4ed0f..061d68a 100644 --- a/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli/Commands/WriteCommand.cs +++ b/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli/Commands/WriteCommand.cs @@ -60,6 +60,9 @@ public sealed class WriteCommand : AbCipCommandBase finally { await driver.ShutdownAsync(CancellationToken.None); + // Driver.AbCip.Cli-005 — flush Serilog before process exit so buffered log + // output emitted during driver shutdown is not lost. + FlushLogging(); } } diff --git a/tests/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli.Tests/AbCipCommandBaseTests.cs b/tests/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli.Tests/AbCipCommandBaseTests.cs new file mode 100644 index 0000000..594b00f --- /dev/null +++ b/tests/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli.Tests/AbCipCommandBaseTests.cs @@ -0,0 +1,210 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli.Commands; + +namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli.Tests; + +/// +/// Covers : the shared BuildOptions projection +/// (driver-options mapping the four commands depend on), the RejectStructure +/// guard, the Timeout override behaviour, and TimeoutMs validation. +/// +[Trait("Category", "Unit")] +public sealed class AbCipCommandBaseTests +{ + /// + /// Local subclass that surfaces the protected helpers + properties under test. + /// + [CliFx.Attributes.Command("test")] + private sealed class TestableCommand : AbCipCommandBase + { + public AbCipDriverOptions InvokeBuildOptions(IReadOnlyList tags) + => BuildOptions(tags); + + public string InvokeDriverInstanceId => DriverInstanceId; + + public override ValueTask ExecuteAsync(CliFx.Infrastructure.IConsole console) + => ValueTask.CompletedTask; + } + + private static AbCipTagDefinition SampleTag(string name = "Motor01") => new( + Name: name, + DeviceHostAddress: "ab://10.0.0.5/1,0", + TagPath: "Motor01", + DataType: AbCipDataType.DInt, + Writable: false); + + [Fact] + public void BuildOptions_disables_probe_so_cli_does_not_race_operator_reads() + { + var cmd = new TestableCommand + { + Gateway = "ab://10.0.0.5/1,0", + Family = AbCipPlcFamily.ControlLogix, + TimeoutMs = 5000, + }; + + var options = cmd.InvokeBuildOptions([SampleTag()]); + + options.Probe.Enabled.ShouldBeFalse(); + } + + [Fact] + public void BuildOptions_disables_controller_browse() + { + var cmd = new TestableCommand + { + Gateway = "ab://10.0.0.5/1,0", + Family = AbCipPlcFamily.ControlLogix, + TimeoutMs = 5000, + }; + + var options = cmd.InvokeBuildOptions([SampleTag()]); + + options.EnableControllerBrowse.ShouldBeFalse(); + } + + [Fact] + public void BuildOptions_disables_alarm_projection() + { + var cmd = new TestableCommand + { + Gateway = "ab://10.0.0.5/1,0", + Family = AbCipPlcFamily.ControlLogix, + TimeoutMs = 5000, + }; + + var options = cmd.InvokeBuildOptions([SampleTag()]); + + options.EnableAlarmProjection.ShouldBeFalse(); + } + + [Fact] + public void BuildOptions_produces_one_device_with_gateway_family_and_derived_name() + { + var cmd = new TestableCommand + { + Gateway = "ab://10.0.0.5/1,0", + Family = AbCipPlcFamily.CompactLogix, + TimeoutMs = 5000, + }; + + var options = cmd.InvokeBuildOptions([SampleTag()]); + + options.Devices.Count.ShouldBe(1); + var device = options.Devices[0]; + device.HostAddress.ShouldBe("ab://10.0.0.5/1,0"); + device.PlcFamily.ShouldBe(AbCipPlcFamily.CompactLogix); + device.DeviceName.ShouldBe("cli-CompactLogix"); + } + + [Fact] + public void BuildOptions_passes_supplied_tag_list_verbatim() + { + var tags = new[] { SampleTag("t1"), SampleTag("t2") }; + var cmd = new TestableCommand + { + Gateway = "ab://10.0.0.5/1,0", + Family = AbCipPlcFamily.ControlLogix, + TimeoutMs = 5000, + }; + + var options = cmd.InvokeBuildOptions(tags); + + options.Tags.Count.ShouldBe(2); + options.Tags[0].Name.ShouldBe("t1"); + options.Tags[1].Name.ShouldBe("t2"); + } + + [Fact] + public void BuildOptions_carries_TimeoutMs_through_to_Timeout() + { + var cmd = new TestableCommand + { + Gateway = "ab://10.0.0.5/1,0", + Family = AbCipPlcFamily.ControlLogix, + TimeoutMs = 7500, + }; + + var options = cmd.InvokeBuildOptions([SampleTag()]); + + options.Timeout.ShouldBe(TimeSpan.FromMilliseconds(7500)); + } + + [Fact] + public void DriverInstanceId_embeds_gateway_for_log_disambiguation() + { + var cmd = new TestableCommand + { + Gateway = "ab://10.0.0.5/1,0", + Family = AbCipPlcFamily.ControlLogix, + }; + + cmd.InvokeDriverInstanceId.ShouldBe("abcip-cli-ab://10.0.0.5/1,0"); + } + + [Fact] + public void Timeout_setter_is_inert_and_does_not_silently_swallow_assignments() + { + // Driver.AbCip.Cli-006 — the empty init body would silently discard an + // object-initializer assignment, hiding a "driven by TimeoutMs" misuse. The fix + // makes it fail-fast with NotSupportedException so the contract is explicit. + Should.Throw(() => new TestableCommand + { + Gateway = "ab://10.0.0.5/1,0", + Timeout = TimeSpan.FromSeconds(99), + }); + } + + [Theory] + [InlineData(0)] + [InlineData(-1)] + public void Timeout_get_throws_CommandException_when_TimeoutMs_is_non_positive(int badMs) + { + // Driver.AbCip.Cli-004 — TimeoutMs must be > 0. Validation is exposed via the + // Timeout getter so any command path that touches Timeout sees the same guard. + var cmd = new TestableCommand + { + Gateway = "ab://10.0.0.5/1,0", + TimeoutMs = badMs, + }; + + var ex = Should.Throw(() => _ = cmd.Timeout); + ex.Message.ShouldContain("--timeout-ms"); + } + + [Fact] + public void RejectStructure_throws_for_Structure_DataType() + { + var ex = Should.Throw( + () => CallRejectStructure(AbCipDataType.Structure)); + ex.Message.ShouldContain("Structure"); + } + + [Theory] + [InlineData(AbCipDataType.DInt)] + [InlineData(AbCipDataType.Bool)] + [InlineData(AbCipDataType.Real)] + public void RejectStructure_passes_for_atomic_types(AbCipDataType type) + { + // No throw — atomic types are allowed. + Should.NotThrow(() => CallRejectStructure(type)); + } + + // The static helper is protected; reflect to it once so the test stays at AbCipCommandBase. + private static void CallRejectStructure(AbCipDataType type) + { + var method = typeof(AbCipCommandBase).GetMethod( + "RejectStructure", + System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Static) + ?? throw new InvalidOperationException("RejectStructure not found"); + try + { + method.Invoke(null, [type]); + } + catch (System.Reflection.TargetInvocationException tie) when (tie.InnerException is not null) + { + throw tie.InnerException; + } + } +} diff --git a/tests/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli.Tests/SubscribeCommandIntervalTests.cs b/tests/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli.Tests/SubscribeCommandIntervalTests.cs new file mode 100644 index 0000000..43bb8ea --- /dev/null +++ b/tests/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli.Tests/SubscribeCommandIntervalTests.cs @@ -0,0 +1,34 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli.Commands; + +namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli.Tests; + +/// +/// Covers — the guard that +/// stops a zero / negative --interval-ms from reaching SubscribeAsync +/// as a non-positive . +/// +[Trait("Category", "Unit")] +public sealed class SubscribeCommandIntervalTests +{ + [Theory] + [InlineData(0)] + [InlineData(-1)] + [InlineData(-500)] + public void ValidateInterval_rejects_non_positive(int badMs) + { + var ex = Should.Throw( + () => SubscribeCommand.ValidateInterval(badMs)); + ex.Message.ShouldContain("--interval-ms"); + } + + [Theory] + [InlineData(1)] + [InlineData(250)] + [InlineData(60_000)] + public void ValidateInterval_accepts_positive(int goodMs) + { + Should.NotThrow(() => SubscribeCommand.ValidateInterval(goodMs)); + } +}