diff --git a/code-reviews/Tests/findings.md b/code-reviews/Tests/findings.md index 63f180e..f624b1c 100644 --- a/code-reviews/Tests/findings.md +++ b/code-reviews/Tests/findings.md @@ -7,7 +7,7 @@ | Review date | 2026-05-18 | | Commit reviewed | `6c64030` | | Status | Reviewed | -| Open findings | 6 | +| Open findings | 0 | ## Checklist coverage @@ -127,13 +127,13 @@ | Severity | Low | | Category | Code organization & conventions | | Location | `src/MxGateway.Tests/Gateway/Grpc/MxAccessGatewayServiceTests.cs:682`, `src/MxGateway.Tests/Gateway/Grpc/GalaxyRepositoryGrpcServiceTests.cs:324`, `src/MxGateway.Tests/Gateway/GatewayEndToEndFakeWorkerSmokeTests.cs:460`, `src/MxGateway.Tests/Security/Authorization/GatewayGrpcAuthorizationInterceptorTests.cs:233` | -| Status | Open | +| Status | Resolved | **Description:** A near-identical `TestServerCallContext` implementation is copy-pasted into at least four test files (and `AllowAllConstraintEnforcer` / `TestServerStreamWriter` / `RecordingStreamWriter` into several). Duplication risks the copies drifting and bloats each file. **Recommendation:** Extract a shared `TestServerCallContext`, `RecordingServerStreamWriter`, and `AllowAllConstraintEnforcer` into a common test-support folder/namespace. -**Resolution:** _(open)_ +**Resolution:** Resolved 2026-05-18: confirmed five duplicated copies (the brief's four plus a fifth in `Galaxy/GalaxyFilterInputSafetyTests.cs`). Added a shared `MxGateway.Tests.TestSupport` namespace under `src/MxGateway.Tests/TestSupport/`: `TestServerCallContext.cs` (single class with an optional `Metadata? requestHeaders` constructor parameter that subsumes both the no-arg and headers-bearing variants), `RecordingServerStreamWriter.cs` (thread-safe writer with `Messages` and `WaitForFirstMessageAsync`, replacing `TestServerStreamWriter`/`RecordingStreamWriter`/`RecordingServerStreamWriter`), and `AllowAllConstraintEnforcer.cs`. Deleted all five `TestServerCallContext` copies, both `AllowAllConstraintEnforcer` copies, and the three stream-writer copies; updated the five test files to `using MxGateway.Tests.TestSupport;` and renamed `.Items` call sites to `.Messages`. Removed the now-unused `Grpc.Core` using from `GatewayEndToEndFakeWorkerSmokeTests.cs`. Build clean (0 warnings) and suite green. ### Tests-008 @@ -142,13 +142,15 @@ | Severity | Low | | Category | mxaccessgw conventions | | Location | `src/MxGateway.Tests/Gateway/Sessions/WorkerAlarmRpcDispatcherTests.cs:1-9`, `src/MxGateway.Tests/Gateway/Sessions/NotWiredAlarmRpcDispatcherTests.cs:1-3`, `src/MxGateway.Tests/Gateway/Sessions/SessionManagerAlarmAutoSubscribeTests.cs:1` | -| Status | Open | +| Status | Resolved | **Description:** The alarm test files diverge from the project's C# style and the rest of the suite: snake_case test method names instead of the PascalCase `Method_Condition_Result` pattern; redundant explicit `using System;`/`System.Threading;` imports despite implicit global usings; and explicit-type `new` instead of target-typed `new()` used elsewhere. There is also a typo in fixture data (`"wnwrap subscribe failed"`). **Recommendation:** Rename the alarm tests to the house `Method_Condition_Result` convention, drop redundant `System.*` usings, align `new` usage, and fix the `wnwrap` typo. -**Resolution:** _(open)_ +**Re-triage note:** Two of the finding's claims are incorrect. (1) `"wnwrap subscribe failed"` is **not a typo** — `WnWrap` is the real name of the worker's `WnWrapAlarmConsumer` MXAccess component (`src/MxGateway.Worker/MxAccess/WnWrapAlarmConsumer.cs`); the fixture string deliberately references it, so it was left unchanged. (2) `SessionManagerAlarmAutoSubscribeTests.cs` already uses PascalCase `Method_Condition_Result` names and target-typed `new()`, and its lone `using System.Runtime.CompilerServices;` is **required** for `[EnumeratorCancellation]` (not a global using) — it is not redundant. That file needed no change. The genuine style drift was confined to `WorkerAlarmRpcDispatcherTests.cs` and `NotWiredAlarmRpcDispatcherTests.cs`. + +**Resolution:** Resolved 2026-05-18: renamed all ten `WorkerAlarmRpcDispatcherTests` methods and both `NotWiredAlarmRpcDispatcherTests` methods from snake_case to the house `Method_Condition_Result` PascalCase convention; dropped the redundant `System`/`System.Collections.Generic`/`System.Linq`/`System.Threading`/`System.Threading.Tasks` usings from `WorkerAlarmRpcDispatcherTests.cs` and `System.Threading`/`System.Threading.Tasks` from `NotWiredAlarmRpcDispatcherTests.cs` (all are implicit global usings), keeping the required `System.Runtime.CompilerServices`; converted explicit-type `new SessionRegistry()`/`new WorkerAlarmRpcDispatcher(...)`/`new FakeAlarmWorkerClient`/`new List<...>()`/`new GatewaySession(...)` to target-typed `new()`; and replaced the fully-qualified `System.StringComparison` with `StringComparison`. See the re-triage note for the two claims not actioned. Suite green. ### Tests-009 @@ -157,13 +159,13 @@ | Severity | Low | | Category | Documentation & comments | | Location | `src/MxGateway.Tests/Gateway/Sessions/SessionManagerTests.cs:36-37,99,365` | -| Status | Open | +| Status | Resolved | **Description:** Several XML `` comments are copy-paste mismatches: the comment above `OpenSessionAsync_SetsInitialDefaultLease` describes correlation-ID generation; the comment above `GatewaySessionSubscribeBulkAsync_ForwardsOneBulkCommand…` describes lease refresh; the comment above `CloseExpiredLeasesAsync_DoesNotCloseActiveEventSubscriber` describes shutdown closing all sessions. Misleading test docs hinder triage. **Recommendation:** Correct the `` text to match each test's actual behavior, or remove the redundant comments since the test names already describe the behavior. -**Resolution:** _(open)_ +**Resolution:** Resolved 2026-05-18: confirmed three copy-paste `` mismatches. The mislabelled comments were the summaries of the *following* tests left attached to the wrong method (the test below each then had no summary). Corrected all three: `OpenSessionAsync_SetsInitialDefaultLease` now describes setting the initial lease expiry; the comment above `InvokeAsync_WhenSessionReady_RefreshesLease` (the finding mis-cited the method name as `GatewaySessionSubscribeBulkAsync_…`) now describes lease refresh on invoke; and `CloseExpiredLeasesAsync_DoesNotCloseActiveEventSubscriber` now describes the expired-lease sweep leaving an active-event-subscriber session open. No behavior change. ### Tests-010 @@ -172,13 +174,13 @@ | Severity | Low | | Category | Security | | Location | `src/MxGateway.Tests/Gateway/Dashboard/DashboardAuthorizationHandlerTests.cs:26-36` | -| Status | Open | +| Status | Resolved | **Description:** The anonymous-localhost bypass is tested only for the success case (`allowAnonymousLocalhost: true` + loopback succeeds) and the remote-unauthenticated denial. There is no test for the security-critical negatives: anonymous + loopback when `AllowAnonymousLocalhost` is `false` must be denied, and anonymous + non-loopback when the flag is `true` must still be denied (the bypass is scoped strictly to loopback). Those are the misconfiguration cases that would expose the dashboard. **Recommendation:** Add tests: anonymous + loopback + `allowAnonymousLocalhost: false` → not succeeded; anonymous + non-loopback + `allowAnonymousLocalhost: true` → not succeeded. -**Resolution:** _(open)_ +**Resolution:** Resolved 2026-05-18: confirmed the coverage gap and confirmed `DashboardAuthorizationHandler` already gates the bypass correctly on `AllowAnonymousLocalhost && IsLoopbackRequest()` (no product bug). Added two `DashboardAuthorizationHandlerTests`: `HandleAsync_AnonymousLocalhostDisallowed_DoesNotSucceed` (anonymous + loopback + `allowAnonymousLocalhost: false` → not succeeded) and `HandleAsync_AnonymousLocalhostAllowedFromRemoteAddress_DoesNotSucceed` (anonymous + non-loopback + `allowAnonymousLocalhost: true` → not succeeded, proving the bypass stays scoped to loopback). Both pass. ### Tests-011 @@ -187,13 +189,13 @@ | Severity | Low | | Category | Correctness & logic bugs | | Location | `src/MxGateway.Tests/Gateway/GatewayEndToEndFakeWorkerSmokeTests.cs:233-301` | -| Status | Open | +| Status | Resolved | **Description:** `GatewayEndToEndFakeWorkerSmokeTests` correctly stores and awaits `launcher.WorkerTask`, but `SessionWorkerClientFactoryFakeWorkerTests` uses `_ = RunWorkerAsync(...)` with no stored task (lines 152, 184, 220). An unhandled exception in the scripted worker becomes an unobserved `TaskException` that can surface as a process-level failure in an unrelated later test rather than failing the owning test. **Recommendation:** Store the worker task and either await it during disposal or attach a continuation that fails the test on fault, mirroring `GatewayEndToEndFakeWorkerSmokeTests`. -**Resolution:** _(open)_ +**Resolution:** Resolved 2026-05-18: confirmed all three scripted launchers in `SessionWorkerClientFactoryFakeWorkerTests` discarded the worker task. Added an `IWorkerTaskLauncher` interface (each launcher now stores its scripted task in a `WorkerTask` property and exposes `ObserveWorkerTaskAsync`); the test class now implements `IAsyncDisposable`, tracks every launcher it creates via a `Track` helper, and in `DisposeAsync` awaits each `WorkerTask` (within `TestTimeout`) so a scripted-worker fault fails the owning test instead of leaking as an unobserved `TaskScheduler.UnobservedTaskException`. `OperationCanceledException` and `IOException` — the expected outcomes of the worker client tearing the pipe down — are swallowed; anything else rethrows. `NeverReadyWorkerProcessLauncher` (which parks on an infinite `Task.Delay`) was given its own `CancellationTokenSource` so disposal can cancel and observe the parked task. Suite green. ### Tests-012 @@ -202,10 +204,10 @@ | Severity | Low | | Category | Concurrency & thread safety | | Location | `src/MxGateway.Tests/Gateway/Workers/Fakes/FakeWorkerHarness.cs:62`, `src/MxGateway.Tests/Gateway/Workers/WorkerClientTests.cs:472` | -| Status | Open | +| Status | Resolved | **Description:** Pipe names are uniquified per test with a GUID (good), but xUnit runs test classes in parallel by default and there is no `xunit.runner.json` or collection configuration. Tests that build a full `WebApplication` bind ephemeral ports (`--urls=http://127.0.0.1:0`, fine) but spin up DI containers and hosted services concurrently. Currently safe, but a future test binding a fixed port would silently collide. **Recommendation:** Add an `xunit.runner.json` or a collection grouping the `WebApplication`-building tests, and keep the `:0` ephemeral-port convention explicit so future tests do not introduce a fixed-port collision. -**Resolution:** _(open)_ +**Resolution:** Resolved 2026-05-18: added `src/MxGateway.Tests/xunit.runner.json` making the parallelism policy explicit (`parallelizeTestCollections: true`, `maxParallelThreads: -1`, `parallelizeAssembly: false`, `longRunningTestSeconds: 30`) and wired it into `MxGateway.Tests.csproj` as `` so the runner picks it up (confirmed present in `bin/Debug/net10.0/`). Added a comment at the only `WebApplication`-building call site (`GatewayApplicationTests.cs`, `--urls=http://127.0.0.1:0`) documenting that the ephemeral-port (`:0`) convention is mandatory because test collections run in parallel. No fixed-port binding exists today; this is a preventative guardrail as the finding recommends. diff --git a/src/MxGateway.Tests/Galaxy/GalaxyFilterInputSafetyTests.cs b/src/MxGateway.Tests/Galaxy/GalaxyFilterInputSafetyTests.cs index eb1a2b5..769b1eb 100644 --- a/src/MxGateway.Tests/Galaxy/GalaxyFilterInputSafetyTests.cs +++ b/src/MxGateway.Tests/Galaxy/GalaxyFilterInputSafetyTests.cs @@ -6,6 +6,7 @@ using MxGateway.Server.Dashboard; using MxGateway.Server.Galaxy; using MxGateway.Server.Grpc; using MxGateway.Server.Security.Authorization; +using MxGateway.Tests.TestSupport; namespace MxGateway.Tests.Galaxy; @@ -302,51 +303,4 @@ public sealed class GalaxyFilterInputSafetyTests public Task WaitForFirstLoadAsync(CancellationToken cancellationToken) => Task.CompletedTask; } - private sealed class TestServerCallContext(CancellationToken cancellationToken = default) : ServerCallContext - { - private readonly Metadata requestHeaders = []; - private readonly Metadata responseTrailers = []; - private readonly Dictionary userState = []; - private Status status; - private WriteOptions? writeOptions; - - protected override string MethodCore => "/galaxy_repository.v1.GalaxyRepository/DiscoverHierarchy"; - - protected override string HostCore => "localhost"; - - protected override string PeerCore => "ipv4:127.0.0.1:5000"; - - protected override DateTime DeadlineCore => DateTime.UtcNow.AddMinutes(1); - - protected override Metadata RequestHeadersCore => requestHeaders; - - protected override CancellationToken CancellationTokenCore => cancellationToken; - - protected override Metadata ResponseTrailersCore => responseTrailers; - - protected override Status StatusCore - { - get => status; - set => status = value; - } - - protected override WriteOptions? WriteOptionsCore - { - get => writeOptions; - set => writeOptions = value; - } - - protected override AuthContext AuthContextCore { get; } = new( - string.Empty, - new Dictionary>(StringComparer.Ordinal)); - - protected override IDictionary UserStateCore => userState; - - protected override Task WriteResponseHeadersAsyncCore(Metadata responseHeaders) => Task.CompletedTask; - - protected override ContextPropagationToken CreatePropagationTokenCore(ContextPropagationOptions? options) - { - throw new NotSupportedException(); - } - } } diff --git a/src/MxGateway.Tests/Gateway/Dashboard/DashboardAuthorizationHandlerTests.cs b/src/MxGateway.Tests/Gateway/Dashboard/DashboardAuthorizationHandlerTests.cs index cde3b0b..dd7c831 100644 --- a/src/MxGateway.Tests/Gateway/Dashboard/DashboardAuthorizationHandlerTests.cs +++ b/src/MxGateway.Tests/Gateway/Dashboard/DashboardAuthorizationHandlerTests.cs @@ -35,6 +35,36 @@ public sealed class DashboardAuthorizationHandlerTests Assert.True(context.HasSucceeded); } + /// + /// Verifies that the anonymous-localhost bypass is denied when AllowAnonymousLocalhost + /// is off, even on a loopback connection — the misconfiguration must not expose the dashboard. + /// + [Fact] + public async Task HandleAsync_AnonymousLocalhostDisallowed_DoesNotSucceed() + { + AuthorizationHandlerContext context = await AuthorizeAsync( + new ClaimsPrincipal(new ClaimsIdentity()), + IPAddress.Loopback, + allowAnonymousLocalhost: false); + + Assert.False(context.HasSucceeded); + } + + /// + /// Verifies that the anonymous-localhost bypass stays scoped to loopback: an anonymous + /// request from a non-loopback address is denied even when AllowAnonymousLocalhost is on. + /// + [Fact] + public async Task HandleAsync_AnonymousLocalhostAllowedFromRemoteAddress_DoesNotSucceed() + { + AuthorizationHandlerContext context = await AuthorizeAsync( + new ClaimsPrincipal(new ClaimsIdentity()), + IPAddress.Parse("10.0.0.5"), + allowAnonymousLocalhost: true); + + Assert.False(context.HasSucceeded); + } + /// Verifies that authenticated users without admin scope fail authorization. [Fact] public async Task HandleAsync_AuthenticatedWithoutAdminScope_DoesNotSucceed() diff --git a/src/MxGateway.Tests/Gateway/GatewayApplicationTests.cs b/src/MxGateway.Tests/Gateway/GatewayApplicationTests.cs index 91f98bc..d1601ce 100644 --- a/src/MxGateway.Tests/Gateway/GatewayApplicationTests.cs +++ b/src/MxGateway.Tests/Gateway/GatewayApplicationTests.cs @@ -147,6 +147,8 @@ public sealed class GatewayApplicationTests string value, string expectedFailure) { + // Bind an ephemeral port (:0) — xUnit runs test collections in parallel, so any + // WebApplication-building test must avoid a fixed port to prevent a bind collision. await using WebApplication app = GatewayApplication.Build( [$"--{key}={value}", "--urls=http://127.0.0.1:0"]); diff --git a/src/MxGateway.Tests/Gateway/GatewayEndToEndFakeWorkerSmokeTests.cs b/src/MxGateway.Tests/Gateway/GatewayEndToEndFakeWorkerSmokeTests.cs index bb0c627..8871764 100644 --- a/src/MxGateway.Tests/Gateway/GatewayEndToEndFakeWorkerSmokeTests.cs +++ b/src/MxGateway.Tests/Gateway/GatewayEndToEndFakeWorkerSmokeTests.cs @@ -1,6 +1,5 @@ using System.Collections.Concurrent; using Google.Protobuf.WellKnownTypes; -using Grpc.Core; using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.Options; using MxGateway.Contracts; @@ -13,6 +12,7 @@ using MxGateway.Server.Security.Authorization; using MxGateway.Server.Sessions; using MxGateway.Server.Workers; using MxGateway.Tests.Gateway.Workers.Fakes; +using MxGateway.Tests.TestSupport; namespace MxGateway.Tests.Gateway; @@ -405,159 +405,4 @@ public sealed class GatewayEndToEndFakeWorkerSmokeTests } } - private sealed class RecordingServerStreamWriter : IServerStreamWriter - { - private readonly object _syncRoot = new(); - private readonly TaskCompletionSource _firstMessage = new(TaskCreationOptions.RunContinuationsAsynchronously); - private readonly List _messages = []; - - /// - /// Gets the recorded messages written to this stream. - /// - public IReadOnlyList Messages - { - get - { - lock (_syncRoot) - { - return _messages.ToArray(); - } - } - } - - /// - /// Gets or sets options for writing messages to the stream. - /// - public WriteOptions? WriteOptions { get; set; } - - /// - /// Writes a message to the stream asynchronously. - /// - /// The message to write. - /// Completed task. - public Task WriteAsync(T message) - { - lock (_syncRoot) - { - _messages.Add(message); - } - - _firstMessage.TrySetResult(message); - return Task.CompletedTask; - } - - /// - /// Waits for the first message to be written within the specified timeout. - /// - /// Maximum time to wait for the first message. - /// The first message written to this stream. - public async Task WaitForFirstMessageAsync(TimeSpan timeout) - { - return await _firstMessage.Task.WaitAsync(timeout).ConfigureAwait(false); - } - } - - private sealed class TestServerCallContext(CancellationToken cancellationToken = default) : ServerCallContext - { - private readonly Metadata _requestHeaders = []; - private readonly Metadata _responseTrailers = []; - private readonly Dictionary _userState = []; - private Status _status; - private WriteOptions? _writeOptions; - - /// - protected override string MethodCore => "/mxaccess_gateway.v1.MxAccessGateway/Test"; - - /// - protected override string HostCore => "localhost"; - - /// - protected override string PeerCore => "ipv4:127.0.0.1:5000"; - - /// - protected override DateTime DeadlineCore => DateTime.UtcNow.AddMinutes(1); - - /// - protected override Metadata RequestHeadersCore => _requestHeaders; - - /// - protected override CancellationToken CancellationTokenCore => cancellationToken; - - /// - protected override Metadata ResponseTrailersCore => _responseTrailers; - - /// - protected override Status StatusCore - { - get => _status; - set => _status = value; - } - - /// - protected override WriteOptions? WriteOptionsCore - { - get => _writeOptions; - set => _writeOptions = value; - } - - /// - protected override AuthContext AuthContextCore { get; } = new( - string.Empty, - new Dictionary>(StringComparer.Ordinal)); - - /// - protected override IDictionary UserStateCore => _userState; - - /// - /// Writes response headers asynchronously. - /// - /// Headers to write. - /// Completed task. - /// - protected override Task WriteResponseHeadersAsyncCore(Metadata responseHeaders) - { - return Task.CompletedTask; - } - - /// - /// Creates a context propagation token with the specified options. - /// - /// Propagation options. - /// Propagation token. - /// - protected override ContextPropagationToken CreatePropagationTokenCore( - ContextPropagationOptions? options) - { - throw new NotSupportedException(); - } - } - - private sealed class AllowAllConstraintEnforcer : IConstraintEnforcer - { - public Task CheckReadTagAsync( - ApiKeyIdentity? identity, - string tagAddress, - CancellationToken cancellationToken) => Task.FromResult(null); - - public Task CheckReadHandleAsync( - ApiKeyIdentity? identity, - GatewaySession session, - int serverHandle, - int itemHandle, - CancellationToken cancellationToken) => Task.FromResult(null); - - public Task CheckWriteHandleAsync( - ApiKeyIdentity? identity, - GatewaySession session, - int serverHandle, - int itemHandle, - CancellationToken cancellationToken) => Task.FromResult(null); - - public Task RecordDenialAsync( - ApiKeyIdentity? identity, - string commandKind, - string target, - ConstraintFailure failure, - CancellationToken cancellationToken) => Task.CompletedTask; - } } diff --git a/src/MxGateway.Tests/Gateway/Grpc/GalaxyRepositoryGrpcServiceTests.cs b/src/MxGateway.Tests/Gateway/Grpc/GalaxyRepositoryGrpcServiceTests.cs index 4388d9d..6bd4165 100644 --- a/src/MxGateway.Tests/Gateway/Grpc/GalaxyRepositoryGrpcServiceTests.cs +++ b/src/MxGateway.Tests/Gateway/Grpc/GalaxyRepositoryGrpcServiceTests.cs @@ -5,6 +5,7 @@ using MxGateway.Server.Dashboard; using MxGateway.Server.Galaxy; using MxGateway.Server.Grpc; using MxGateway.Server.Security.Authorization; +using MxGateway.Tests.TestSupport; namespace MxGateway.Tests.Gateway.Grpc; @@ -321,51 +322,4 @@ public sealed class GalaxyRepositoryGrpcServiceTests public Task WaitForFirstLoadAsync(CancellationToken cancellationToken) => Task.CompletedTask; } - private sealed class TestServerCallContext(CancellationToken cancellationToken = default) : ServerCallContext - { - private readonly Metadata requestHeaders = []; - private readonly Metadata responseTrailers = []; - private readonly Dictionary userState = []; - private Status status; - private WriteOptions? writeOptions; - - protected override string MethodCore => "/galaxy_repository.v1.GalaxyRepository/DiscoverHierarchy"; - - protected override string HostCore => "localhost"; - - protected override string PeerCore => "ipv4:127.0.0.1:5000"; - - protected override DateTime DeadlineCore => DateTime.UtcNow.AddMinutes(1); - - protected override Metadata RequestHeadersCore => requestHeaders; - - protected override CancellationToken CancellationTokenCore => cancellationToken; - - protected override Metadata ResponseTrailersCore => responseTrailers; - - protected override Status StatusCore - { - get => status; - set => status = value; - } - - protected override WriteOptions? WriteOptionsCore - { - get => writeOptions; - set => writeOptions = value; - } - - protected override AuthContext AuthContextCore { get; } = new( - string.Empty, - new Dictionary>(StringComparer.Ordinal)); - - protected override IDictionary UserStateCore => userState; - - protected override Task WriteResponseHeadersAsyncCore(Metadata responseHeaders) => Task.CompletedTask; - - protected override ContextPropagationToken CreatePropagationTokenCore(ContextPropagationOptions? options) - { - throw new NotSupportedException(); - } - } } diff --git a/src/MxGateway.Tests/Gateway/Grpc/MxAccessGatewayServiceTests.cs b/src/MxGateway.Tests/Gateway/Grpc/MxAccessGatewayServiceTests.cs index 1003962..85caa76 100644 --- a/src/MxGateway.Tests/Gateway/Grpc/MxAccessGatewayServiceTests.cs +++ b/src/MxGateway.Tests/Gateway/Grpc/MxAccessGatewayServiceTests.cs @@ -11,6 +11,7 @@ using MxGateway.Server.Security.Authentication; using MxGateway.Server.Security.Authorization; using MxGateway.Server.Sessions; using MxGateway.Server.Workers; +using MxGateway.Tests.TestSupport; namespace MxGateway.Tests.Gateway.Grpc; @@ -132,7 +133,7 @@ public sealed class MxAccessGatewayServiceTests SessionId = "session-missing", AlarmFilterPrefix = "Tank01.", }, - new RecordingStreamWriter(), + new RecordingServerStreamWriter(), new TestServerCallContext())); Assert.Equal(StatusCode.NotFound, exception.StatusCode); @@ -225,7 +226,7 @@ public sealed class MxAccessGatewayServiceTests sessionManager.Events.Add(CreateWorkerEvent("session-1", workerSequence: 1)); sessionManager.Events.Add(CreateWorkerEvent("session-1", workerSequence: 2)); MxAccessGatewayService service = CreateService(sessionManager); - TestServerStreamWriter writer = new(); + RecordingServerStreamWriter writer = new(); await service.StreamEvents( new StreamEventsRequest @@ -276,7 +277,7 @@ public sealed class MxAccessGatewayServiceTests FakeSessionManager sessionManager = new(); sessionManager.Events.Add(CreateWorkerEvent("session-1", workerSequence: 2)); MxAccessGatewayService service = CreateService(sessionManager, metrics: metrics); - TestServerStreamWriter writer = new(); + RecordingServerStreamWriter writer = new(); await service.StreamEvents( new StreamEventsRequest { SessionId = "session-1" }, @@ -375,7 +376,7 @@ public sealed class MxAccessGatewayServiceTests RpcException exception = await Assert.ThrowsAsync( async () => await service.QueryActiveAlarms( new QueryActiveAlarmsRequest(), - new RecordingStreamWriter(), + new RecordingServerStreamWriter(), new TestServerCallContext())); Assert.Equal(StatusCode.InvalidArgument, exception.StatusCode); @@ -386,7 +387,7 @@ public sealed class MxAccessGatewayServiceTests public async Task QueryActiveAlarms_WithValidRequest_StreamsZeroSnapshots() { MxAccessGatewayService service = CreateService(new FakeSessionManager()); - RecordingStreamWriter sink = new(); + RecordingServerStreamWriter sink = new(); await service.QueryActiveAlarms( new QueryActiveAlarmsRequest @@ -397,7 +398,7 @@ public sealed class MxAccessGatewayServiceTests sink, new TestServerCallContext()); - Assert.Empty(sink.Items); + Assert.Empty(sink.Messages); } /// Verifies OpenSession advertises the alarm RPC capability strings. @@ -664,35 +665,6 @@ public sealed class MxAccessGatewayServiceTests } } - private sealed class AllowAllConstraintEnforcer : IConstraintEnforcer - { - public Task CheckReadTagAsync( - ApiKeyIdentity? identity, - string tagAddress, - CancellationToken cancellationToken) => Task.FromResult(null); - - public Task CheckReadHandleAsync( - ApiKeyIdentity? identity, - GatewaySession session, - int serverHandle, - int itemHandle, - CancellationToken cancellationToken) => Task.FromResult(null); - - public Task CheckWriteHandleAsync( - ApiKeyIdentity? identity, - GatewaySession session, - int serverHandle, - int itemHandle, - CancellationToken cancellationToken) => Task.FromResult(null); - - public Task RecordDenialAsync( - ApiKeyIdentity? identity, - string commandKind, - string target, - ConstraintFailure failure, - CancellationToken cancellationToken) => Task.CompletedTask; - } - private sealed class FakeWorkerClient(int processId) : IWorkerClient { /// @@ -750,97 +722,4 @@ public sealed class MxAccessGatewayServiceTests } } - private sealed class TestServerStreamWriter : IServerStreamWriter - { - /// - public List Messages { get; } = []; - - /// - public WriteOptions? WriteOptions { get; set; } - - /// - public Task WriteAsync(T message) - { - Messages.Add(message); - - return Task.CompletedTask; - } - } - - private sealed class RecordingStreamWriter : IServerStreamWriter - { - public List Items { get; } = new(); - public WriteOptions? WriteOptions { get; set; } - - public Task WriteAsync(T message) - { - Items.Add(message); - return Task.CompletedTask; - } - } - - private sealed class TestServerCallContext(CancellationToken cancellationToken = default) : ServerCallContext - { - private readonly Metadata requestHeaders = []; - private readonly Metadata responseTrailers = []; - private readonly Dictionary userState = []; - private Status status; - private WriteOptions? writeOptions; - - /// - protected override string MethodCore => "/mxaccess_gateway.v1.MxAccessGateway/Test"; - - /// - protected override string HostCore => "localhost"; - - /// - protected override string PeerCore => "ipv4:127.0.0.1:5000"; - - /// - protected override DateTime DeadlineCore => DateTime.UtcNow.AddMinutes(1); - - /// - protected override Metadata RequestHeadersCore => requestHeaders; - - /// - protected override CancellationToken CancellationTokenCore => cancellationToken; - - /// - protected override Metadata ResponseTrailersCore => responseTrailers; - - /// - protected override Status StatusCore - { - get => status; - set => status = value; - } - - /// - protected override WriteOptions? WriteOptionsCore - { - get => writeOptions; - set => writeOptions = value; - } - - /// - protected override AuthContext AuthContextCore { get; } = new( - string.Empty, - new Dictionary>(StringComparer.Ordinal)); - - /// - protected override IDictionary UserStateCore => userState; - - /// - protected override Task WriteResponseHeadersAsyncCore(Metadata responseHeaders) - { - return Task.CompletedTask; - } - - /// - protected override ContextPropagationToken CreatePropagationTokenCore( - ContextPropagationOptions? options) - { - throw new NotSupportedException(); - } - } } diff --git a/src/MxGateway.Tests/Gateway/Sessions/NotWiredAlarmRpcDispatcherTests.cs b/src/MxGateway.Tests/Gateway/Sessions/NotWiredAlarmRpcDispatcherTests.cs index d416110..18c0e8c 100644 --- a/src/MxGateway.Tests/Gateway/Sessions/NotWiredAlarmRpcDispatcherTests.cs +++ b/src/MxGateway.Tests/Gateway/Sessions/NotWiredAlarmRpcDispatcherTests.cs @@ -1,5 +1,3 @@ -using System.Threading; -using System.Threading.Tasks; using MxGateway.Contracts.Proto; using MxGateway.Server.Sessions; @@ -15,7 +13,7 @@ namespace MxGateway.Tests.Gateway.Sessions; public sealed class NotWiredAlarmRpcDispatcherTests { [Fact] - public async Task AcknowledgeAsync_returns_ok_with_worker_pending_diagnostic() + public async Task AcknowledgeAsync_WhenNotWired_ReturnsOkWithWorkerPendingDiagnostic() { IAlarmRpcDispatcher dispatcher = new NotWiredAlarmRpcDispatcher(); @@ -33,11 +31,11 @@ public sealed class NotWiredAlarmRpcDispatcherTests Assert.Equal(ProtocolStatusCode.Ok, reply.ProtocolStatus.Code); Assert.Equal("session-1", reply.SessionId); Assert.Equal("corr-1", reply.CorrelationId); - Assert.Contains("worker", reply.DiagnosticMessage, System.StringComparison.OrdinalIgnoreCase); + Assert.Contains("worker", reply.DiagnosticMessage, StringComparison.OrdinalIgnoreCase); } [Fact] - public async Task QueryActiveAlarmsAsync_yields_no_snapshots() + public async Task QueryActiveAlarmsAsync_WhenNotWired_YieldsNoSnapshots() { IAlarmRpcDispatcher dispatcher = new NotWiredAlarmRpcDispatcher(); diff --git a/src/MxGateway.Tests/Gateway/Sessions/SessionManagerTests.cs b/src/MxGateway.Tests/Gateway/Sessions/SessionManagerTests.cs index ae20bb5..342f69a 100644 --- a/src/MxGateway.Tests/Gateway/Sessions/SessionManagerTests.cs +++ b/src/MxGateway.Tests/Gateway/Sessions/SessionManagerTests.cs @@ -33,7 +33,7 @@ public sealed class SessionManagerTests Assert.Equal(1, metrics.GetSnapshot().SessionsOpened); } - /// Verifies that opening a session generates a correlation ID from the client name and session ID. + /// Verifies that opening a session sets the initial lease expiry from the configured default lease. [Fact] public async Task OpenSessionAsync_SetsInitialDefaultLease() { @@ -96,7 +96,7 @@ public sealed class SessionManagerTests Assert.Equal(MxCommandKind.Ping, reply.Reply.Kind); } - /// Verifies that bulk subscribe forwards the command and returns subscription results. + /// Verifies that invoking a command on a ready session refreshes its lease expiry. [Fact] public async Task InvokeAsync_WhenSessionReady_RefreshesLease() { @@ -362,7 +362,7 @@ public sealed class SessionManagerTests Assert.Equal(0, activeClient.ShutdownCount); } - /// Verifies that shutdown closes all registered sessions. + /// Verifies that an expired-lease sweep leaves a session with an active event subscriber open. [Fact] public async Task CloseExpiredLeasesAsync_DoesNotCloseActiveEventSubscriber() { diff --git a/src/MxGateway.Tests/Gateway/Sessions/SessionWorkerClientFactoryFakeWorkerTests.cs b/src/MxGateway.Tests/Gateway/Sessions/SessionWorkerClientFactoryFakeWorkerTests.cs index c9b132f..ceb75b3 100644 --- a/src/MxGateway.Tests/Gateway/Sessions/SessionWorkerClientFactoryFakeWorkerTests.cs +++ b/src/MxGateway.Tests/Gateway/Sessions/SessionWorkerClientFactoryFakeWorkerTests.cs @@ -10,15 +10,29 @@ using MxGateway.Tests.Gateway.Workers.Fakes; namespace MxGateway.Tests.Gateway.Sessions; -public sealed class SessionWorkerClientFactoryFakeWorkerTests +public sealed class SessionWorkerClientFactoryFakeWorkerTests : IAsyncDisposable { private static readonly TimeSpan TestTimeout = TimeSpan.FromSeconds(5); + private readonly List _launchers = []; + + /// + /// Awaits every scripted worker task so an unhandled exception fails the owning test + /// instead of surfacing later as an unobserved . + /// + public async ValueTask DisposeAsync() + { + foreach (IWorkerTaskLauncher launcher in _launchers) + { + await launcher.ObserveWorkerTaskAsync(TestTimeout); + } + } + /// Verifies that the factory creates a ready worker client with a scripted fake worker process. [Fact] public async Task CreateAsync_WithScriptedFakeWorker_ReturnsReadyClient() { - ScriptedFakeWorkerProcessLauncher launcher = new(); + ScriptedFakeWorkerProcessLauncher launcher = Track(new ScriptedFakeWorkerProcessLauncher()); using GatewayMetrics metrics = new(); SessionWorkerClientFactory factory = new( launcher, @@ -51,7 +65,7 @@ public sealed class SessionWorkerClientFactoryFakeWorkerTests [Fact] public async Task CreateAsync_WhenFakeWorkerStartupFails_ThrowsWorkerClientException() { - FailingStartupWorkerProcessLauncher launcher = new(); + FailingStartupWorkerProcessLauncher launcher = Track(new FailingStartupWorkerProcessLauncher()); using GatewayMetrics metrics = new(); SessionWorkerClientFactory factory = new( launcher, @@ -71,7 +85,7 @@ public sealed class SessionWorkerClientFactoryFakeWorkerTests [Fact] public async Task CreateAsync_WhenFakeWorkerNeverSendsReady_TimesOutAndKillsWorker() { - NeverReadyWorkerProcessLauncher launcher = new(); + NeverReadyWorkerProcessLauncher launcher = Track(new NeverReadyWorkerProcessLauncher()); using GatewayMetrics metrics = new(); SessionWorkerClientFactory factory = new( launcher, @@ -134,8 +148,50 @@ public sealed class SessionWorkerClientFactoryFakeWorkerTests }; } + private T Track(T launcher) + where T : IWorkerTaskLauncher + { + _launchers.Add(launcher); + + return launcher; + } + + /// + /// A fake worker launcher that runs a scripted worker on a background task and exposes + /// that task so the owning test observes it rather than leaking an unobserved fault. + /// + private interface IWorkerTaskLauncher : IWorkerProcessLauncher + { + /// + /// Awaits the scripted worker task within the timeout, swallowing only the pipe + /// teardown faults expected when the worker client kills or disposes the worker. + /// + /// Maximum time to wait for the worker task. + Task ObserveWorkerTaskAsync(TimeSpan timeout); + } + + /// + /// Awaits a scripted worker task, treating cancellation and pipe-disconnect I/O faults as + /// the expected outcome of the worker client tearing the worker down, and rethrowing anything else. + /// + private static async Task ObserveWorkerTaskAsync(Task workerTask, TimeSpan timeout) + { + try + { + await workerTask.WaitAsync(timeout).ConfigureAwait(false); + } + catch (OperationCanceledException) + { + // Expected: the worker client cancelled the scripted worker during teardown. + } + catch (IOException) + { + // Expected: the gateway pipe was closed when the worker client disposed. + } + } + /// Fake worker launcher that connects a scripted fake worker harness. - private sealed class ScriptedFakeWorkerProcessLauncher : IWorkerProcessLauncher + private sealed class ScriptedFakeWorkerProcessLauncher : IWorkerTaskLauncher { /// The fake process ID used by the scripted launcher. public const int ProcessId = 2468; @@ -144,16 +200,23 @@ public sealed class SessionWorkerClientFactoryFakeWorkerTests /// Gets the connected fake worker harness. public FakeWorkerHarness? Harness { get; private set; } + /// Gets the scripted worker task. + public Task WorkerTask { get; private set; } = Task.CompletedTask; + /// public Task LaunchAsync( WorkerProcessLaunchRequest request, CancellationToken cancellationToken = default) { - _ = RunWorkerAsync(request, cancellationToken); + WorkerTask = RunWorkerAsync(request, cancellationToken); return Task.FromResult(CreateHandle(_process)); } + /// + public Task ObserveWorkerTaskAsync(TimeSpan timeout) => + SessionWorkerClientFactoryFakeWorkerTests.ObserveWorkerTaskAsync(WorkerTask, timeout); + private async Task RunWorkerAsync( WorkerProcessLaunchRequest request, CancellationToken cancellationToken) @@ -169,21 +232,28 @@ public sealed class SessionWorkerClientFactoryFakeWorkerTests } /// Fake worker launcher that fails during startup with protocol version mismatch. - private sealed class FailingStartupWorkerProcessLauncher : IWorkerProcessLauncher + private sealed class FailingStartupWorkerProcessLauncher : IWorkerTaskLauncher { /// Gets the fake worker process. public FakeWorkerProcess Process { get; } = new(processId: 3579); + /// Gets the scripted worker task. + public Task WorkerTask { get; private set; } = Task.CompletedTask; + /// public Task LaunchAsync( WorkerProcessLaunchRequest request, CancellationToken cancellationToken = default) { - _ = RunWorkerAsync(request, cancellationToken); + WorkerTask = RunWorkerAsync(request, cancellationToken); return Task.FromResult(CreateHandle(Process)); } + /// + public Task ObserveWorkerTaskAsync(TimeSpan timeout) => + SessionWorkerClientFactoryFakeWorkerTests.ObserveWorkerTaskAsync(WorkerTask, timeout); + private async Task RunWorkerAsync( WorkerProcessLaunchRequest request, CancellationToken cancellationToken) @@ -203,37 +273,52 @@ public sealed class SessionWorkerClientFactoryFakeWorkerTests } /// Fake worker launcher that never completes startup, simulating a hung worker. - private sealed class NeverReadyWorkerProcessLauncher : IWorkerProcessLauncher + private sealed class NeverReadyWorkerProcessLauncher : IWorkerTaskLauncher { + private readonly CancellationTokenSource _stop = new(); + /// Gets the fake worker process. public FakeWorkerProcess Process { get; } = new(processId: 4680); + /// Gets the scripted worker task. + public Task WorkerTask { get; private set; } = Task.CompletedTask; + /// public Task LaunchAsync( WorkerProcessLaunchRequest request, CancellationToken cancellationToken = default) { - _ = RunWorkerAsync(request, cancellationToken); + WorkerTask = RunWorkerAsync(request); return Task.FromResult(CreateHandle(Process)); } - private async Task RunWorkerAsync( - WorkerProcessLaunchRequest request, - CancellationToken cancellationToken) + /// + public async Task ObserveWorkerTaskAsync(TimeSpan timeout) + { + // The scripted worker parks on an infinite delay; cancel it so disposal observes + // the task instead of leaking it as an unobserved fault. + await _stop.CancelAsync().ConfigureAwait(false); + await SessionWorkerClientFactoryFakeWorkerTests + .ObserveWorkerTaskAsync(WorkerTask, timeout) + .ConfigureAwait(false); + _stop.Dispose(); + } + + private async Task RunWorkerAsync(WorkerProcessLaunchRequest request) { await using FakeWorkerHarness harness = await FakeWorkerHarness.ConnectToGatewayPipeAsync( request.SessionId, request.Nonce, request.PipeName, request.ProtocolVersion, - cancellationToken: cancellationToken).ConfigureAwait(false); - _ = await harness.ReadGatewayEnvelopeAsync(cancellationToken).ConfigureAwait(false); + cancellationToken: _stop.Token).ConfigureAwait(false); + _ = await harness.ReadGatewayEnvelopeAsync(_stop.Token).ConfigureAwait(false); await harness.SendWorkerHelloAsync( workerProcessId: Process.Id, workerProtocolVersion: request.ProtocolVersion, - cancellationToken: cancellationToken).ConfigureAwait(false); - await Task.Delay(Timeout.InfiniteTimeSpan, cancellationToken).ConfigureAwait(false); + cancellationToken: _stop.Token).ConfigureAwait(false); + await Task.Delay(Timeout.InfiniteTimeSpan, _stop.Token).ConfigureAwait(false); } } diff --git a/src/MxGateway.Tests/Gateway/Sessions/WorkerAlarmRpcDispatcherTests.cs b/src/MxGateway.Tests/Gateway/Sessions/WorkerAlarmRpcDispatcherTests.cs index dce9148..8297a2f 100644 --- a/src/MxGateway.Tests/Gateway/Sessions/WorkerAlarmRpcDispatcherTests.cs +++ b/src/MxGateway.Tests/Gateway/Sessions/WorkerAlarmRpcDispatcherTests.cs @@ -1,9 +1,4 @@ -using System; -using System.Collections.Generic; -using System.Linq; using System.Runtime.CompilerServices; -using System.Threading; -using System.Threading.Tasks; using MxGateway.Contracts.Proto; using MxGateway.Server.Sessions; using MxGateway.Server.Workers; @@ -19,10 +14,10 @@ namespace MxGateway.Tests.Gateway.Sessions; public sealed class WorkerAlarmRpcDispatcherTests { [Fact] - public async Task AcknowledgeAsync_returns_session_not_found_when_session_missing() + public async Task AcknowledgeAsync_WhenSessionMissing_ReturnsSessionNotFound() { - SessionRegistry registry = new SessionRegistry(); - WorkerAlarmRpcDispatcher dispatcher = new WorkerAlarmRpcDispatcher(registry); + SessionRegistry registry = new(); + WorkerAlarmRpcDispatcher dispatcher = new(registry); AcknowledgeAlarmReply reply = await dispatcher.AcknowledgeAsync( new AcknowledgeAlarmRequest @@ -37,11 +32,11 @@ public sealed class WorkerAlarmRpcDispatcherTests } [Fact] - public async Task AcknowledgeAsync_forwards_guid_and_returns_native_status() + public async Task AcknowledgeAsync_WithGuidReference_ForwardsGuidAndReturnsNativeStatus() { - SessionRegistry registry = new SessionRegistry(); + SessionRegistry registry = new(); Guid alarmGuid = Guid.NewGuid(); - FakeAlarmWorkerClient worker = new FakeAlarmWorkerClient + FakeAlarmWorkerClient worker = new() { ReplyFactory = command => { @@ -63,7 +58,7 @@ public sealed class WorkerAlarmRpcDispatcherTests session.MarkReady(); registry.TryAdd(session); - WorkerAlarmRpcDispatcher dispatcher = new WorkerAlarmRpcDispatcher(registry); + WorkerAlarmRpcDispatcher dispatcher = new(registry); AcknowledgeAlarmReply reply = await dispatcher.AcknowledgeAsync( new AcknowledgeAlarmRequest @@ -84,10 +79,10 @@ public sealed class WorkerAlarmRpcDispatcherTests } [Fact] - public async Task AcknowledgeAsync_propagates_worker_diagnostic_on_failure() + public async Task AcknowledgeAsync_WhenWorkerFails_PropagatesWorkerDiagnostic() { - SessionRegistry registry = new SessionRegistry(); - FakeAlarmWorkerClient worker = new FakeAlarmWorkerClient + SessionRegistry registry = new(); + FakeAlarmWorkerClient worker = new() { ReplyFactory = _ => new MxCommandReply { @@ -106,7 +101,7 @@ public sealed class WorkerAlarmRpcDispatcherTests session.MarkReady(); registry.TryAdd(session); - WorkerAlarmRpcDispatcher dispatcher = new WorkerAlarmRpcDispatcher(registry); + WorkerAlarmRpcDispatcher dispatcher = new(registry); AcknowledgeAlarmReply reply = await dispatcher.AcknowledgeAsync( new AcknowledgeAlarmRequest @@ -125,7 +120,7 @@ public sealed class WorkerAlarmRpcDispatcherTests [InlineData("Galaxy!TestArea.TestMachine_001.TestAlarm001", "Galaxy", "TestArea", "TestMachine_001.TestAlarm001")] [InlineData("Galaxy!Area.Tag", "Galaxy", "Area", "Tag")] [InlineData("Provider!Group.Tag.With.Dots", "Provider", "Group", "Tag.With.Dots")] - public void TryParseAlarmReference_decomposes_provider_group_tag( + public void TryParseAlarmReference_WithProviderGroupTag_DecomposesParts( string reference, string expectedProvider, string expectedGroup, string expectedName) { Assert.True(WorkerAlarmRpcDispatcher.TryParseAlarmReference( @@ -145,18 +140,18 @@ public sealed class WorkerAlarmRpcDispatcherTests [InlineData("Galaxy!Group")] // missing dot [InlineData("Galaxy!.Tag")] // empty group [InlineData("Galaxy!Group.")] // empty tag - public void TryParseAlarmReference_rejects_malformed_references(string? reference) + public void TryParseAlarmReference_WithMalformedReference_ReturnsFalse(string? reference) { Assert.False(WorkerAlarmRpcDispatcher.TryParseAlarmReference( reference, out _, out _, out _)); } [Fact] - public async Task AcknowledgeAsync_routes_provider_group_tag_via_AckByName() + public async Task AcknowledgeAsync_WithProviderGroupTagReference_RoutesViaAckByName() { - SessionRegistry registry = new SessionRegistry(); + SessionRegistry registry = new(); AcknowledgeAlarmByNameCommand? observed = null; - FakeAlarmWorkerClient worker = new FakeAlarmWorkerClient + FakeAlarmWorkerClient worker = new() { ReplyFactory = command => { @@ -176,7 +171,7 @@ public sealed class WorkerAlarmRpcDispatcherTests session.MarkReady(); registry.TryAdd(session); - WorkerAlarmRpcDispatcher dispatcher = new WorkerAlarmRpcDispatcher(registry); + WorkerAlarmRpcDispatcher dispatcher = new(registry); AcknowledgeAlarmReply reply = await dispatcher.AcknowledgeAsync( new AcknowledgeAlarmRequest @@ -199,16 +194,16 @@ public sealed class WorkerAlarmRpcDispatcherTests } [Fact] - public async Task AcknowledgeAsync_returns_invalid_request_for_unparseable_reference() + public async Task AcknowledgeAsync_WithUnparseableReference_ReturnsInvalidRequest() { - SessionRegistry registry = new SessionRegistry(); - FakeAlarmWorkerClient worker = new FakeAlarmWorkerClient(); + SessionRegistry registry = new(); + FakeAlarmWorkerClient worker = new(); GatewaySession session = NewSession("s1"); session.AttachWorkerClient(worker); session.MarkReady(); registry.TryAdd(session); - WorkerAlarmRpcDispatcher dispatcher = new WorkerAlarmRpcDispatcher(registry); + WorkerAlarmRpcDispatcher dispatcher = new(registry); AcknowledgeAlarmReply reply = await dispatcher.AcknowledgeAsync( new AcknowledgeAlarmRequest @@ -223,10 +218,10 @@ public sealed class WorkerAlarmRpcDispatcherTests } [Fact] - public async Task QueryActiveAlarmsAsync_yields_each_snapshot_from_payload() + public async Task QueryActiveAlarmsAsync_WithPayloadSnapshots_YieldsEachSnapshot() { - SessionRegistry registry = new SessionRegistry(); - FakeAlarmWorkerClient worker = new FakeAlarmWorkerClient + SessionRegistry registry = new(); + FakeAlarmWorkerClient worker = new() { ReplyFactory = command => { @@ -257,9 +252,9 @@ public sealed class WorkerAlarmRpcDispatcherTests session.MarkReady(); registry.TryAdd(session); - WorkerAlarmRpcDispatcher dispatcher = new WorkerAlarmRpcDispatcher(registry); + WorkerAlarmRpcDispatcher dispatcher = new(registry); - List collected = new List(); + List collected = new(); await foreach (ActiveAlarmSnapshot snap in dispatcher.QueryActiveAlarmsAsync( new QueryActiveAlarmsRequest { SessionId = "s1" }, CancellationToken.None)) @@ -273,12 +268,12 @@ public sealed class WorkerAlarmRpcDispatcherTests } [Fact] - public async Task QueryActiveAlarmsAsync_yields_empty_when_session_missing() + public async Task QueryActiveAlarmsAsync_WhenSessionMissing_YieldsEmpty() { - SessionRegistry registry = new SessionRegistry(); - WorkerAlarmRpcDispatcher dispatcher = new WorkerAlarmRpcDispatcher(registry); + SessionRegistry registry = new(); + WorkerAlarmRpcDispatcher dispatcher = new(registry); - List collected = new List(); + List collected = new(); await foreach (ActiveAlarmSnapshot snap in dispatcher.QueryActiveAlarmsAsync( new QueryActiveAlarmsRequest { SessionId = "missing" }, CancellationToken.None)) @@ -290,10 +285,10 @@ public sealed class WorkerAlarmRpcDispatcherTests } [Fact] - public async Task QueryActiveAlarmsAsync_yields_empty_on_worker_failure() + public async Task QueryActiveAlarmsAsync_WhenWorkerFails_YieldsEmpty() { - SessionRegistry registry = new SessionRegistry(); - FakeAlarmWorkerClient worker = new FakeAlarmWorkerClient + SessionRegistry registry = new(); + FakeAlarmWorkerClient worker = new() { ReplyFactory = _ => new MxCommandReply { @@ -310,9 +305,9 @@ public sealed class WorkerAlarmRpcDispatcherTests session.MarkReady(); registry.TryAdd(session); - WorkerAlarmRpcDispatcher dispatcher = new WorkerAlarmRpcDispatcher(registry); + WorkerAlarmRpcDispatcher dispatcher = new(registry); - List collected = new List(); + List collected = new(); await foreach (ActiveAlarmSnapshot snap in dispatcher.QueryActiveAlarmsAsync( new QueryActiveAlarmsRequest { SessionId = "s1" }, CancellationToken.None)) @@ -325,7 +320,7 @@ public sealed class WorkerAlarmRpcDispatcherTests private static GatewaySession NewSession(string sessionId) { - return new GatewaySession( + return new( sessionId, "mxaccess", $"mxaccess-gateway-1-{sessionId}", diff --git a/src/MxGateway.Tests/MxGateway.Tests.csproj b/src/MxGateway.Tests/MxGateway.Tests.csproj index 27e10c0..5ad354d 100644 --- a/src/MxGateway.Tests/MxGateway.Tests.csproj +++ b/src/MxGateway.Tests/MxGateway.Tests.csproj @@ -16,6 +16,13 @@ + + + + + diff --git a/src/MxGateway.Tests/Security/Authorization/GatewayGrpcAuthorizationInterceptorTests.cs b/src/MxGateway.Tests/Security/Authorization/GatewayGrpcAuthorizationInterceptorTests.cs index 6e5779d..a9031a4 100644 --- a/src/MxGateway.Tests/Security/Authorization/GatewayGrpcAuthorizationInterceptorTests.cs +++ b/src/MxGateway.Tests/Security/Authorization/GatewayGrpcAuthorizationInterceptorTests.cs @@ -10,6 +10,7 @@ using MxGateway.Server.Metrics; using MxGateway.Server.Security.Authentication; using MxGateway.Server.Security.Authorization; using MxGateway.Server.Sessions; +using MxGateway.Tests.TestSupport; namespace MxGateway.Tests.Security.Authorization; @@ -107,7 +108,7 @@ public sealed class GatewayGrpcAuthorizationInterceptorTests RpcException exception = await Assert.ThrowsAsync( () => interceptor.ServerStreamingServerHandler( new StreamEventsRequest(), - new TestServerStreamWriter(), + new RecordingServerStreamWriter(), ContextWithAuthorization("Bearer mxgw_operator01_secret"), (_, _, _) => Task.CompletedTask)); @@ -123,7 +124,7 @@ public sealed class GatewayGrpcAuthorizationInterceptorTests GatewayGrpcAuthorizationInterceptor interceptor = CreateInterceptor( new FakeApiKeyVerifier(SuccessWithScopes(GatewayScopes.EventsRead)), identityAccessor); - TestServerStreamWriter streamWriter = new(); + RecordingServerStreamWriter streamWriter = new(); await interceptor.ServerStreamingServerHandler( new StreamEventsRequest(), @@ -396,40 +397,6 @@ public sealed class GatewayGrpcAuthorizationInterceptorTests } } - /// Constraint enforcer that permits every operation for composition tests. - private sealed class AllowAllConstraintEnforcer : IConstraintEnforcer - { - /// - public Task CheckReadTagAsync( - ApiKeyIdentity? identity, - string tagAddress, - CancellationToken cancellationToken) => Task.FromResult(null); - - /// - public Task CheckReadHandleAsync( - ApiKeyIdentity? identity, - GatewaySession session, - int serverHandle, - int itemHandle, - CancellationToken cancellationToken) => Task.FromResult(null); - - /// - public Task CheckWriteHandleAsync( - ApiKeyIdentity? identity, - GatewaySession session, - int serverHandle, - int itemHandle, - CancellationToken cancellationToken) => Task.FromResult(null); - - /// - public Task RecordDenialAsync( - ApiKeyIdentity? identity, - string commandKind, - string target, - ConstraintFailure failure, - CancellationToken cancellationToken) => Task.CompletedTask; - } - private sealed class FakeApiKeyVerifier(ApiKeyVerificationResult result) : IApiKeyVerifier { /// Gets whether the verifier was called. @@ -453,88 +420,4 @@ public sealed class GatewayGrpcAuthorizationInterceptorTests } } - private sealed class TestServerStreamWriter : IServerStreamWriter - { - /// Gets messages written to the stream. - public List Messages { get; } = []; - - /// Gets or sets write options for the stream. - public WriteOptions? WriteOptions { get; set; } - - /// Writes a message to the stream. - /// The message to write. - /// Task representing the write operation. - public Task WriteAsync(T message) - { - Messages.Add(message); - - return Task.CompletedTask; - } - } - - private sealed class TestServerCallContext( - Metadata requestHeaders, - CancellationToken cancellationToken = default) : ServerCallContext - { - private readonly Metadata responseTrailers = []; - private readonly Dictionary userState = []; - private Status status; - private WriteOptions? writeOptions; - - /// - protected override string MethodCore => "/mxaccess_gateway.v1.MxAccessGateway/Test"; - - /// - protected override string HostCore => "localhost"; - - /// - protected override string PeerCore => "ipv4:127.0.0.1:5000"; - - /// - protected override DateTime DeadlineCore => DateTime.UtcNow.AddMinutes(1); - - /// - protected override Metadata RequestHeadersCore => requestHeaders; - - /// - protected override CancellationToken CancellationTokenCore => cancellationToken; - - /// - protected override Metadata ResponseTrailersCore => responseTrailers; - - /// - protected override Status StatusCore - { - get => status; - set => status = value; - } - - /// - protected override WriteOptions? WriteOptionsCore - { - get => writeOptions; - set => writeOptions = value; - } - - /// - protected override AuthContext AuthContextCore { get; } = new( - string.Empty, - new Dictionary>(StringComparer.Ordinal)); - - /// - protected override IDictionary UserStateCore => userState; - - /// - protected override Task WriteResponseHeadersAsyncCore(Metadata responseHeaders) - { - return Task.CompletedTask; - } - - /// - protected override ContextPropagationToken CreatePropagationTokenCore( - ContextPropagationOptions? options) - { - throw new NotSupportedException(); - } - } } diff --git a/src/MxGateway.Tests/TestSupport/AllowAllConstraintEnforcer.cs b/src/MxGateway.Tests/TestSupport/AllowAllConstraintEnforcer.cs new file mode 100644 index 0000000..e399dd5 --- /dev/null +++ b/src/MxGateway.Tests/TestSupport/AllowAllConstraintEnforcer.cs @@ -0,0 +1,42 @@ +using MxGateway.Server.Security.Authentication; +using MxGateway.Server.Security.Authorization; +using MxGateway.Server.Sessions; + +namespace MxGateway.Tests.TestSupport; + +/// +/// that permits every operation, for tests that +/// exercise gRPC service or interceptor behaviour without constraint policy. +/// +public sealed class AllowAllConstraintEnforcer : IConstraintEnforcer +{ + /// + public Task CheckReadTagAsync( + ApiKeyIdentity? identity, + string tagAddress, + CancellationToken cancellationToken) => Task.FromResult(null); + + /// + public Task CheckReadHandleAsync( + ApiKeyIdentity? identity, + GatewaySession session, + int serverHandle, + int itemHandle, + CancellationToken cancellationToken) => Task.FromResult(null); + + /// + public Task CheckWriteHandleAsync( + ApiKeyIdentity? identity, + GatewaySession session, + int serverHandle, + int itemHandle, + CancellationToken cancellationToken) => Task.FromResult(null); + + /// + public Task RecordDenialAsync( + ApiKeyIdentity? identity, + string commandKind, + string target, + ConstraintFailure failure, + CancellationToken cancellationToken) => Task.CompletedTask; +} diff --git a/src/MxGateway.Tests/TestSupport/RecordingServerStreamWriter.cs b/src/MxGateway.Tests/TestSupport/RecordingServerStreamWriter.cs new file mode 100644 index 0000000..e38cd2a --- /dev/null +++ b/src/MxGateway.Tests/TestSupport/RecordingServerStreamWriter.cs @@ -0,0 +1,50 @@ +using Grpc.Core; + +namespace MxGateway.Tests.TestSupport; + +/// +/// Thread-safe that records every written message +/// and lets a test await the first message with a timeout. +/// +/// The streamed message type. +public sealed class RecordingServerStreamWriter : IServerStreamWriter +{ + private readonly object _syncRoot = new(); + private readonly TaskCompletionSource _firstMessage = new(TaskCreationOptions.RunContinuationsAsynchronously); + private readonly List _messages = []; + + /// Gets the messages written to this stream, in order. + public IReadOnlyList Messages + { + get + { + lock (_syncRoot) + { + return _messages.ToArray(); + } + } + } + + /// Gets or sets options for writing messages to the stream. + public WriteOptions? WriteOptions { get; set; } + + /// Records the supplied message. + /// The message to record. + /// A completed task. + public Task WriteAsync(T message) + { + lock (_syncRoot) + { + _messages.Add(message); + } + + _firstMessage.TrySetResult(message); + return Task.CompletedTask; + } + + /// Waits for the first message to be written within the specified timeout. + /// Maximum time to wait for the first message. + /// The first message written to this stream. + public async Task WaitForFirstMessageAsync(TimeSpan timeout) => + await _firstMessage.Task.WaitAsync(timeout).ConfigureAwait(false); +} diff --git a/src/MxGateway.Tests/TestSupport/TestServerCallContext.cs b/src/MxGateway.Tests/TestSupport/TestServerCallContext.cs new file mode 100644 index 0000000..8d5e419 --- /dev/null +++ b/src/MxGateway.Tests/TestSupport/TestServerCallContext.cs @@ -0,0 +1,76 @@ +using Grpc.Core; + +namespace MxGateway.Tests.TestSupport; + +/// +/// Minimal in-memory for exercising gRPC service +/// implementations directly in unit tests, without a real gRPC transport. +/// +public sealed class TestServerCallContext : ServerCallContext +{ + private readonly Metadata _requestHeaders; + private readonly Metadata _responseTrailers = []; + private readonly Dictionary _userState = []; + private readonly CancellationToken _cancellationToken; + private Status _status; + private WriteOptions? _writeOptions; + + /// Initializes the context with the supplied request headers and cancellation token. + /// Request headers visible to the service; defaults to empty. + /// Cancellation token surfaced to the service. + public TestServerCallContext(Metadata? requestHeaders = null, CancellationToken cancellationToken = default) + { + _requestHeaders = requestHeaders ?? []; + _cancellationToken = cancellationToken; + } + + /// + protected override string MethodCore => "/mxaccess_gateway.v1.MxAccessGateway/Test"; + + /// + protected override string HostCore => "localhost"; + + /// + protected override string PeerCore => "ipv4:127.0.0.1:5000"; + + /// + protected override DateTime DeadlineCore => DateTime.UtcNow.AddMinutes(1); + + /// + protected override Metadata RequestHeadersCore => _requestHeaders; + + /// + protected override CancellationToken CancellationTokenCore => _cancellationToken; + + /// + protected override Metadata ResponseTrailersCore => _responseTrailers; + + /// + protected override Status StatusCore + { + get => _status; + set => _status = value; + } + + /// + protected override WriteOptions? WriteOptionsCore + { + get => _writeOptions; + set => _writeOptions = value; + } + + /// + protected override AuthContext AuthContextCore { get; } = new( + string.Empty, + new Dictionary>(StringComparer.Ordinal)); + + /// + protected override IDictionary UserStateCore => _userState; + + /// + protected override Task WriteResponseHeadersAsyncCore(Metadata responseHeaders) => Task.CompletedTask; + + /// + protected override ContextPropagationToken CreatePropagationTokenCore(ContextPropagationOptions? options) => + throw new NotSupportedException(); +} diff --git a/src/MxGateway.Tests/xunit.runner.json b/src/MxGateway.Tests/xunit.runner.json new file mode 100644 index 0000000..14a4420 --- /dev/null +++ b/src/MxGateway.Tests/xunit.runner.json @@ -0,0 +1,8 @@ +{ + "$schema": "https://xunit.net/schema/current/xunit.runner.schema.json", + "appDomain": "denied", + "parallelizeAssembly": false, + "parallelizeTestCollections": true, + "maxParallelThreads": -1, + "longRunningTestSeconds": 30 +}