Repository navigation
fix: Keep the query string in the HTTP wait strategy path - #1807
HofmeisterAn merged 2 commits into
Conversation
HttpWaitStrategy passed the configured path to UriBuilder as the path
component. UriBuilder escapes "?", so ForPath("/health?ready=1") sent
"GET /health%3Fready=1" and the container never received the query
string. Endpoints that need a query parameter returned an unexpected
status and the wait strategy did not succeed.
Split the configured value at the first "?" and set the remainder as
the UriBuilder query.
✅ Deploy Preview for testcontainers-dotnet ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
No actionable comments were generated in the recent review.
|
| Layer / File(s) | Summary |
|---|---|
Separate query from request path src/Testcontainers/Configurations/WaitStrategies/HttpWaitStrategy.cs, tests/Testcontainers.Tests/Unit/Configurations/WaitUntilHttpRequestIsSucceededTest.cs |
UntilAsync assigns the path and query separately to UriBuilder. A unit test checks a request to /health?ready=1&probe=startup, the successful wait, and the logged request. |
Priority:
Estimated code review effort: 2 (Simple) | ~10 minutes
Change: Bug fix
Merge Risk: 
Low · up to 47fd0
-
Only unusual query values beginning with a question mark remain affected; ordinary query strings are handled correctly.
Pre-merge checks: 
5
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 1 functions across 2 files.
-
Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
A rabbit checks the health route,
With query strings carried clear.
The wait succeeds, the logs confirm,
Each parameter arrives here.
I twitch my nose and hop away,
While paths and queries persevere.
Comment @coderabbitai help to get the list of available commands.
73d9766
into
testcontainers:develop
What does this PR do?
HttpWaitStrategynow splits the value passed toForPath(string)at the first?and sets the remainder as theUriBuilderquery. Before this change, the whole value went to theUriBuilderpath, which escapes?, soForPath("/health?ready=1")sentGET /health%3Fready=1.The PR adds
HttpWaitStrategySendsQueryStringtoWaitUntilHttpRequestIsSucceededTest. It uses the existing socat fixture, which logs the raw request, and checks that the request line contains the query string.Why is it important?
Some health endpoints need a query parameter. Without the query string they return 404 or another unexpected status, so the wait strategy retries until it times out.
Related issues
How to test this PR
dotnet test tests/Testcontainers.Tests --filter "FullyQualifiedName~WaitUntilHttpRequestIsSucceededTest"(needs Docker). The new test fails ondevelopbecause the request line isGET /health%3Fready=1&probe=startup HTTP/1.1, and passes with this change.Summary by CodeRabbit