fix(test): skip the Redis container tests when Docker is unreachable (#526) - #529
Merged
Conversation
`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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RVMGTQ2k3CwrkcannhZ9sA
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
RedisIngestionStreamIntegrationTestsis meant to report as skipped when no container runtime is reachable, but both tests failed on a Docker-less machine, making a plaindotnet test Proxytrace.slnred for reasons unrelated to the change under test. The container was built one line above thetry, and Testcontainers validates a builder by resolving and pinging the Docker endpoint — sonew RedisBuilder(RedisImage).Build()is where the throw actually happens and thecatch (Exception ex) when (!DockerRequired)guard never saw it. This movesBuild()inside the guarded block so endpoint-resolution failures take the same skip path as start failures.Closes #526
Changes
Proxytrace.Messaging.Tests/RedisIngestionStreamIntegrationTests.cs— construct theRedisContainerinside the guardedtry, hold it in a nullable local, and null-check before disposing on the skip path. No!suppression needed:Assert.Inconclusiveis[DoesNotReturn], so the non-null return flows cleanly.docs/testing.md— the container-backed section now states that the guard must wrapBuild(), not justStartAsync, and why..claude/skills/test/SKILL.md— same one-line rule in the container-backed-tests section, so the next such suite does not re-derive it.Verification
dotnet build Proxytrace.Messaging.Tests -warnaserror— 0 warnings, 0 errors.dotnet test Proxytrace.Messaging.Tests—Failed: 0, Passed: 11, Skipped: 2(wasFailed: 2before the change).PROXYTRACE_REQUIRE_DOCKER_TESTS=1 dotnet test Proxytrace.Messaging.Tests --filter "FullyQualifiedName~RedisIngestionStreamIntegrationTests"—Failed: 2, confirming the enforce path still turns a missing runtime into a hard failure, so CI cannot lose the coverage silently.docs/testing.mdand thetestskill updated. NoCHANGELOG.mdentry — the changelog is for user-facing changes and this is test-harness-only, with no product behaviour affected.🤖 Generated with Claude Code
https://claude.ai/code/session_01RVMGTQ2k3CwrkcannhZ9sA
Generated by Claude Code