diff --git a/archreview/plans/R2-09-driver-fleet-batch-plan.md b/archreview/plans/R2-09-driver-fleet-batch-plan.md index 2bfe4d8c..5b56c0f5 100644 --- a/archreview/plans/R2-09-driver-fleet-batch-plan.md +++ b/archreview/plans/R2-09-driver-fleet-batch-plan.md @@ -569,3 +569,19 @@ Rules: tasks are bite-sized (≤ ~5 min of implementation each — TDD steps spl **Overall effort:** ≈ 5.5–7 dev-days across 6 independently shippable PRs (B1/B2/B5/B6 parallelizable from day one; B4 unblocks after B3's first two tasks). **Explicit hand-offs:** `ConnectionBackoff` → **R2-01** (S7 `EnsureConnectedAsync` throttle + the STAB-14/15 wrapper/classification fixes; B3.6's fork deletion removes the poll-loop half of STAB-14's blast surface in whichever order the two plans land). Parser strictness / `ReadEnum` / remaining equipment-parser parity → **R2-11**. Carried and untouched here: STAB-10, STAB-11, full STAB-12 (FOCAS close-PDU + TwinCAT blocking `Connect`), CONV-3 (`ILoggerFactory` — worth an early ride-along in any of these PRs per the report, but owned elsewhere), PERF-1/2, CONV-5 idioms. + +--- + +## Execution deviations (R2-09) + +Executed on branch `r2/09-driver-fleet-batch` off master `1a698cbb` (all 29 tasks completed; none deferred-live). Deviations from the plan, each a best-reversible call: + +- **Single branch, no PRs/merges.** The executor owns one isolated branch and never pushes/PRs/merges (house rule). The plan's per-sub-batch PR boundaries are collapsed onto this one branch; every "merge PR N" verification task was satisfied by running that sub-batch's driver unit suite(s) green and committing a status marker. The branch is ready for the parent to slice into PRs or merge as it sees fit. +- **Pre-existing unrelated failure (not R2-09).** `Core.Abstractions.Tests.InterfaceIndependenceTests.AllPublicTypes_LiveInRootNamespace` fails at base `1a698cbb` — it flags the pre-existing `Historian/` sub-namespace types (`HistorizationCommitMode`, `IHistorianValueWriter`, …), none of which this plan touched. The one new public type added here, `ConnectionBackoff`, correctly lives in the root `ZB.MOM.WW.OtOpcUa.Core.Abstractions` namespace. Left as-is (out of scope). +- **B3.6 stale handle-prefix lock-in test.** `S7DiscoveryAndSubscribeTests.SubscribeAsync_returns_unique_handles…` asserted the retired fork's `s7-sub-` `DiagnosticId` prefix. The fork retirement moves S7 onto `PollGroupEngine`, whose handle is `poll-sub-`; the assertion was updated to `poll-sub-` (a stale lock-in, per the watch-for-stale-tests note). +- **B3.5 test 3 reframed.** `SustainedPollFailure_DegradesHealth_WithBackoff` was landed as `SustainedPollFailure_DegradesHealth` — S7's `ReadAsync` already degrades health on comm failure (independent of `onError`), so a sustained-failing poll degrades health on both fork and engine; the timing-sensitive "WithBackoff" cadence assertion is covered deterministically by the engine-level `Backoff_*` tests (B3.2). S7's new `HandlePollError` health wiring is exercised by the migrated poll path. +- **FOCAS `ResolveHost` test initializes the driver.** FOCAS resolves equipment refs via `ResolveEquipmentTagRef`, which validates against the `_devices` map populated at `InitializeAsync` (the other three multi-device drivers parse directly, no init needed). The FOCAS `ResolveHostTests` init the driver — matching how `ResolveHost` is always called at runtime (post-init). +- **FOCAS connect backoff is best-effort.** FOCAS's `EnsureConnectedAsync` is not gate-serialized (its read/write/probe/fixed-tree/recycle loops already race the client swap by design), so the per-device `ConnectionBackoff` there is consulted/mutated without a gate — atomic enough on 64-bit for its int/DateTime state, with a benign extra/skipped attempt the worst outcome of a race. Documented in the field's XML doc. (TwinCAT/AbCip consult it under their existing gate / probe discipline.) +- **B6.5 is a white-box reflection test.** `RecycleClient_HonorsCancellation` invokes the private `RecycleClientAsync(device, ct)` with a cancelled token (via reflection, with the device's `ConnectGate` held) and asserts a prompt `OperationCanceledException` rather than an uncancellable hang. A full behavioral probe-loop wedge is impractical: in the single-probe-loop structure the loop blocks in the *already-cancellable* `EnsureConnectedAsync`, not `RecycleClientAsync`; the blocking-SDK-`Connect` root that makes the real wedge reproducible is carried under STAB-12 (out of scope). The reflection test deterministically pins the `None`→`ct` fix. +- **Live gates not exercised (offline macOS).** The optional/recommended live gates (B1.3 modbus `exception_injector`, B3.7 s7 fixture bounce) were left offline-unit-verified — every task's core behavior is fully covered by the per-driver unit suites (the authoritative gate for TwinCAT/FOCAS/AbLegacy, which have no fixture at all). No task was blocked on a fixture, so none is marked `deferred-live`. +- **Warning hygiene.** The five per-driver `PollErrorHealthTests` guard `DriverHealth.LastError` (nullable) with an explicit `ShouldNotBeNull()` before `ShouldContain` to clear a CS8604 nullable-flow warning. diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipPollErrorHealthTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipPollErrorHealthTests.cs index 35df58e5..3ba4a1af 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipPollErrorHealthTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipPollErrorHealthTests.cs @@ -35,6 +35,7 @@ public sealed class AbCipPollErrorHealthTests var after = drv.GetHealth(); after.State.ShouldBe(DriverState.Degraded); after.LastSuccessfulRead.ShouldBe(before.LastSuccessfulRead); + after.LastError.ShouldNotBeNull(); after.LastError.ShouldContain("poll boom"); } diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyPollErrorHealthTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyPollErrorHealthTests.cs index aebc691c..df66ca48 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyPollErrorHealthTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyPollErrorHealthTests.cs @@ -35,6 +35,7 @@ public sealed class AbLegacyPollErrorHealthTests var after = drv.GetHealth(); after.State.ShouldBe(DriverState.Degraded); after.LastSuccessfulRead.ShouldBe(before.LastSuccessfulRead); + after.LastError.ShouldNotBeNull(); after.LastError.ShouldContain("poll boom"); } diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasPollErrorHealthTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasPollErrorHealthTests.cs index 84a9da76..dd0dc166 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasPollErrorHealthTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasPollErrorHealthTests.cs @@ -34,6 +34,7 @@ public sealed class FocasPollErrorHealthTests var after = drv.GetHealth(); after.State.ShouldBe(DriverState.Degraded); after.LastSuccessfulRead.ShouldBe(before.LastSuccessfulRead); + after.LastError.ShouldNotBeNull(); after.LastError.ShouldContain("poll boom"); } diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusPollErrorHealthTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusPollErrorHealthTests.cs index df86c6ac..2b6515ed 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusPollErrorHealthTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusPollErrorHealthTests.cs @@ -39,6 +39,7 @@ public sealed class ModbusPollErrorHealthTests var after = drv.GetHealth(); after.State.ShouldBe(DriverState.Degraded); after.LastSuccessfulRead.ShouldBe(before.LastSuccessfulRead); + after.LastError.ShouldNotBeNull(); after.LastError.ShouldContain("poll boom"); } diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATPollErrorHealthTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATPollErrorHealthTests.cs index 1f97f399..61e21e93 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATPollErrorHealthTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATPollErrorHealthTests.cs @@ -37,6 +37,7 @@ public sealed class TwinCATPollErrorHealthTests var after = drv.GetHealth(); after.State.ShouldBe(DriverState.Degraded); after.LastSuccessfulRead.ShouldBe(before.LastSuccessfulRead); + after.LastError.ShouldNotBeNull(); after.LastError.ShouldContain("poll boom"); }