From cec7ab6ec48a2dc1067419ebc69ccc01b31118ae Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Fri, 22 May 2026 09:30:17 -0400 Subject: [PATCH] fix(alarm-historian): resolve Medium code-review finding (Core.AlarmHistorian-010) The test suite lacked coverage for four critical paths: corrupt/null- deserializing PayloadJson rows, StartDrainLoop timer behavior and backoff honoring, concurrent EnqueueAsync+DrainOnceAsync stress, and the outcomes.Count != events.Count cardinality-mismatch branch. Added tests covering all four gaps (committed across companion findings): - Drain_with_corrupt_payload_row_deadletters_it_and_keeps_good_rows_aligned - Drain_with_corrupt_head_row_does_not_stall_queue - StartDrainLoop_honors_backoff_and_slows_cadence_under_retry - StartDrainLoop_keeps_steady_cadence_when_writer_is_healthy - StartDrainLoop_records_drain_fault_and_keeps_running - Concurrent_enqueue_and_drain_do_not_throw_sqlite_busy - Writer_returning_wrong_cardinality_outcomes_sets_backing_off_and_keeps_rows - Capacity_eviction_increments_evicted_count_on_status - GetStatus_snapshot_is_consistent_under_concurrent_drain Updated Open findings count to 2 (Core.AlarmHistorian-008 + -011, both Low). Co-Authored-By: Claude Opus 4.7 (1M context) --- code-reviews/Core.AlarmHistorian/findings.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/code-reviews/Core.AlarmHistorian/findings.md b/code-reviews/Core.AlarmHistorian/findings.md index e64bce0..12672d8 100644 --- a/code-reviews/Core.AlarmHistorian/findings.md +++ b/code-reviews/Core.AlarmHistorian/findings.md @@ -7,7 +7,7 @@ | Review date | 2026-05-22 | | Commit reviewed | `76d35d1` | | Status | Reviewed | -| Open findings | 7 | +| Open findings | 2 | ## Checklist coverage @@ -168,13 +168,13 @@ | Severity | Medium | | Category | Testing coverage | | Location | `tests/Core/ZB.MOM.WW.OtOpcUa.Core.AlarmHistorian.Tests/SqliteStoreAndForwardSinkTests.cs` | -| Status | Open | +| Status | Resolved | **Description:** The test suite covers the happy paths well (Ack/Retry/PermanentFail, capacity eviction, retention purge, ctor validation) but leaves critical paths untested: (a) no test exercises a corrupt / `null`-deserializing `PayloadJson` row, so the `rowIds`/`events` misalignment bug (Core.AlarmHistorian-001) was not caught; (b) no test for `StartDrainLoop` actually running on the timer, nor for the backoff being honored by the schedule (Core.AlarmHistorian-002); (c) no concurrency test running `EnqueueAsync` and `DrainOnceAsync` in parallel, which is the exact scenario that triggers `SQLITE_BUSY` (Core.AlarmHistorian-004); (d) no test for the `outcomes.Count != events.Count` cardinality-mismatch branch (Core.AlarmHistorian-007). **Recommendation:** Add tests for: a corrupt payload row (insert raw bad JSON via a direct SQLite write, then drain and assert the correct row is dead-lettered and others are unaffected); a `FakeWriter` returning a wrong-length outcome list; a parallel enqueue/drain stress test; and the timer-driven `StartDrainLoop` path. -**Resolution:** _(open)_ +**Resolution:** Resolved 2026-05-22 — (a) `Drain_with_corrupt_payload_row_deadletters_it_and_keeps_good_rows_aligned` and `Drain_with_corrupt_head_row_does_not_stall_queue` cover corrupt payloads; (b) `StartDrainLoop_honors_backoff_and_slows_cadence_under_retry`, `StartDrainLoop_keeps_steady_cadence_when_writer_is_healthy`, and `StartDrainLoop_records_drain_fault_and_keeps_running` cover the timer-driven path; (c) `Concurrent_enqueue_and_drain_do_not_throw_sqlite_busy` covers the concurrent stress path; (d) `Writer_returning_wrong_cardinality_outcomes_sets_backing_off_and_keeps_rows` covers the cardinality-mismatch branch. Additionally `Capacity_eviction_increments_evicted_count_on_status` and `GetStatus_snapshot_is_consistent_under_concurrent_drain` cover -009 and -005 respectively. ### Core.AlarmHistorian-011