Resolve Tests-025..026
Tests-025 (Conventions): Extracted the previously-duplicated NullDashboardEventBroadcaster into TestSupport/NullDashboardEventBroadcaster.cs (singleton Instance, private ctor). The two nested copies in EventStreamServiceTests and GatewayEndToEndFakeWorkerSmokeTests were removed; both files now use the shared type via 'using ZB.MOM.WW.MxGateway.Tests.TestSupport;'. The Server-041 regression test's ThrowingDashboardEventBroadcaster is intentionally left nested — single-file usage doesn't warrant promotion to TestSupport. The third copy in IntegrationTests/WorkerLiveMxAccessSmokeTests was handled by IntegrationTests-024 in its own commit. Tests-026 (Testing coverage): Added a new RecordingDashboardEventBroadcaster test double in TestSupport — a thread-safe (ConcurrentQueue<DashboardEventCapture>) recorder. New fixture StreamEventsAsync_PublishesEachEventToDashboardBroadcaster in EventStreamServiceTests pushes two events through the fake session and asserts the broadcaster received both with the correct sessionId and WorkerSequence. TDD red→green confirmed: the deliberately-wrong "Expected 3, Actual 2" red phase proved the recording fake was actually invoked by the production code path. Verification: 486/486 server tests passing (485 previous + 1 new). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Joseph Doherty
@@ -7,7 +7,7 @@
|
||||
| Review date | 2026-05-24 |
|
||||
| Commit reviewed | `d692232` |
|
||||
| Status | Reviewed |
|
||||
| Open findings | 2 |
|
||||
| Open findings | 0 |
|
||||
|
||||
## Checklist coverage
|
||||
|
||||
@@ -429,13 +429,13 @@ The cancellation tests for `WorkerClient` in `WorkerClientTests` *do* exercise t
|
||||
| Severity | Low |
|
||||
| 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` |
|
||||
| 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.
|
||||
|
||||
**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
|
||||
|
||||
@@ -444,10 +444,10 @@ The cancellation tests for `WorkerClient` in `WorkerClientTests` *do* exercise t
|
||||
| Severity | Medium |
|
||||
| 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` |
|
||||
| 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.
|
||||
|
||||
**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).
|
||||
|
||||
@@ -414,9 +414,4 @@ public sealed class GatewayEndToEndFakeWorkerSmokeTests
|
||||
}
|
||||
}
|
||||
|
||||
private sealed class NullDashboardEventBroadcaster : ZB.MOM.WW.MxGateway.Server.Dashboard.Hubs.IDashboardEventBroadcaster
|
||||
{
|
||||
public static readonly NullDashboardEventBroadcaster Instance = new();
|
||||
public void Publish(string sessionId, ZB.MOM.WW.MxGateway.Contracts.Proto.MxEvent mxEvent) { }
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,26 @@
|
||||
using ZB.MOM.WW.MxGateway.Contracts.Proto;
|
||||
using ZB.MOM.WW.MxGateway.Server.Dashboard.Hubs;
|
||||
|
||||
namespace ZB.MOM.WW.MxGateway.Tests.TestSupport;
|
||||
|
||||
/// <summary>
|
||||
/// <see cref="IDashboardEventBroadcaster"/> that drops every event on the floor.
|
||||
/// Used by tests that need to wire up <c>EventStreamService</c> but do not care
|
||||
/// about the dashboard fan-out side-channel. The singleton <see cref="Instance"/>
|
||||
/// mirrors the Tests-007 / Tests-021 shared-fake pattern under
|
||||
/// <c>src/ZB.MOM.WW.MxGateway.Tests/TestSupport/</c>.
|
||||
/// </summary>
|
||||
public sealed class NullDashboardEventBroadcaster : IDashboardEventBroadcaster
|
||||
{
|
||||
/// <summary>Shared instance for tests that do not need to inspect broadcaster state.</summary>
|
||||
public static readonly NullDashboardEventBroadcaster Instance = new();
|
||||
|
||||
private NullDashboardEventBroadcaster()
|
||||
{
|
||||
}
|
||||
|
||||
/// <inheritdoc />
|
||||
public void Publish(string sessionId, MxEvent mxEvent)
|
||||
{
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,30 @@
|
||||
using System.Collections.Concurrent;
|
||||
using ZB.MOM.WW.MxGateway.Contracts.Proto;
|
||||
using ZB.MOM.WW.MxGateway.Server.Dashboard.Hubs;
|
||||
|
||||
namespace ZB.MOM.WW.MxGateway.Tests.TestSupport;
|
||||
|
||||
/// <summary>
|
||||
/// <see cref="IDashboardEventBroadcaster"/> test double that captures every
|
||||
/// <c>Publish(sessionId, mxEvent)</c> call into a thread-safe list, so tests
|
||||
/// can prove the gRPC producer loop actually mirrors events to the dashboard
|
||||
/// fan-out seam. Tests-026.
|
||||
/// </summary>
|
||||
public sealed class RecordingDashboardEventBroadcaster : IDashboardEventBroadcaster
|
||||
{
|
||||
private readonly ConcurrentQueue<DashboardEventCapture> _captures = new();
|
||||
|
||||
/// <summary>Gets a snapshot of every <c>Publish</c> call in arrival order.</summary>
|
||||
public IReadOnlyList<DashboardEventCapture> Captures => _captures.ToArray();
|
||||
|
||||
/// <inheritdoc />
|
||||
public void Publish(string sessionId, MxEvent mxEvent)
|
||||
{
|
||||
_captures.Enqueue(new DashboardEventCapture(sessionId, mxEvent));
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>Captured <c>(sessionId, mxEvent)</c> pair from a Publish invocation.</summary>
|
||||
/// <param name="SessionId">The session id passed to Publish.</param>
|
||||
/// <param name="MxEvent">The MxEvent passed to Publish.</param>
|
||||
public sealed record DashboardEventCapture(string SessionId, MxEvent MxEvent);
|
||||
Reference in New Issue
Block a user