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.

Sign up for GitHub

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

Jump to bottom

Change PostgreSQLContainer wait behaviour (fix #317) #327

Merged
merged 4 commits into from
Apr 30, 2017
Merged

Change PostgreSQLContainer wait behaviour (fix #317) #327

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7f9ccb1
Update to latest postgres jdbc driver to make error logs visible
Apr 12, 2017
d1de096
Change PostgreSQLContainer to use WaitingConsumer for waiting instead…
Apr 12, 2017
5bfd7f4
Make OutputFrame return empty String if containing no bytes
kiview Apr 14, 2017
9b627c8
Change PostgreSQLContainer to use LogMessageWaitStrategy (#317)
Apr 21, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion modules/jdbc-test/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
<dependency>
<groupId>org.postgresql</groupId>
<artifactId>postgresql</artifactId>
<version>9.3-1101-jdbc41</version>
<version>42.0.0</version>
<scope>test</scope>
</dependency>
<dependency>
Expand Down
26 changes: 26 additions & 0 deletions modules/postgresql/src/main/java/org/testcontainers/containers/PostgreSQLContainer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
package org.testcontainers.containers;

import org.testcontainers.containers.output.OutputFrame;
import org.testcontainers.containers.output.WaitingConsumer;

import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
import java.util.function.Predicate;

/**
* @author richardnorth
*/
Expand Down Expand Up @@ -73,4 +80,23 @@ public SELF withPassword(final String password) {
this.password = password;
return self();
}

@Override
protected void waitUntilContainerStarted() {
String regEx = ".*database system is ready to accept connections.*\\s";
logger().info("Waiting for database to log '{}'", regEx);

WaitingConsumer waitingConsumer = new WaitingConsumer();
this.followOutput(waitingConsumer);


Predicate<OutputFrame> waitPredicate = outputFrame ->
outputFrame.getUtf8String().matches(regEx);
Copy link
Contributor

Choose a reason for hiding this comment

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

This is an out of the way way to mention this, but I've had this kind of code throw a NullPointerException when the regEx doesn't match any output. Pretty sure it's because getUtf8String returns null. I don't see the harm in having that return an empty string instead of null, but haven't tested it. With the code the way it is, I think we could use a null check here.

Copy link
Member Author

@kiview kiview Apr 13, 2017 •
edited

Choose a reason for hiding this comment

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

Good point, this means it's a problem for LogMessageWaitStrategy as well I assume?

@rnorth Would it be okay for getUtf8String() to return an empty String? I'd prefer it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, I'd prefer having getUtf8String return an empty string too. Otherwise I think we end up with null checks in all kinds of places, like https://github.com/testcontainers/testcontainers-java/blob/master/docs/usage/options.md#waiting-for-container-output-to-contain-expected-content.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, that sounds like a good idea - thanks.

Copy link
Member

Choose a reason for hiding this comment

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

I've had a look - I can't see how ToStringConsumer could return null though. It uses:

        byte[] bytes = stringBuffer.toByteArray();
        return new String(bytes, Charsets.UTF_8);

If stringBuffer is empty, bytes becomes an empty byte array. new String(...) does create an empty String (not null) if passed a zero-length byte array.

Have I missed something?

Copy link
Contributor

Choose a reason for hiding this comment

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

We're talking about OutputFrame::getUtf8String, not ToStringConsumer::getUtf8String...

Copy link
Member

Choose a reason for hiding this comment

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

Ah, sorry!

Copy link
Member Author

Choose a reason for hiding this comment

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

I've added the change in this PR.
Can someone with write access please retry the TravisCI build? The same build was green in my branch, so I think this might have been some problem with Travis or Docker in the build.


try {
waitingConsumer.waitUntil(waitPredicate, 120, TimeUnit.SECONDS, 2);
} catch (TimeoutException e) {
throw new ContainerLaunchException("Timed out waiting for log output matching '" + regEx + "'");
}
}
}