Skip to content

Comments

Use HttpWaitStrategy for SimpleNginxTest#590

Merged
rnorth merged 1 commit intomasterfrom
fix-nginx-test-failures
Feb 25, 2018
Merged

Use HttpWaitStrategy for SimpleNginxTest#590
rnorth merged 1 commit intomasterfrom
fix-nginx-test-failures

Conversation

@rnorth
Copy link
Member

@rnorth rnorth commented Feb 24, 2018

To ensure proper waits and avoid race conditions (flaky test)

Since the Gradle migration we've been seeing a lot of the following stack trace. I have no idea why, but I suspect an improvement in test execution speed might have uncovered a previously hidden timing bug in our tests:

Gradle Test Executor 7 > org.testcontainers.junit.SimpleNginxTest > testSimple FAILED
   java.net.SocketException: Unexpected end of file from server
       at sun.net.www.http.HttpClient.parseHTTPHeader(HttpClient.java:851)
       at sun.net.www.http.HttpClient.parseHTTP(HttpClient.java:678)
       at sun.net.www.http.HttpClient.parseHTTPHeader(HttpClient.java:848)
       at sun.net.www.http.HttpClient.parseHTTP(HttpClient.java:678)
       at sun.net.www.protocol.http.HttpURLConnection.getInputStream0(HttpURLConnection.java:1587)
       at sun.net.www.protocol.http.HttpURLConnection.getInputStream(HttpURLConnection.java:1492)
       at org.testcontainers.junit.SimpleNginxTest.testSimple(SimpleNginxTest.java:49)

This PR simply attaches an HttpWaitStrategy to the test. We could have added this to the NginxContainer itself, but I felt that would be counterproductive: users will typically need to customise the http wait strategy according to their nginx configuration, so we should leave it to them.

To ensure proper waits and avoid race conditions (flaky test)
@rnorth rnorth merged commit 14159e7 into master Feb 25, 2018
@kiview
Copy link
Member

kiview commented Feb 25, 2018

@rnorth I feel like having a HttpWaitStrategy as the default would be helpful (I think many first time users of TC don't even know about wait strategies in the beginning). Or what are the special use cases where users would need to customize the Strategy? Shouldn't status code 200 suffice?

@kiview kiview deleted the fix-nginx-test-failures branch February 25, 2018 12:21
@bsideup bsideup added this to the next milestone Apr 4, 2018
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.

3 participants