Skip to content

Expand documentation about Singleton Containers and lambdas #6587

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

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

pan-daniel
Copy link

@pan-daniel pan-daniel commented Feb 8, 2023

Hello! : ) This is my first commit here. Hopefully I did it right!

I have stumbled upon this while using TestContainers for a slow-spinning Docker image. Naturally I wanted to use a singleton container and I tried to make it work using the withResponsePredicate() method opposed to sleeping the health check. After some debugging I have found out that the reason it did not work, even though local uses of same predicates worked, was NoClassDefFoundError. It will be visible in HttpWaitStrategy.java, if you put breakpoint in here:

if (!responsePredicate.test(responseBody)) {
}

You can replicate it creating an abstract base class and trying to use any function derivative.

There are ways to work around it (at least I think so), but I have chosen instead to update documentation to save others time and to not introduce dirty hacks.

@pan-daniel pan-daniel requested a review from a team as a code owner February 8, 2023 20:57
Copy link
Member

@eddumelendez eddumelendez left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution @pan-daniel! I have left some comments.

@@ -13,7 +13,7 @@ try (GenericContainer container = new GenericContainer("imagename")) {
container.start();
// ... use the container
// no need to call stop() afterwards
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Why this?

Copy link
Author

Choose a reason for hiding this comment

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

A stray format, will revert.

Comment on lines +52 to +126
Please keep in mind that putting the container in a static block will influence the configuration of the container.
You will not be able to use methods such as `forResponsePredicate` by just providing a Lambda expression, you will have to use
anonymous classes, or alternatively provide them from non-abstract class.
This is not due to the limitation of `TestContainers`, but because of how they work under the hood, namely that lambdas get compiled
to static methods on a class level. Since your container is in a static block, the container gets created
before your parent and children classes get initialized and as such you cannot pass the reference to them.

Therefore, once again - it is advised to use anonymous classes in such case or full predicates coming in from different, non-abstract class.

Instead of:

```java
abstract class AbstractContainerBaseTest {

static final GenericContainer GENERIC_CONTAINER;

static {
GENERIC_CONTAINER = new GENERIC_CONTAINER(
new ImageFromDockerfile().waitingFor(Wait.forHttp('/path'))
.forStatusCode(200)
.forResponsePredicate(yourLambda -> yourLambda(here)) //This is never going to get executed due to NoClassDefFoundError
);
GENERIC_CONTAINER.start();
}
}
```

You can do an anonymous class:

```java
abstract class AbstractContainerBaseTest {

static final GenericContainer GENERIC_CONTAINER;

static {
GENERIC_CONTAINER = new GENERIC_CONTAINER(
new ImageFromDockerfile().waitingFor(Wait.forHttp('/path'))
.forStatusCode(200)
.forResponsePredicate(new Predicate<String>() {

@Override
public boolean test(String s) {
return yourConditionHere;
}
})
);

GENERIC_CONTAINER.start();
}
}
```
Or full predicate coming in from different class:

```java
abstract class AbstractContainerBaseTest {

static final GenericContainer GENERIC_CONTAINER;

static {
GENERIC_CONTAINER = new GENERIC_CONTAINER(
new ImageFromDockerfile().waitingFor(Wait.forHttp('/path'))
.forStatusCode(200)
.forResponsePredicate(PredicateHolder.getPredicate())
);
GENERIC_CONTAINER.start();
}
}

public class PredicateHolder {

public static Predicate<String> getPredicate() {
return yourLambda -> yourLambda(here);
}
}
```
Copy link
Member

Choose a reason for hiding this comment

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

There is an issue reported in JDK which describe what's happening here. So, static methods from other classes can not be called from a lambda in a static block. But, if those are used as a method reference then it will work.

Also, if a container class is created then this issue can be avoided we would encourage to do that

class HWContainer extends GenericContainer<HWContainer> {
    public HWContainer() {
        super("testcontainers/helloworld");
        withExposedPorts(8080);
        waitingFor(Wait.forHttp("/ping").forResponsePredicate(x -> Util.isContains(x)));
    }
}

I would like to make a note about this and give the proper context and recommendations. Also, Can you please move the inline code to actual code and use codeblock in the file to display them? I think code examples would be avoided if the note is quite clear.

Note: The project is Testcontainers not TestContainers :)

Copy link
Author

@pan-daniel pan-daniel Feb 17, 2023

Choose a reason for hiding this comment

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

Hey! Thanks for the response.

I do firmly believe that this is not about deadlocking per se (even though somewhat related), but rather that static block gets initialized before the initialization of a class - and since the lambda gets compiled to a static field, trying to reference something that does not exist is impossible.

The reason why I am thinking that is we will not even get to the class initialization yet, as the container is just getting started.

Could you point me in the direction why do you believe that this is the deadlock situation?

Also, when you respond I will:

  1. Move out the examples to examples/ module. Question here, does it need to be completely valid code with imports etc. or can I just copy this somewhat pseudocode?
  2. Reference the JDK bug/discussion.
  3. Would you rather me deleting some examples rather than including them at all? From your message I got a feeling you'd delete some of them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants