Skip to content

Fix: HTTP wait strategy does not take query params into account#2466

Merged
mdelapenya merged 1 commit intotestcontainers:mainfrom
benja-M-1:fix/wait-http-query-params
Apr 4, 2024
Merged

Fix: HTTP wait strategy does not take query params into account#2466
mdelapenya merged 1 commit intotestcontainers:mainfrom
benja-M-1:fix/wait-http-query-params

Conversation

@benja-M-1
Copy link
Contributor

@benja-M-1 benja-M-1 commented Apr 4, 2024 •
edited
Loading

What does this PR do?

When specifying a wait strategy using HTTP with query params, they are not sent to the container.
For example, to ensure the elasticmq container is ready, as advised in this issue, one must
poll the /?Action=ListQueues endpoint.
Currently this returns a 400 Bad request because the query parameters are not sent correctly to the endpoint.

The way the fix work is to parse the given path to create a url.URL instance and update the fields with the other values. This way the query parameters and the path are correclty handled.

Why is it important?

Related issues

Here is a reproduction with the elasticmq container:

req := testcontainers.ContainerRequest{
	Image:        "softwaremill/elasticmq-native:1.5.7",
	ExposedPorts: []string{"9324/tcp"},
	WaitingFor:   wait.ForHTTP("/?Action=ListQueues").WithPort("9324").WithStartupTimeout(3 * time.Second),
}

genericContainerReq := testcontainers.GenericContainerRequest{
	ContainerRequest: req,
	Started:          true,
}

c, err := testcontainers.GenericContainer(ctx, genericContainerReq)
if err != nil {
	return fmt.Errorf("failed to start repos-new elasticmq container: %w", err)
}

@benja-M-1 benja-M-1 requested a review from a team as a code owner April 4, 2024 11:04
@netlify
Copy link

netlify bot commented Apr 4, 2024 •
edited
Loading

✅ Deploy Preview for testcontainers-go ready!

Name Link
🔨 Latest commit d24da57
🔍 Latest deploy log https://app.netlify.com/sites/testcontainers-go/deploys/660e894e37784000080b8855
😎 Deploy Preview https://deploy-preview-2466--testcontainers-go.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@mdelapenya mdelapenya self-assigned this Apr 4, 2024
@mdelapenya mdelapenya added the bug An issue with the library label Apr 4, 2024
mdelapenya
mdelapenya approved these changes Apr 4, 2024
View reviewed changes
Copy link
Member

@mdelapenya mdelapenya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@mdelapenya mdelapenya merged commit 4a079a0 into testcontainers:main Apr 4, 2024
@benja-M-1 benja-M-1 deleted the fix/wait-http-query-params branch April 4, 2024 13:00
mdelapenya added a commit to coffeegoddd/testcontainers-go that referenced this pull request Apr 12, 2024
@mdelapenya
Merge branch 'main' into db/dolt
6dcebf2 
* main: (115 commits)
  chore: create TLS certs in a consistent manner (testcontainers#2478)
  chore(deps): bump idna from 3.6 to 3.7 (testcontainers#2480)
  Elasticsearch disable CA retrieval when ssl is disabled (testcontainers#2475)
  fix: handle dockerignore exclusions properly (testcontainers#2476)
  chore: prepare for next minor development cycle (0.31.0)
  chore: use new version (v0.30.0) in modules and examples
  Fix url creation to handle query params when using HTTP wait strategy (testcontainers#2466)
  fix: data race on container run (testcontainers#2345)
  fix: logging deadlock (testcontainers#2346)
  feat(k6):Add remote test scripts (testcontainers#2350)
  feat: optimizes file copies to and from containers (testcontainers#2450)
  fix(exec): updates the `Multiplexed` opt to combine stdout and stderr (testcontainers#2452)
  Upgrade neo4j module to use features from v0.29.1 of testcontainers-go (testcontainers#2463)
  bug:Fix AMQPS url (testcontainers#2462)
  chore: more compose updates in comments
  chore: use "docker compose" (v2) instead of "docker-compose" (v1) (testcontainers#2464)
  chore(deps): bump github/codeql-action from 2.22.12 to 3.24.9 (testcontainers#2459)
  refactor: Add Weaviate modules tests (testcontainers#2447)
  feat(exitcode): Add exit code sugar method (testcontainers#2342)
  feat: add module to support InfluxDB v1.x (testcontainers#1703)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mdelapenya mdelapenya mdelapenya approved these changes

Assignees

@mdelapenya mdelapenya

Labels

bug An issue with the library

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@benja-M-1 @mdelapenya