[outofcoffee]

Purpose

This PR adds a pluggable wait strategy, to enable containers to be checked for readiness using arbitrary logic.

It also adds an HTTP(S) wait strategy to wait for a particular endpoint to be available.

Implementation

• Refactored existing waitForListeningPort(...) logic into its own class.
• Adds new HTTP(S) wait implementation

Examples

Wait for 200 OK:

@ClassRule
public static GenericContainer elasticsearch =
    new GenericContainer("elasticsearch:2.3")
               .withExposedPorts(9200)
               .waitingFor(Wait.forHttp("/all"));

Wait for arbitrary status code on an HTTPS endpoint:

@ClassRule
public static GenericContainer elasticsearch =
    new GenericContainer("elasticsearch:2.3")
               .withExposedPorts(9200)
               .waitingFor(Wait.forHttp("/all")
               .forStatusCode("301)
               .usingTls());

[rnorth]

Set to Wait.defaultWaitStrategy() and mark @ NonNull on the field and withWaitStrategy parameter? Then no need for a null check at time of use...

[outofcoffee]

Wanted to avoid additional assignment but happy to change if you feel strongly :)

[rnorth]

Thanks for this! Just a few comments so far, but looking good!
Please can you add some tests some time ;) ?

BTW the CI builds have both failed on the Virtuoso container module - it's possibly unrelated but I'm looking into it...

[outofcoffee]

@rnorth the tests failed because I rebased after pushing ;-) Have switched to pushing additional commits now to track comments above.

[rnorth]

Aha! No worries then :)

On 18 Apr 2016, at 21:16, Pete Cornish notifications@github.com wrote:

@rnorth https://github.com/rnorth the tests failed because I rebased after pushing ;-) Have switched to pushing additional commits now to track comments above.

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub #133 (comment)

[bsideup]

.forContainer() should return new instance or should be removed in favour of an argument in waitUntilReady and stateless strategies.

Motivation - current implementation creates a false positive feeling that we can re-use existing strategy for different containers, but we can't.

[outofcoffee]

We could fix this by moving the container assignment to the constructor or, alternatively, throwing an IllegalStateException on calling forContainer more than once.

@rnorth thoughts?

[bsideup]

constructor sounds fine for me 👍

[outofcoffee]

Ah, no, on second thought that would break chaining, as the container ref isn't initialised yet when declaring the class rule field. Might just make the forContainer method private and remove it from the interface. The call from within GenericContainer will still work because AbstractWaitStrategy is an inner class.

[rnorth]

FWIW I'll accept this PR with only Travis CI passing, as the CircleCI builds are not receiving the patch on master to disable Virtuoso build. FYI @outofcoffee, when you're ready.

[outofcoffee]

Tests failing because of #140

[outofcoffee]

@rnorth as discussed; Travis build is passing, but Circle still having issues.

[rnorth]

Looks good to me - will merge.