From 9638b4d3e2cfc3ad172d400e490f2e37c01769c6 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 7 Aug 2026 20:11:32 +0000 Subject: [PATCH] fix(test): skip the Redis container tests when Docker is unreachable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `RedisIngestionStreamIntegrationTests.StartRedisAsync` built the Redis container outside the `try`, so the guard that turns a missing container runtime into `Assert.Inconclusive` only covered `StartAsync`. Testcontainers validates a builder by resolving and pinging the Docker endpoint, which means `new RedisBuilder(RedisImage).Build()` is where a Docker-less machine actually throws — the catch never saw it, and a plain `dotnet test Proxytrace.sln` reported two failures unrelated to the change under test. Move `Build()` inside the guarded block and null-check the container before disposing it on the skip path, so endpoint-resolution failures take the same skip route as start failures. `PROXYTRACE_REQUIRE_DOCKER_TESTS=1` still turns both into hard failures, so CI cannot lose the coverage silently. Verified with no Docker socket: 2 skipped / 11 passed unset, 2 failed with the variable set. Documented the builder rule in docs/testing.md and the test skill so the next container-backed suite does not re-derive it. Refs #526 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01RVMGTQ2k3CwrkcannhZ9sA --- .claude/skills/test/SKILL.md | 4 +++- .../RedisIngestionStreamIntegrationTests.cs | 12 ++++++++++-- docs/testing.md | 5 +++++ 3 files changed, 18 insertions(+), 3 deletions(-) diff --git a/.claude/skills/test/SKILL.md b/.claude/skills/test/SKILL.md index 38fb3680..296918c7 100644 --- a/.claude/skills/test/SKILL.md +++ b/.claude/skills/test/SKILL.md @@ -254,7 +254,9 @@ real service in a throwaway container instead: Rules: build the container **inside the test method** (no shared fixture — the isolation rule still applies), pin the image to the tag `docker-compose.yml` runs, and `Assert.Inconclusive` when no runtime is reachable *unless* `PROXYTRACE_REQUIRE_DOCKER_TESTS` is set — `dotnet test` must not -require Docker locally, while CI must not skip silently. Full rationale in +require Docker locally, while CI must not skip silently. The skip guard must wrap the builder's +`Build()` too, not only `StartAsync`: `Build()` pings the Docker endpoint, so that is where a +missing runtime actually throws. Full rationale in [`docs/testing.md`](../../../docs/testing.md#container-backed-tests). Default to a mock; this is a supplement for the cases a mock structurally cannot cover. diff --git a/Proxytrace.Messaging.Tests/RedisIngestionStreamIntegrationTests.cs b/Proxytrace.Messaging.Tests/RedisIngestionStreamIntegrationTests.cs index a4545860..183f3f96 100644 --- a/Proxytrace.Messaging.Tests/RedisIngestionStreamIntegrationTests.cs +++ b/Proxytrace.Messaging.Tests/RedisIngestionStreamIntegrationTests.cs @@ -131,9 +131,13 @@ public async Task ConsumeAsync_WithEntryPendingOnADeadConsumer_ReclaimsItViaAuto private static async Task StartRedisAsync(CancellationToken cancellationToken) { - RedisContainer container = new RedisBuilder(RedisImage).Build(); + RedisContainer? container = null; try { + // Build() must stay inside the guard: it validates the builder by resolving and pinging + // the Docker endpoint, so on a machine without a runtime the throw happens here and + // never reaches StartAsync — a skip guard wrapping only the start never fires. + container = new RedisBuilder(RedisImage).Build(); await container.StartAsync(cancellationToken); } // Docker unavailable surfaces as anything from a socket-level HttpRequestException to a @@ -142,7 +146,11 @@ private static async Task StartRedisAsync(CancellationToken canc // runtime is guaranteed (CI), nothing is swallowed and the failure is reported as-is. catch (Exception ex) when (!DockerRequired) { - await container.DisposeAsync(); + if (container is not null) + { + await container.DisposeAsync(); + } + Assert.Inconclusive( $"Skipping the real-Redis transport test — no usable container runtime: {ex.Message}"); } diff --git a/docs/testing.md b/docs/testing.md index 1a4878e3..d6efb86d 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -95,6 +95,11 @@ Docker dependency. Setting **`PROXYTRACE_REQUIRE_DOCKER_TESTS=1|true`** inverts failure is then a real failure. CI's `backend` job sets it (see [`ci.md`](ci.md)) so the coverage can never be lost silently, which is the same class of false-green the tests exist to close. +The guard has to wrap the builder's `Build()` call, not just `StartAsync`: Testcontainers validates +a builder by resolving and pinging the Docker endpoint, so on a machine without a runtime the throw +happens at `Build()` and a `try` that starts one line later never sees it (#526). Construct the +container inside the guarded block and null-check it before disposing on the skip path. + Run them locally like any other test — with Docker up they just run: ```bash