test(tst-24): drive the .NET and Python clients against real in-process gRPC servers
TST-24 asked for per-client wire tests against a fake gateway. An audit first corrected the finding's premise: Go, Rust, and Java already had them — bufconn, a loopback tonic server, and InProcessServerBuilder respectively — each already asserting the round trip, the server-observed bearer header, and the ReplayGap sentinel. The two genuine gaps were .NET (every test substituted the transport interface; the test project had no server package at all) and Python (stub monkeypatching everywhere but one opt-in TLS test). Both now serve mxaccess_gateway.v1.MxAccessGateway over a real transport — Kestrel h2c and grpc.aio, each on an ephemeral loopback port — and drive the ordinary public client API against it. Only the gateway's behaviour is canned; the framing, serialization, metadata, and status codes are genuine. Four shapes each: full round trip with every reply field asserted, the authorization header as received by the server (including on the streaming RPC), the ReplayGap sentinel surfaced as the client's typed signal, and a real PERMISSION_DENIED mapping to the typed authorization error. The .NET client was only ever compiled in CI, never tested, so the portable job gains a dotnet test step. Fixes a bug the new tests caught on their first run: Python's connect() built the grpc.aio channel inside asyncio.to_thread, and a grpc.aio channel binds to the event loop current on the constructing thread, so every non-stub connection raised 'There is no current event loop in thread'. No mock-based test could see it, and the test guarding the off-loop behaviour patched create_channel and so asserted the bug. Split resolve_channel_security (blocking TOFU probe, off-loop) from create_channel (on-loop); the guard tests now assert both halves.
This commit is contained in:
Joseph Doherty
@@ -150,7 +150,7 @@ Sequence these together rather than piecemeal — several are one change set spa
|
||||
|
||||
- Close **CLI-24** and **CLI-34** as `Done` (incidentally fixed; evidence in [../50-clients.md](../50-clients.md)).
|
||||
- ~~When CLI-38 lands, close old **CLI-08** with a pointer here.~~ Done 2026-08-07: CLI-38 landed and old CLI-08 is now `Done` in the first-cycle tracker, pointing at [CLI-38](50-clients.md#cli-38--align-netgojava-on-hresult--0-lands-prior-cli-08-cures-the-doc-drift---medium--p1).
|
||||
- ~~When WRK-26 lands, its doc section also discharges the WorkerFrameProtocol gap~~ Done 2026-08-07: WRK-26 landed and **IPC-29** is discharged, pointing at [WRK-26](20-worker.md#wrk-26--write-priority-and-overflow-doc-drift-from-the-wrk-07-change---low--p1). ~~When TST-25 lands, revisit old **TST-05** (scheduled live smoke) and **TST-24** (client wire tests), which it unlocks.~~ TST-05 revisited 2026-08-10: `Partially done` in the first-cycle tracker — the `nightly-windev` job closes the scheduled-cadence half, but the finding's coverage-audit half stays open (the live suite reaches all six late-added COM commands and none of the five control commands). TST-24 revisited in the same change.
|
||||
- ~~When WRK-26 lands, its doc section also discharges the WorkerFrameProtocol gap~~ Done 2026-08-07: WRK-26 landed and **IPC-29** is discharged, pointing at [WRK-26](20-worker.md#wrk-26--write-priority-and-overflow-doc-drift-from-the-wrk-07-change---low--p1). ~~When TST-25 lands, revisit old **TST-05** (scheduled live smoke) and **TST-24** (client wire tests), which it unlocks.~~ TST-05 revisited 2026-08-10: `Partially done` in the first-cycle tracker — the `nightly-windev` job closes the scheduled-cadence half, but the finding's coverage-audit half stays open (the live suite reaches all six late-added COM commands and none of the five control commands). TST-24 revisited in the same change and closed `Done`: Go/Rust/Java already had real-server wire tests, and the two genuine gaps (.NET, Python) now have them plus a CI step.
|
||||
|
||||
## Change log
|
||||
|
||||
|
||||
@@ -175,7 +175,7 @@ Independent of the runner count, document the **no-cancel** reality (Gitea 1.26
|
||||
|
||||
## Cross-domain dependencies
|
||||
|
||||
- **TST-25 → old TST-05 / old TST-24:** the SSH-driven nightly is where the scheduled live-MXAccess smoke lands (closes old TST-05), and a working CI Windows tier unblocks wiring client wire-behavior tests into CI (old TST-24).
|
||||
- **TST-25 → old TST-05 / old TST-24:** the SSH-driven nightly is where the scheduled live-MXAccess smoke lands (closes old TST-05), and a working CI Windows tier unblocks wiring client wire-behavior tests into CI (old TST-24). *Followed up 2026-08-10:* old TST-05 is `Partially done` — the nightly closes the scheduling half, but the live suite still covers none of the five worker **control** commands; old TST-24 is `Done`, and its client wire tests turned out to need no Windows tier at all (they run in the `portable` job). See the first-cycle tracker.
|
||||
- **TST-25 ↔ IPC-24/IPC-25:** the nightly windev job is also the natural home for any Windows-side codegen verification the contracts/IPC remediation adds; coordinate job naming so both plans extend the same `windows-x86`/nightly jobs rather than adding parallel ones.
|
||||
- **TST-26 ⊂ TST-25:** same commit, by rule.
|
||||
- **TST-27:** ships in the cycle's P1 doc-drift batch alongside WRK-26 and CLI-42 (roadmap item 8); its `/browse` residual stays with TST-16 (prior cycle).
|
||||
|
||||
@@ -236,7 +236,7 @@ Full design + implementation for each row lives in the linked domain doc under i
|
||||
| TST-21 | Low | — | S | — | Not started | Log rotation configured but minimal |
|
||||
| TST-22 | Low | — | S | — | Not started | Config-shape JSON block omits documented keys |
|
||||
| TST-23 | Low | P2 | S | — | Done | Bidirectional `Session` RPC never built |
|
||||
| TST-24 | Low | P2 | M | TST-03 | Not started | Client wire behaviour has no automated verification. **Gate cleared:** TST-03 CI is Done (live and green 2026-07-10; Windows/x86 tier green 2026-07-13 via the TST-25/TST-26 SSH-driven windev job), so TST-24 is unblocked — deferred by choice now, not CI-gated |
|
||||
| TST-24 | Low | P2 | M | TST-03 | Done | Client wire behaviour has no automated verification — closed 2026-08-10. Go/Rust/Java already had real-server wire tests (the finding's premise was stale); the genuine gaps were .NET (transport-interface fake everywhere, no server package) and Python (stub monkeypatch everywhere but one opt-in TLS test). Added `MxGatewayClientWireTests` + `WireFakeGatewayServer` (Kestrel h2c) and `tests/test_wire_fake_gateway.py` (`grpc.aio` loopback), plus a `dotnet test` step for the .NET client in the `portable` CI job. Caught a real bug: Python `connect()` built the `grpc.aio` channel inside `asyncio.to_thread` and failed for every non-stub connection |
|
||||
|
||||
## Cross-cutting clusters
|
||||
|
||||
@@ -253,6 +253,7 @@ Findings the review flagged as one coordinated design pass — sequence them tog
|
||||
|
||||
| Date | Change |
|
||||
|---|---|
|
||||
| 2026-08-10 | **TST-24 → `Done`: per-client wire tests land for the two clients that lacked them** (branch `feat/tst-24-client-wire-tests`). Audit first corrected the finding's premise: **Go, Rust, and Java already had real-server wire tests** — `newBufconnClient`/`fakeGatewayServer` over `grpc/test/bufconn`, `spawn_fake_gateway` over a loopback `TcpListener` with tonic's `Server`, and `InProcessGateway`/`TestGatewayService` over `InProcessServerBuilder` — each already asserting the round trip, the server-observed `authorization` bearer header, and the `ReplayGap` sentinel. The real gaps were **.NET** (every test substituted `FakeGatewayTransport` for `IMxGatewayClientTransport`, and the test project had no server package) and **Python** (stub monkeypatching everywhere except one opt-in TLS test serving only `OpenSession`). Added `WireFakeGatewayServer` + `MxGatewayClientWireTests` (Kestrel h2c on `127.0.0.1:0` serving `MxAccessGatewayBase`; new `Grpc.AspNetCore.Server` 2.76.0 + `Microsoft.AspNetCore.App` refs on the test project) and `clients/python/tests/test_wire_fake_gateway.py` (`grpc.aio` server on `127.0.0.1:0`, no new deps). Four shapes each: full round trip with every reply field asserted, the bearer header **as received by the server** on the streaming RPC too, the `ReplayGap` sentinel surfaced as the client's typed signal, and a genuine `PERMISSION_DENIED` mapping to the typed authorization error. CI: the `portable` job only *built* the .NET client, so a `dotnet test` step was added. **The new tests immediately caught a shipped bug** — Python `GatewayClient.connect()`/`GalaxyRepositoryClient.connect()` constructed the `grpc.aio` channel inside `asyncio.to_thread`, which raises `RuntimeError: There is no current event loop in thread 'asyncio_0'` because a `grpc.aio` channel binds to the loop current on the constructing thread; every non-stub connection failed, and the one test guarding the off-loop behaviour (Client.Python-028) monkeypatched `create_channel` and so asserted the bug. Fixed by splitting `resolve_channel_security` (blocking TOFU probe, off-loop) from `create_channel` (on-loop), with the `-028` tests retargeted to assert both halves. Verified: .NET 133 passed/1 skipped (pre-existing live-gateway skip), Python 168 passed/1 skipped plus 6/6 opt-in TLS. Docs: `docs/GatewayTesting.md` § Client Wire Tests, `clients/dotnet/README.md`, `clients/python/README.md`. |
|
||||
| 2026-08-10 | **TST-05 revisited under the restored Windows tier → `Partially done`** (branch `feat/tst-24-client-wire-tests`, doc/tracker-only). The finding's **scheduling** half is closed: cycle-2 TST-25's `nightly-windev` job (cron `0 6 * * *`) runs `scripts/ci/run-windev-ci.sh live` → `windev-worker-ci.ps1 -Mode live`, which sets `MXGATEWAY_RUN_LIVE_MXACCESS_TESTS=1`, runs `WorkerLiveMxAccessSmokeTests` on windev after the x86 build/Worker.Tests/full-slnx steps, and files a Gitea issue when red. The **coverage-audit** half is *not* closed, and the audit the design asked for now has a negative answer: the suite's eight `[LiveMxAccessFact]`s cover all six late-added COM commands (`Suspend`, `Activate`, `AuthenticateUser`, `ArchestrAUserToId`, `AddBufferedItem`, `SetBufferedUpdateInterval`) but zero of the five control commands — `MxCommandKind.{Ping,GetSessionState,GetWorkerInfo,DrainEvents,ShutdownWorker}` appear nowhere in `WorkerLiveMxAccessSmokeTests.cs`, so the exact paths the Finding calls masked are still only proven against `FakeWorkerHarness` canned replies while the real implementations live in `Worker/Ipc/WorkerPipeSession.cs`. Residual work (two `[LiveMxAccessFact]`s, windev-only to author and verify) is specified in [60-testing-docs-gaps.md](60-testing-docs-gaps.md#tst-05--real-worker-controlcom-paths-verified-opt-in-only---medium--p1). |
|
||||
| 2026-07-10 | **TST-15 design fleshed out** (still `Not started` — design only, not implementation): `docs/plans/2026-07-10-dashboard-session-acl-tst15.md`. Resolves the crux the deferral left open — the dashboard is LDAP-identity (Admin/Viewer) while sessions are API-key-owned (`OwnerKeyId`), two disjoint identity domains — via a **session tag** sourced from the owning API key (rides in the existing `ApiKeyConstraints` JSON blob, no SQLite migration). Admin-sees-all; Viewer may `SubscribeSession` iff `session.Tags ∩ viewer.GrantedTags ≠ ∅` (new `Dashboard:GroupToTag` map → hub-token tag claims); untagged sessions Admin-only by default (`Dashboard:UntaggedSessionVisibility`). Includes the enforcement path (`HubTokenPayload.Tags` + `IDashboardSessionAcl` gate at `SubscribeSession`), task breakdown (epic Tasks 16–19), test plan incl. live-LDAP, and rejected alternatives (client-supplied tag; group→key-id map). Tracker + `60-testing-docs-gaps.md` TST-15 section point at the doc. **TST-03 investigated:** the CI never ran because the repo had **zero registered Gitea Actions runners** (Actions is enabled; runs are created on push/PR/nightly but fail instantly with nothing to execute them). A Mac runner proved the pipeline executes but cannot clone — this Gitea hands runners the internal `http://gitea:3000` URL, reachable only by a runner co-located on the gitea Docker network. Fix = run a co-located runner on the Gitea host (recipe prepared, `scratchpad/gitea-runner/setup-gitea-host-runner.sh`); pending host access. TST-03 stays `In review`. |
|
||||
| 2026-07-09 | **P2 Epic wrap — user decision: DEFER TST-15 + TST-24, close the epic.** Epic bucket result: 5 of 7 findings `Done` (CLI-15, CLI-04, CLI-30, TST-01, TST-04); **TST-15** and **TST-24** intentionally deferred to a follow-up (kept `Not started`, not `Won't fix` — they are gated, not rejected). **TST-15** (dashboard EventsHub per-session ACL) is epic Phase 4 — a real feature needing a new session-"tag" mechanism + dashboard group→tag config, not a mechanical fix; the `EventsHub` `TODO(per-session-acl)` stays, and the already-shipped **SEC-25** mitigation (tag *values* redacted from the dashboard mirror by default) means no sensitive payload leaks through the hub today regardless of the missing ACL — so deferring carries no value-leak risk. **TST-24** (per-client wire tests) depends on **TST-03** (CI), which is `In review` (YAML authored, never run on a Gitea runner) — no point wiring client tests into a pipeline that isn't live yet. Net P2: 35/38 `Done`; remaining = TST-15 (deferred feature), TST-24 (deferred, CI-gated), TST-14 (user deletes their own untracked gitignored `*-docs-*.md` files). |
|
||||
|
||||
@@ -457,6 +457,24 @@ If TST-02's interim mitigation (flip retention off) is chosen instead of impleme
|
||||
|
||||
## TST-24 — Client wire behaviour has no automated verification `Low` · `—`
|
||||
|
||||
> **Resolution 2026-08-10 (branch `feat/tst-24-client-wire-tests`): `Done`.** All five clients now drive their public API against a fake gateway served over a real gRPC transport, in the client's own default suite, and all five run in CI.
|
||||
>
|
||||
> **Corrected premise.** The Finding's "no in-process gateway integration tests" was already stale when it was re-verified: **Go, Rust, and Java had real-server wire tests**, not mocks — Go's `newBufconnClient`/`fakeGatewayServer` (`clients/go/mxgateway/client_session_test.go`) over `grpc/test/bufconn`, Rust's `spawn_fake_gateway` (`clients/rust/tests/client_behavior.rs`) over a loopback `TcpListener` with tonic's `Server`, and Java's `InProcessGateway`/`TestGatewayService` (`MxGatewayClientSessionTests.java`) over `InProcessServerBuilder`. Each already asserted the round trip, the `authorization` bearer header as *observed by the server*, and the `ReplayGap` sentinel. The cycle-2 re-verification cited `clients/python/tests/test_replay_gap.py` as evidence for "the other four clients still unit-test against mocks"; that generalized from Python to Go/Rust/Java incorrectly. `InProcessGatewayHarness` (the "template" the Impact paragraph names) is in fact the *thinner* of the Java harnesses — it serves only `streamEvents`/`closeSession` for the CLI tests.
|
||||
>
|
||||
> **Real gap, and what was built.** Two clients genuinely had none. **.NET** substituted `FakeGatewayTransport` for `IMxGatewayClientTransport` in every test, so not even the generated stub ran, and its test project had no server package at all. **Python** monkeypatched `MxAccessGatewayStub` everywhere except one opt-in TLS test that served only `OpenSession`. Both now have the pattern:
|
||||
> - `clients/dotnet/ZB.MOM.WW.MxGateway.Client.Tests/WireFakeGatewayServer.cs` + `MxGatewayClientWireTests.cs` — Kestrel h2c on `127.0.0.1:0` serving `MxAccessGateway.MxAccessGatewayBase`; needed new `Grpc.AspNetCore.Server` + `Microsoft.AspNetCore.App` references on the test project.
|
||||
> - `clients/python/tests/test_wire_fake_gateway.py` — a `grpc.aio` server on `127.0.0.1:0` serving `MxAccessGatewayServicer`; no new dependencies (`grpcio` is a runtime dep).
|
||||
>
|
||||
> Each covers the four shapes the Design asked for: round trip (`OpenSession` → `Invoke`/`Register` → `StreamEvents` → `CloseSession` with every reply field asserted), the bearer header as received by the server on the streaming RPC as well as the unary ones, the `ReplayGap` sentinel surfaced as the client's typed signal (TST-01), and a real `PERMISSION_DENIED` mapping to the typed authorization error.
|
||||
>
|
||||
> **CI.** The `portable` job previously only *built* the .NET client; a `dotnet test` step was added, so its wire tests actually run. Go/Rust/Python already ran their suites there and Java in the `java` job.
|
||||
>
|
||||
> **Bug this immediately caught** — the justification for the whole finding. `GatewayClient.connect()` / `GalaxyRepositoryClient.connect()` in the Python client were **broken for every real (non-stub) connection**: they built the `grpc.aio` channel inside `asyncio.to_thread`, and a `grpc.aio` channel binds to the event loop current on the constructing thread, so the worker thread raised `RuntimeError: There is no current event loop in thread 'asyncio_0'`. No mock-based test could see it — the one test asserting the off-loop behaviour (`Client.Python-028`) monkeypatched `create_channel` and therefore asserted the bug. Fixed by splitting `resolve_channel_security` (blocking TOFU probe, runs off-loop) from `create_channel` (must run on the loop thread), keeping the Client.Python-028 guarantee; the two `-028` tests were retargeted to assert both halves.
|
||||
>
|
||||
> **Deliberately out of scope.** Only the four session RPCs are served — the alarm feed (`StreamAlarms`, `QueryActiveAlarms`, `AcknowledgeAlarm`) and Galaxy browse are not, matching the Design's "full parity is out of scope". Java's `InProcessGatewayHarness` still lacks `openSession`/`invoke`; the client-module tests cover those shapes, so it was left alone.
|
||||
>
|
||||
> **Docs.** `docs/GatewayTesting.md` § Client Wire Tests (the cross-client pattern + per-client harness table), `clients/dotnet/README.md`, `clients/python/README.md`.
|
||||
|
||||
**Finding.** All five clients have unit tests (13/8/3/13/7 files for dotnet/go/rust/python/java) but no in-process or containerized gateway integration tests; the only cross-language verification is the operator-run `scripts/run-client-e2e-tests.ps1`. `CrossLanguageSmokeMatrixTests` checks shapes only.
|
||||
|
||||
**Impact.** Low-to-moderate: a gateway contract change can pass every default suite and break all five clients (partly mitigated by shared-proto codegen). The Java CLI already proves the cheap pattern — `InProcessGatewayHarness` (`stillpending.md` §8).
|
||||
|
||||
Reference in New Issue
Block a user