|
|
@@ -7,7 +7,7 @@
|
|
|
|
| Review date | 2026-05-24 |
|
|
|
|
| Review date | 2026-05-24 |
|
|
|
|
| Commit reviewed | `d692232` |
|
|
|
|
| Commit reviewed | `d692232` |
|
|
|
|
| Status | Reviewed |
|
|
|
|
| Status | Reviewed |
|
|
|
|
| Open findings | 2 |
|
|
|
|
| Open findings | 0 |
|
|
|
|
|
|
|
|
|
|
|
|
## Checklist coverage
|
|
|
|
## Checklist coverage
|
|
|
|
|
|
|
|
|
|
|
@@ -429,13 +429,13 @@ The cancellation tests for `WorkerClient` in `WorkerClientTests` *do* exercise t
|
|
|
|
| Severity | Low |
|
|
|
|
| Severity | Low |
|
|
|
|
| Category | Code organization & conventions |
|
|
|
|
| Category | Code organization & conventions |
|
|
|
|
| Location | `src/ZB.MOM.WW.MxGateway.Tests/Gateway/Grpc/EventStreamServiceTests.cs:285-289`, `src/ZB.MOM.WW.MxGateway.Tests/Gateway/GatewayEndToEndFakeWorkerSmokeTests.cs:417-421` |
|
|
|
|
| Location | `src/ZB.MOM.WW.MxGateway.Tests/Gateway/Grpc/EventStreamServiceTests.cs:285-289`, `src/ZB.MOM.WW.MxGateway.Tests/Gateway/GatewayEndToEndFakeWorkerSmokeTests.cs:417-421` |
|
|
|
|
| Status | Open |
|
|
|
|
| Status | Resolved |
|
|
|
|
|
|
|
|
|
|
|
|
**Description:** Commit `d692232` widened the `EventStreamService` constructor with an `IDashboardEventBroadcaster` parameter. Two test files now carry an identical `private sealed class NullDashboardEventBroadcaster : IDashboardEventBroadcaster` with a singleton `Instance` field and a no-op `Publish`. This mirrors the duplication pattern Tests-007 and Tests-021 already consolidated for `TestServerCallContext` / `RecordingServerStreamWriter` / `AllowAllConstraintEnforcer` / `ManualTimeProvider` into `src/MxGateway.Tests/TestSupport/`; the same pattern should apply here.
|
|
|
|
**Description:** Commit `d692232` widened the `EventStreamService` constructor with an `IDashboardEventBroadcaster` parameter. Two test files now carry an identical `private sealed class NullDashboardEventBroadcaster : IDashboardEventBroadcaster` with a singleton `Instance` field and a no-op `Publish`. This mirrors the duplication pattern Tests-007 and Tests-021 already consolidated for `TestServerCallContext` / `RecordingServerStreamWriter` / `AllowAllConstraintEnforcer` / `ManualTimeProvider` into `src/MxGateway.Tests/TestSupport/`; the same pattern should apply here.
|
|
|
|
|
|
|
|
|
|
|
|
**Recommendation:** Extract `NullDashboardEventBroadcaster` to `src/ZB.MOM.WW.MxGateway.Tests/TestSupport/NullDashboardEventBroadcaster.cs` (or a single `DashboardTestDoubles.cs`), delete both nested copies, and update the two `using`-bearing files to import from `TestSupport`.
|
|
|
|
**Recommendation:** Extract `NullDashboardEventBroadcaster` to `src/ZB.MOM.WW.MxGateway.Tests/TestSupport/NullDashboardEventBroadcaster.cs` (or a single `DashboardTestDoubles.cs`), delete both nested copies, and update the two `using`-bearing files to import from `TestSupport`.
|
|
|
|
|
|
|
|
|
|
|
|
**Resolution:** _(empty until closed)_
|
|
|
|
**Resolution:** 2026-05-24 — Extracted the shared no-op broadcaster to `src/ZB.MOM.WW.MxGateway.Tests/TestSupport/NullDashboardEventBroadcaster.cs` (single `public sealed` class with the singleton `Instance` field and a private constructor — matches the Tests-007 / Tests-021 consolidation pattern). Deleted both nested duplicates (`EventStreamServiceTests.cs:319-323` and `GatewayEndToEndFakeWorkerSmokeTests.cs:417-421`); the latter already imported `ZB.MOM.WW.MxGateway.Tests.TestSupport` so `NullDashboardEventBroadcaster.Instance` resolves against the shared type. `EventStreamServiceTests.cs` gained a `using ZB.MOM.WW.MxGateway.Tests.TestSupport;`. The integration-tests copy in `src/ZB.MOM.WW.MxGateway.IntegrationTests/WorkerLiveMxAccessSmokeTests.cs` was left alone (different module, per scope). Server-041's `ThrowingDashboardEventBroadcaster` remains nested in `EventStreamServiceTests` (single-file usage, no consolidation needed). Build clean (0 warnings), full Tests suite green (486/486).
|
|
|
|
|
|
|
|
|
|
|
|
### Tests-026
|
|
|
|
### Tests-026
|
|
|
|
|
|
|
|
|
|
|
@@ -444,10 +444,10 @@ The cancellation tests for `WorkerClient` in `WorkerClientTests` *do* exercise t
|
|
|
|
| Severity | Medium |
|
|
|
|
| Severity | Medium |
|
|
|
|
| Category | Testing coverage |
|
|
|
|
| Category | Testing coverage |
|
|
|
|
| Location | `src/ZB.MOM.WW.MxGateway.Tests/Gateway/Grpc/EventStreamServiceTests.cs`, `src/ZB.MOM.WW.MxGateway.Server/Grpc/EventStreamService.cs:123-126` |
|
|
|
|
| Location | `src/ZB.MOM.WW.MxGateway.Tests/Gateway/Grpc/EventStreamServiceTests.cs`, `src/ZB.MOM.WW.MxGateway.Server/Grpc/EventStreamService.cs:123-126` |
|
|
|
|
| Status | Open |
|
|
|
|
| Status | Resolved |
|
|
|
|
|
|
|
|
|
|
|
|
**Description:** The new `IDashboardEventBroadcaster` is wired into `EventStreamService` at line 123 (commit `d692232`) and the broadcaster's `Publish` is the only path that mirrors per-session events into the dashboard `EventsHub`. The unit tests inject `NullDashboardEventBroadcaster.Instance`, so the broadcaster invocation is never observed — a regression that silently dropped the `Publish` call (e.g. an `if` accidentally added around it, or removing the broadcaster ctor parameter) would not be caught by any test in this module. The hub-registration tests (`DashboardHubsRegistrationTests`) verify the endpoints exist but not the producer-side hook.
|
|
|
|
**Description:** The new `IDashboardEventBroadcaster` is wired into `EventStreamService` at line 123 (commit `d692232`) and the broadcaster's `Publish` is the only path that mirrors per-session events into the dashboard `EventsHub`. The unit tests inject `NullDashboardEventBroadcaster.Instance`, so the broadcaster invocation is never observed — a regression that silently dropped the `Publish` call (e.g. an `if` accidentally added around it, or removing the broadcaster ctor parameter) would not be caught by any test in this module. The hub-registration tests (`DashboardHubsRegistrationTests`) verify the endpoints exist but not the producer-side hook.
|
|
|
|
|
|
|
|
|
|
|
|
**Recommendation:** Add a fixture to `EventStreamServiceTests` named e.g. `StreamEventsAsync_PublishesEachEventToDashboardBroadcaster`: inject a recording fake that captures `(sessionId, mxEvent)` calls, push two events through the fake session, and assert the broadcaster received both with the correct session id and matching `WorkerSequence`. This pins the broadcast hook and proves the dashboard event mirror is not a no-op.
|
|
|
|
**Recommendation:** Add a fixture to `EventStreamServiceTests` named e.g. `StreamEventsAsync_PublishesEachEventToDashboardBroadcaster`: inject a recording fake that captures `(sessionId, mxEvent)` calls, push two events through the fake session, and assert the broadcaster received both with the correct session id and matching `WorkerSequence`. This pins the broadcast hook and proves the dashboard event mirror is not a no-op.
|
|
|
|
|
|
|
|
|
|
|
|
**Resolution:** _(empty until closed)_
|
|
|
|
**Resolution:** 2026-05-24 — Added `src/ZB.MOM.WW.MxGateway.Tests/TestSupport/RecordingDashboardEventBroadcaster.cs` — a thread-safe `IDashboardEventBroadcaster` test double backed by a `ConcurrentQueue<DashboardEventCapture>` that captures every `(sessionId, mxEvent)` invocation. Added `StreamEventsAsync_PublishesEachEventToDashboardBroadcaster` to `EventStreamServiceTests`: pushes two events (`WorkerSequence` 7 / `OnDataChange` and 8 / `OnWriteComplete`) through the fake session, drains the stream, and asserts the recording broadcaster captured exactly two publishes with the matching `SessionId`, `WorkerSequence`, and `Family` for each. TDD red/green confirmed: a deliberate "expected 3 captures" failed (`Expected: 3, Actual: 2`) before flipping to the correct count; the green run passes deterministically. The fixture would have caught a regression that drops or wraps `dashboardEventBroadcaster.Publish` at `EventStreamService.cs:133`. Build clean (0 warnings); full Tests suite green (486/486).
|
|
|
|