testcontainers / testcontainers-java

Testcontainers is a Java library that supports JUnit tests, providing lightweight, throwaway instances of common databases, Selenium web browsers, or anything else that can run in a Docker container.
https://testcontainers.org
MIT License
8.04k stars 1.66k forks source link

Stop container that doesn't match wait strategy #9474

Closed ssheikin closed 2 weeks ago

ssheikin commented 3 weeks ago
new GenericContainer<>(IMAGE_NAME)
    .withStartupAttempts(3)
    .waitingFor(forLogMessage("this text does not exist in logs", 1)
        .withStartupTimeout(Duration.ofSeconds(5)))

Leaves not closed containers in case of timeout and retry.

For more extensive description see https://github.com/testcontainers/testcontainers-java/issues/2877 which this PR fixes.

reportLeakedContainers adapted from https://github.com/trinodb/trino/pull/20297 https://github.com/trinodb/trino/pull/21280

eddumelendez commented 3 weeks ago

Can you please elaborate about the change?

ssheikin commented 3 weeks ago

cc @nineinchnick

nineinchnick commented 3 weeks ago

Would this solve https://github.com/testcontainers/testcontainers-java/issues/2877 ?

ssheikin commented 3 weeks ago

@nineinchnick thanks for pointing out to the existing issue. I've added tests and updated the PR description @eddumelendez please review

ssheikin commented 3 weeks ago

@kiview @bsideup @rnorth Please take a look.

ssheikin commented 2 weeks ago

@eddumelendez @kiview @bsideup @rnorth Could you please indicate when you might have a chance to take a look?

eddumelendez commented 2 weeks ago

Hi @ssheikin, with all due respect, please stop tagging people in this PR. It is only creating noise in emails, slack and notifications to members who are taking a break from the project. As I mentioned via DM, when a PR is raised the team is notified. Please also be aware that there are other PRs, tasks that we are currently working on.

We really appreciate your contribution and it is going to be reviewed but give some time meanwhile it is triaged and discussed.

eddumelendez commented 2 weeks ago

Thanks for your contribution, @ssheikin.

ssheikin commented 2 weeks ago

Thanks for getting this across the finish line! Could you please let us know when the next release is scheduled? We’re excited to use withStartupAttempts for containers that occasionally fail to start.