-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
CouchbaseContainer: Fix java.net.SocketException exception in stop() #859
CouchbaseContainer: Fix java.net.SocketException exception in stop() #859
Conversation
Motivation ========== When stop() was called without a prior call to createCouchbaseEnvironment(), it would throw a SocketException because stopCluster() would end up trying to initialize the cluster at the same time the container is being shut down. Modifications ============= Call stopCluster() before stopping the container, instead of in parallel. Result ====== stopCluster() might still call createCouchbaseEnvironment(), but the call succeeds and the environment is immediately cleaned up.
@@ -12,4 +12,11 @@ public void shouldUseCorrectDockerImage() { | |||
Assert.assertEquals(CouchbaseContainer.DOCKER_IMAGE_NAME + CouchbaseContainer.VERSION, | |||
couchbaseContainer.getDockerImageName()); | |||
} | |||
|
|||
@Test | |||
public void shouldStopWithoutThrowingException() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since this test does not test anything automatically, I would remove it, just to not add time to the Testcontainers test suite
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm fine with removing it, as long as it's understood that this test fails prior the main code change in this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah, ok, I thought that it is only about the logging, but if it fails with current master - let's keep it 👍
@dnault merged, thanks 👍 |
We have this out in a Release Candidate build (1.9.0-rc1) for anyone who is keen to try it! |
Motivation
When
stop()
was called without a prior call tocreateCouchbaseEnvironment()
, it would throw aSocketException
becausestopCluster()
would end up trying to initialize the cluster at the same time the container is being shut down.Modifications
Call
stopCluster()
before stopping the container, instead of in parallel.Result
stopCluster()
might still callcreateCouchbaseEnvironment()
, but the call succeeds and the environment is immediately cleaned up.