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

Support restart on 5 containers with previously fixed ports #895

Merged
merged 5 commits into from
Jan 13, 2025

Conversation

stscoundrel
Copy link
Contributor

As pointed out in #893 (comment), individual testcontainers should not store their mapped ports in class members, as the mapped port will change in restart.

There are many test containers that currently use that pattern of port mapping, meaning they wont work after a restart. This PR fixes five of them:

  • MariaDB
  • MySQL
  • ScyllaDB
  • Cassandra
  • ElasticSearch

There are few others that have similar port mapping (Neo4j, ArangoDB, PostgreSQL etc...), but they utilize wait strategies that do not currently work with restarts. For example, Neo4j expects a certain log message to appear before it is "ready", but that log message only appears on initial start, not on restart. This seems a bit more complex to solve, so I left them outside of scope of this PR for now. Will probably take another look at it later in separate PR, as there were previous commits solving this very issue, but they were later overwritten in another fix.

For successful CI run, see PR in this fork

Copy link

netlify bot commented Jan 12, 2025

Deploy Preview for testcontainers-node ready!

Name Link
🔨 Latest commit a3221f5
🔍 Latest deploy log https://app.netlify.com/sites/testcontainers-node/deploys/678413c7784d0f0008c3506f
😎 Deploy Preview https://deploy-preview-895--testcontainers-node.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.

@cristianrgreco
Copy link
Collaborator

Amazing, thanks @stscoundrel !

@cristianrgreco cristianrgreco added bug Something isn't working patch Backward compatible bug fix labels Jan 13, 2025
@cristianrgreco cristianrgreco merged commit 742e839 into testcontainers:main Jan 13, 2025
179 checks passed
@stscoundrel stscoundrel deleted the on-demand-ports branch January 13, 2025 16:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working patch Backward compatible bug fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants