Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DockerComposeEnvironment missing validation for wait strategies? #837

Open
piotrgajow-gsg opened this issue Sep 30, 2024 · 1 comment
Open
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@piotrgajow-gsg
Copy link

Expected Behaviour

I would expect that test containers would log error when I use withWaitStrategy with not existing container name
OR
that the documentation highlights where should I serviceName vs containerName explaining difference between those two to avoid confusion and related issues.

Actual Behaviour

Testcontainers 'silently fails' to override the wait for strategy for given container, causing the startup to fail.

Testcontainer Logs
N/A

Steps to Reproduce

  1. docker-compose.yml
version: '3.7'
services:
...
  mockServer:
    image: mockserver/mockserver:mockserver-5.13.0
    command: -serverPort 1080 -logLevel DEBUG
    ports:
      - "1080:1080"
...      
  1. Run the code
const environment = await new  ('./', 'docker-compose.yml')
        .withBuild()
        .withWaitStrategy('mockServer', Wait.forLogMessage(/.+started on port.+/))
        .up(['mockServer']);
  1. Observe error
Error: Port 1080 not bound after 60000ms

Notes:
The documentation under https://node.testcontainers.org/features/compose/ uses exactly the same value for service name and container name (both using suffix -1) instead of highlighting that those are different things. As a result users might use the same value in their code, which might lead to confusing results. It appears as if the user is not able to override the default strategy of waiting until port is bound - no error saying that the container with given name does not exist, but error about port not being bound.
Switching the code to use withWaitStrategy('mockServer-1', Wait.forLogMessage(/.+started on port.+/)) fixes the issue.
On the other hand, changing the code to use .up(['mockServer-1']) results in error no such service: mockServer-1.

Thus, couple of improvements that could be considered:

  • Log error/warning if a wait strategy was assigned to a container that does not exist, e.g. no such container: mockServer.
  • Update the documentation to highlight difference between service name and container name
  • Switch the API to only allow use either service name and container name

Environment Information

  • Operating System: MacOS 14.2
  • Docker Version: 27.2.0
  • Node version: 20.11.0
  • Testcontainers version: 10.13.1
@cristianrgreco
Copy link
Collaborator

Good point, PR welcome

@cristianrgreco cristianrgreco added enhancement New feature or request good first issue Good for newcomers labels Nov 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants