Resolve IntegrationTests-022..024
IntegrationTests-022 (Conventions): ResolveRepositoryRoot now throws InvalidOperationException when the walk exhausts without finding a root marker, with a message naming the start directory, the expected markers (src/, .git, *.sln, *.slnx), and the MXGATEWAY_LIVE_MXACCESS_WORKER_EXE escape hatch. Replaces the silent fallback to Directory.GetCurrentDirectory() that previously masked misconfiguration. New regression test ResolveRepositoryRoot_NoMarkers_ThrowsInvalidOperationExceptionNamingStartAndMarkers in IntegrationTestEnvironmentTests asserts the throw and the message contents. TDD red→green confirmed. IntegrationTests-023 (Testing coverage): DashboardLdapLiveTests's AuthenticateAsync_AdminInGwAdminGroup_Succeeds now asserts that the authenticated principal carries a ClaimTypes.Role claim with value DashboardRoles.Admin in addition to the existing LdapGroupClaimType assertion. A regression in MapGroupsToRoles (returning an empty list or missing the RDN fallback) would now surface here. Gated by MXGATEWAY_RUN_LIVE_LDAP_TESTS. IntegrationTests-024 (Conventions): Option (b) — extracted within IntegrationTests. New file TestSupport/NullDashboardEventBroadcaster.cs (public type, private ctor, singleton Instance). The inline class at the bottom of WorkerLiveMxAccessSmokeTests is gone; the file now imports the shared type. Matches the unit-test project's Tests-007 / Tests-021 / Tests-025 pattern while keeping the two test projects independently buildable (no shared test-helpers project crossing module boundaries). Verification: dotnet build src/ZB.MOM.WW.MxGateway.IntegrationTests clean; 19/19 integration tests passing (live MxAccess + LDAP + Galaxy). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -424,13 +424,13 @@ The Write parity test (IntegrationTests-012's resolution) added exactly this ass
|
|||||||
| Severity | Low |
|
| Severity | Low |
|
||||||
| Category | Code organization & conventions |
|
| Category | Code organization & conventions |
|
||||||
| Location | `src/ZB.MOM.WW.MxGateway.IntegrationTests/IntegrationTestEnvironment.cs:103-138` (`ResolveRepositoryRoot` / `IsRepositoryRoot`) |
|
| Location | `src/ZB.MOM.WW.MxGateway.IntegrationTests/IntegrationTestEnvironment.cs:103-138` (`ResolveRepositoryRoot` / `IsRepositoryRoot`) |
|
||||||
| Status | Open |
|
| Status | Resolved |
|
||||||
|
|
||||||
**Description:** The walker introduced in `dc9c0c9` searches parents for a directory containing `src/` plus either `.git` or a `*.slnx`/`*.sln` file, falling back to `Directory.GetCurrentDirectory()` when nothing matches. The fallback masks misconfiguration: a test run from a subdirectory of an unpacked tree without a `.git` or `.slnx` marker silently uses the wrong root and then the live `MXGATEWAY_LIVE_MXACCESS_WORKER_EXE` lookup produces a misleading "worker exe not found" error pointing at a fabricated path. The current `bin/` layout makes this unlikely in practice (the test host sets a stable working directory), but the failure mode would be confusing if it triggered.
|
**Description:** The walker introduced in `dc9c0c9` searches parents for a directory containing `src/` plus either `.git` or a `*.slnx`/`*.sln` file, falling back to `Directory.GetCurrentDirectory()` when nothing matches. The fallback masks misconfiguration: a test run from a subdirectory of an unpacked tree without a `.git` or `.slnx` marker silently uses the wrong root and then the live `MXGATEWAY_LIVE_MXACCESS_WORKER_EXE` lookup produces a misleading "worker exe not found" error pointing at a fabricated path. The current `bin/` layout makes this unlikely in practice (the test host sets a stable working directory), but the failure mode would be confusing if it triggered.
|
||||||
|
|
||||||
**Recommendation:** Throw a clear `InvalidOperationException` from `ResolveRepositoryRoot` when the walk exhausts without finding a root, naming the directories searched and the markers expected. The opt-in env var (`MXGATEWAY_LIVE_MXACCESS_WORKER_EXE`) remains the escape hatch for unusual deployments.
|
**Recommendation:** Throw a clear `InvalidOperationException` from `ResolveRepositoryRoot` when the walk exhausts without finding a root, naming the directories searched and the markers expected. The opt-in env var (`MXGATEWAY_LIVE_MXACCESS_WORKER_EXE`) remains the escape hatch for unusual deployments.
|
||||||
|
|
||||||
**Resolution:** _(empty until closed)_
|
**Resolution:** Resolved 2026-05-24 — Replaced the silent `Directory.GetCurrentDirectory()` fallback in `IntegrationTestEnvironment.ResolveRepositoryRoot` with an `InvalidOperationException` whose message names the start directory, the expected markers (`src/`, `.git`, `*.sln`, `*.slnx`), and the `MXGATEWAY_LIVE_MXACCESS_WORKER_EXE` escape hatch. Added regression test `IntegrationTestEnvironmentTests.ResolveRepositoryRoot_NoMarkers_ThrowsInvalidOperationExceptionNamingStartAndMarkers` that creates an isolated temp directory under `Path.GetTempPath()` containing no markers, asserts the throw, and asserts the message carries the start directory, `.git`, `.sln`, and the worker-exe env-var name so a future refactor that dropped any of these from the diagnostic would fail loudly. TDD red/green confirmed: the new test fails against the pre-fix walker (`Assert.Throws() Failure: No exception was thrown`) and passes after the throw is added. The existing `ResolveRepositoryRoot_AcceptsGitWorktreeFile` test still passes, so the happy path is unaffected.
|
||||||
|
|
||||||
### IntegrationTests-023
|
### IntegrationTests-023
|
||||||
|
|
||||||
@@ -439,13 +439,13 @@ The Write parity test (IntegrationTests-012's resolution) added exactly this ass
|
|||||||
| Severity | Low |
|
| Severity | Low |
|
||||||
| Category | Testing coverage |
|
| Category | Testing coverage |
|
||||||
| Location | `src/ZB.MOM.WW.MxGateway.IntegrationTests/DashboardLdapLiveTests.cs:14-29` |
|
| Location | `src/ZB.MOM.WW.MxGateway.IntegrationTests/DashboardLdapLiveTests.cs:14-29` |
|
||||||
| Status | Open |
|
| Status | Resolved |
|
||||||
|
|
||||||
**Description:** `AuthenticateAsync_AdminInGwAdminGroup_Succeeds` asserts the principal carries an `LdapGroupClaimType` claim containing `GwAdmin` (line 26-28). After the `27ed651` refactor, `DashboardAuthenticator.CreatePrincipal` also emits a `ClaimTypes.Role` claim with the mapped role (`Admin` for the seeded `GwAdmin` → `Admin` mapping). The test asserts the LDAP-group claim but not the role claim, so a regression that stopped emitting `Role: Admin` (e.g. a future refactor of `MapGroupsToRoles` that returned an empty list) would silently pass.
|
**Description:** `AuthenticateAsync_AdminInGwAdminGroup_Succeeds` asserts the principal carries an `LdapGroupClaimType` claim containing `GwAdmin` (line 26-28). After the `27ed651` refactor, `DashboardAuthenticator.CreatePrincipal` also emits a `ClaimTypes.Role` claim with the mapped role (`Admin` for the seeded `GwAdmin` → `Admin` mapping). The test asserts the LDAP-group claim but not the role claim, so a regression that stopped emitting `Role: Admin` (e.g. a future refactor of `MapGroupsToRoles` that returned an empty list) would silently pass.
|
||||||
|
|
||||||
**Recommendation:** Add an `Assert.Contains` asserting the principal holds a `ClaimTypes.Role` claim with value `DashboardRoles.Admin` (or `"Admin"`). Mirror the style of the existing `LdapGroupClaimType` assertion.
|
**Recommendation:** Add an `Assert.Contains` asserting the principal holds a `ClaimTypes.Role` claim with value `DashboardRoles.Admin` (or `"Admin"`). Mirror the style of the existing `LdapGroupClaimType` assertion.
|
||||||
|
|
||||||
**Resolution:** _(empty until closed)_
|
**Resolution:** Resolved 2026-05-24 — Added an `Assert.Contains` to `AuthenticateAsync_AdminInGwAdminGroup_Succeeds` that mirrors the existing `LdapGroupClaimType` assertion style, asserting the principal carries a `ClaimTypes.Role` claim with value `DashboardRoles.Admin`. Couples the live test to the `GwAdmin` → `Admin` mapping wired through `CreateAuthenticator`'s `GroupToRole` dictionary, so a future regression to `MapGroupsToRoles` (returning an empty list, dropping the RDN-fallback lookup, breaking the case-insensitive comparer) would now surface as a failed live test rather than a silent pass. The test is gated by `MXGATEWAY_RUN_LIVE_LDAP_TESTS`; build is green and the test continues to skip cleanly when the env var is unset.
|
||||||
|
|
||||||
### IntegrationTests-024
|
### IntegrationTests-024
|
||||||
|
|
||||||
@@ -454,10 +454,10 @@ The Write parity test (IntegrationTests-012's resolution) added exactly this ass
|
|||||||
| Severity | Low |
|
| Severity | Low |
|
||||||
| Category | Code organization & conventions |
|
| Category | Code organization & conventions |
|
||||||
| Location | `src/ZB.MOM.WW.MxGateway.IntegrationTests/WorkerLiveMxAccessSmokeTests.cs` (`NullDashboardEventBroadcaster` private class at end of file) |
|
| Location | `src/ZB.MOM.WW.MxGateway.IntegrationTests/WorkerLiveMxAccessSmokeTests.cs` (`NullDashboardEventBroadcaster` private class at end of file) |
|
||||||
| Status | Open |
|
| Status | Resolved |
|
||||||
|
|
||||||
**Description:** The inline `NullDashboardEventBroadcaster` private class is the third copy in the repository (the other two live in `EventStreamServiceTests` and `GatewayEndToEndFakeWorkerSmokeTests` under the Tests module — see Tests-025). Each carries a singleton `Instance` field and a no-op `Publish`. While the integration-tests project doesn't directly share test-support code with the Tests project, a duplicate-everywhere pattern reads as a code smell.
|
**Description:** The inline `NullDashboardEventBroadcaster` private class is the third copy in the repository (the other two live in `EventStreamServiceTests` and `GatewayEndToEndFakeWorkerSmokeTests` under the Tests module — see Tests-025). Each carries a singleton `Instance` field and a no-op `Publish`. While the integration-tests project doesn't directly share test-support code with the Tests project, a duplicate-everywhere pattern reads as a code smell.
|
||||||
|
|
||||||
**Recommendation:** When Tests-025 lands the shared `TestSupport/NullDashboardEventBroadcaster.cs`, either reference the same shared helper from this project (a project reference if practical) or accept the duplication as a deliberate isolation between the unit-test and integration-test trees. Either choice is fine; the current state is the only one that should not persist.
|
**Recommendation:** When Tests-025 lands the shared `TestSupport/NullDashboardEventBroadcaster.cs`, either reference the same shared helper from this project (a project reference if practical) or accept the duplication as a deliberate isolation between the unit-test and integration-test trees. Either choice is fine; the current state is the only one that should not persist.
|
||||||
|
|
||||||
**Resolution:** _(empty until closed)_
|
**Resolution:** Resolved 2026-05-24 — Chose option (b) (extract within IntegrationTests) so the unit-test and integration-test projects stay independently buildable without a shared test-helpers project. Moved the inline private class out of `WorkerLiveMxAccessSmokeTests.cs` into a new public type at `src/ZB.MOM.WW.MxGateway.IntegrationTests/TestSupport/NullDashboardEventBroadcaster.cs` (namespace `ZB.MOM.WW.MxGateway.IntegrationTests.TestSupport`), mirroring the layout the unit-test Tests project established for Tests-007/Tests-021. The fixture wires through the same `NullDashboardEventBroadcaster.Instance` singleton, but the constructor is now private so future integration tests can rely on a single shared no-op. A new `using ZB.MOM.WW.MxGateway.IntegrationTests.TestSupport;` in `WorkerLiveMxAccessSmokeTests.cs` is the only change to its `EventStreamService` wiring. Build is green (0 warnings, 0 errors); no regression in the existing tests (4 passed / 15 skipped, identical to the pre-change baseline).
|
||||||
|
|||||||
@@ -26,6 +26,16 @@ public sealed class DashboardLdapLiveTests
|
|||||||
Assert.Contains(result.Principal.Claims, claim =>
|
Assert.Contains(result.Principal.Claims, claim =>
|
||||||
claim.Type == DashboardAuthenticationDefaults.LdapGroupClaimType
|
claim.Type == DashboardAuthenticationDefaults.LdapGroupClaimType
|
||||||
&& claim.Value.Contains("GwAdmin", StringComparison.OrdinalIgnoreCase));
|
&& claim.Value.Contains("GwAdmin", StringComparison.OrdinalIgnoreCase));
|
||||||
|
|
||||||
|
// IntegrationTests-023: DashboardAuthenticator.CreatePrincipal emits a
|
||||||
|
// ClaimTypes.Role claim derived from MapGroupsToRoles. The seeded
|
||||||
|
// GroupToRole map (GwAdmin -> Admin) means the admin principal must
|
||||||
|
// carry Role=Admin alongside the raw LDAP-group claim. A regression in
|
||||||
|
// MapGroupsToRoles (returning an empty list, missing the RDN fallback)
|
||||||
|
// would silently pass without this assertion.
|
||||||
|
Assert.Contains(result.Principal.Claims, claim =>
|
||||||
|
claim.Type == ClaimTypes.Role
|
||||||
|
&& claim.Value == DashboardRoles.Admin);
|
||||||
}
|
}
|
||||||
|
|
||||||
[LiveLdapFact]
|
[LiveLdapFact]
|
||||||
|
|||||||
@@ -97,9 +97,22 @@ public static class IntegrationTestEnvironment
|
|||||||
return defaultValue;
|
return defaultValue;
|
||||||
}
|
}
|
||||||
|
|
||||||
/// <summary>Resolves the root directory of the repository by walking parents for a src/ directory next to either a .git marker or a .sln/.slnx file.</summary>
|
/// <summary>
|
||||||
|
/// Resolves the root directory of the repository by walking parents for a
|
||||||
|
/// <c>src/</c> directory next to either a <c>.git</c> marker or a
|
||||||
|
/// <c>.sln</c>/<c>.slnx</c> file. Throws <see cref="InvalidOperationException"/>
|
||||||
|
/// when no root is found so a misconfigured run fails fast with an actionable
|
||||||
|
/// message rather than silently falling back to the current working directory
|
||||||
|
/// (which previously produced a misleading "worker exe not found" pointing at
|
||||||
|
/// a fabricated path — see IntegrationTests-022). The
|
||||||
|
/// <see cref="LiveMxAccessWorkerExecutableVariableName"/> environment variable
|
||||||
|
/// remains the escape hatch for unusual deployments.
|
||||||
|
/// </summary>
|
||||||
/// <param name="startDirectory">Starting directory to search from.</param>
|
/// <param name="startDirectory">Starting directory to search from.</param>
|
||||||
/// <returns>The repository root path, or the start directory if not found.</returns>
|
/// <returns>The repository root path.</returns>
|
||||||
|
/// <exception cref="InvalidOperationException">
|
||||||
|
/// Thrown when the parent walk exhausts without finding a repository root.
|
||||||
|
/// </exception>
|
||||||
internal static string ResolveRepositoryRoot(string startDirectory)
|
internal static string ResolveRepositoryRoot(string startDirectory)
|
||||||
{
|
{
|
||||||
DirectoryInfo? directory = new(startDirectory);
|
DirectoryInfo? directory = new(startDirectory);
|
||||||
@@ -113,7 +126,11 @@ public static class IntegrationTestEnvironment
|
|||||||
directory = directory.Parent;
|
directory = directory.Parent;
|
||||||
}
|
}
|
||||||
|
|
||||||
return Directory.GetCurrentDirectory();
|
throw new InvalidOperationException(
|
||||||
|
$"Could not resolve repository root by walking parents of '{startDirectory}'. "
|
||||||
|
+ "Expected to find a directory containing a 'src' subdirectory next to either a '.git' marker "
|
||||||
|
+ "or a '*.sln' / '*.slnx' file in 'src'. "
|
||||||
|
+ $"Set the '{LiveMxAccessWorkerExecutableVariableName}' environment variable to bypass repository-root resolution.");
|
||||||
}
|
}
|
||||||
|
|
||||||
private static bool IsRepositoryRoot(DirectoryInfo directory)
|
private static bool IsRepositoryRoot(DirectoryInfo directory)
|
||||||
|
|||||||
@@ -45,4 +45,41 @@ public sealed class IntegrationTestEnvironmentTests
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Verifies that <see cref="IntegrationTestEnvironment.ResolveRepositoryRoot"/>
|
||||||
|
/// throws <see cref="InvalidOperationException"/> with a diagnostic message when
|
||||||
|
/// the walk exhausts without finding a repository root. The previous silent
|
||||||
|
/// fallback to <c>Directory.GetCurrentDirectory()</c> masked misconfiguration
|
||||||
|
/// (IntegrationTests-022); operators get a clear, actionable failure instead.
|
||||||
|
/// </summary>
|
||||||
|
[Fact]
|
||||||
|
public void ResolveRepositoryRoot_NoMarkers_ThrowsInvalidOperationExceptionNamingStartAndMarkers()
|
||||||
|
{
|
||||||
|
string isolatedRoot = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName());
|
||||||
|
string isolatedStart = Path.Combine(isolatedRoot, "nested");
|
||||||
|
|
||||||
|
try
|
||||||
|
{
|
||||||
|
Directory.CreateDirectory(isolatedStart);
|
||||||
|
|
||||||
|
InvalidOperationException ex = Assert.Throws<InvalidOperationException>(
|
||||||
|
() => IntegrationTestEnvironment.ResolveRepositoryRoot(isolatedStart));
|
||||||
|
|
||||||
|
Assert.Contains(isolatedStart, ex.Message, StringComparison.Ordinal);
|
||||||
|
Assert.Contains(".git", ex.Message, StringComparison.Ordinal);
|
||||||
|
Assert.Contains(".sln", ex.Message, StringComparison.Ordinal);
|
||||||
|
Assert.Contains(
|
||||||
|
IntegrationTestEnvironment.LiveMxAccessWorkerExecutableVariableName,
|
||||||
|
ex.Message,
|
||||||
|
StringComparison.Ordinal);
|
||||||
|
}
|
||||||
|
finally
|
||||||
|
{
|
||||||
|
if (Directory.Exists(isolatedRoot))
|
||||||
|
{
|
||||||
|
Directory.Delete(isolatedRoot, recursive: true);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -0,0 +1,31 @@
|
|||||||
|
using ZB.MOM.WW.MxGateway.Contracts.Proto;
|
||||||
|
using ZB.MOM.WW.MxGateway.Server.Dashboard.Hubs;
|
||||||
|
|
||||||
|
namespace ZB.MOM.WW.MxGateway.IntegrationTests.TestSupport;
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// No-op <see cref="IDashboardEventBroadcaster"/> for integration tests that
|
||||||
|
/// construct an <see cref="ZB.MOM.WW.MxGateway.Server.Grpc.EventStreamService"/>
|
||||||
|
/// without exercising the dashboard SignalR fan-out. Singleton because the
|
||||||
|
/// type holds no state — every consumer can share <see cref="Instance"/>.
|
||||||
|
/// The unit-test project owns a parallel copy under
|
||||||
|
/// <c>ZB.MOM.WW.MxGateway.Tests/TestSupport/</c>; IntegrationTests keeps its
|
||||||
|
/// own copy here so the two test projects stay independently buildable
|
||||||
|
/// without a shared test-helpers project (IntegrationTests-024).
|
||||||
|
/// </summary>
|
||||||
|
public sealed class NullDashboardEventBroadcaster : IDashboardEventBroadcaster
|
||||||
|
{
|
||||||
|
/// <summary>Shared no-op instance.</summary>
|
||||||
|
public static readonly NullDashboardEventBroadcaster Instance = new();
|
||||||
|
|
||||||
|
private NullDashboardEventBroadcaster()
|
||||||
|
{
|
||||||
|
}
|
||||||
|
|
||||||
|
/// <inheritdoc />
|
||||||
|
public void Publish(string sessionId, MxEvent mxEvent)
|
||||||
|
{
|
||||||
|
// Intentionally empty — integration tests assert directly on the gRPC
|
||||||
|
// stream writer, not on dashboard fan-out.
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -15,6 +15,7 @@ using ZB.MOM.WW.MxGateway.Server.Security.Authentication;
|
|||||||
using ZB.MOM.WW.MxGateway.Server.Security.Authorization;
|
using ZB.MOM.WW.MxGateway.Server.Security.Authorization;
|
||||||
using ZB.MOM.WW.MxGateway.Server.Sessions;
|
using ZB.MOM.WW.MxGateway.Server.Sessions;
|
||||||
using ZB.MOM.WW.MxGateway.Server.Workers;
|
using ZB.MOM.WW.MxGateway.Server.Workers;
|
||||||
|
using ZB.MOM.WW.MxGateway.IntegrationTests.TestSupport;
|
||||||
using Xunit.Abstractions;
|
using Xunit.Abstractions;
|
||||||
|
|
||||||
namespace ZB.MOM.WW.MxGateway.IntegrationTests;
|
namespace ZB.MOM.WW.MxGateway.IntegrationTests;
|
||||||
@@ -1595,9 +1596,4 @@ public sealed class WorkerLiveMxAccessSmokeTests(ITestOutputHelper output)
|
|||||||
CancellationToken cancellationToken) => Task.CompletedTask;
|
CancellationToken cancellationToken) => Task.CompletedTask;
|
||||||
}
|
}
|
||||||
|
|
||||||
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) { }
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user