From b3fd0b4ebbe123aab8364940343a7db007b033a6 Mon Sep 17 00:00:00 2001 From: Paul Nepywoda Date: Wed, 16 Oct 2019 16:15:16 -0700 Subject: [PATCH 1/8] doesnt work - jupiter always calls after anyway --- ...DockerComposeExtensionIntegrationTest.java | 6 +++ ...sionIntegrationTestWithFailingCommand.java | 45 +++++++++++++++++++ .../docker-compose-with-failing-command.yaml | 5 +++ .../compose/DockerComposeExtension.java | 7 ++- .../docker/compose/connection/Container.java | 3 +- 5 files changed, 64 insertions(+), 2 deletions(-) create mode 100644 docker-compose-junit-jupiter/src/integrationTest/java/com/palantir/docker/compose/DockerComposeExtensionIntegrationTestWithFailingCommand.java create mode 100644 docker-compose-junit-jupiter/src/integrationTest/resources/docker-compose-with-failing-command.yaml diff --git a/docker-compose-junit-jupiter/src/integrationTest/java/com/palantir/docker/compose/DockerComposeExtensionIntegrationTest.java b/docker-compose-junit-jupiter/src/integrationTest/java/com/palantir/docker/compose/DockerComposeExtensionIntegrationTest.java index b107f67bf..f620224bb 100644 --- a/docker-compose-junit-jupiter/src/integrationTest/java/com/palantir/docker/compose/DockerComposeExtensionIntegrationTest.java +++ b/docker-compose-junit-jupiter/src/integrationTest/java/com/palantir/docker/compose/DockerComposeExtensionIntegrationTest.java @@ -49,4 +49,10 @@ public void an_external_port_exists() { public void container_stays_up_between_tests() { assertThat(docker.containers().container("db").port(5432).getExternalPort()).isEqualTo(port); } + + @Test + @Order(3) + public void calls_after_on_exception() { + assertThat(docker.containers().container("db").port(5432).getExternalPort()).isEqualTo(port); + } } diff --git a/docker-compose-junit-jupiter/src/integrationTest/java/com/palantir/docker/compose/DockerComposeExtensionIntegrationTestWithFailingCommand.java b/docker-compose-junit-jupiter/src/integrationTest/java/com/palantir/docker/compose/DockerComposeExtensionIntegrationTestWithFailingCommand.java new file mode 100644 index 000000000..378c65f97 --- /dev/null +++ b/docker-compose-junit-jupiter/src/integrationTest/java/com/palantir/docker/compose/DockerComposeExtensionIntegrationTestWithFailingCommand.java @@ -0,0 +1,45 @@ +/* + * (c) Copyright 2019 Palantir Technologies Inc. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.palantir.docker.compose; + +import static org.assertj.core.api.Assertions.assertThat; + +import com.palantir.docker.compose.configuration.DockerComposeFiles; +import com.palantir.docker.compose.connection.State; +import com.palantir.docker.compose.connection.waiting.HealthChecks; +import java.io.IOException; +import org.joda.time.Duration; +import org.junit.jupiter.api.Order; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +public class DockerComposeExtensionIntegrationTestWithFailingCommand { + + @RegisterExtension + public static final DockerComposeExtension docker = DockerComposeExtension.builder() + .files(DockerComposeFiles.from("src/integrationTest/resources/docker-compose-with-failing-command" + + ".yaml")) + .waitingForService("cmd", HealthChecks.toRespondOverHttp(9999, _$ -> "http://will-fail"), + Duration.millis(50)) + .build(); + + @Test + @Order(1) + public void calls_after_on_exception() throws IOException, InterruptedException { + assertThat(docker.containers().container("cmd").state()).isEqualTo(State.DOWN); + } +} diff --git a/docker-compose-junit-jupiter/src/integrationTest/resources/docker-compose-with-failing-command.yaml b/docker-compose-junit-jupiter/src/integrationTest/resources/docker-compose-with-failing-command.yaml new file mode 100644 index 000000000..1c52447e9 --- /dev/null +++ b/docker-compose-junit-jupiter/src/integrationTest/resources/docker-compose-with-failing-command.yaml @@ -0,0 +1,5 @@ +cmd: + image: appropriate/nc + command: /bin/sh -c 'sleep 1000' + ports: + - "9999:9999" diff --git a/docker-compose-junit-jupiter/src/main/java/com/palantir/docker/compose/DockerComposeExtension.java b/docker-compose-junit-jupiter/src/main/java/com/palantir/docker/compose/DockerComposeExtension.java index 2b8834a2c..d9497cf1c 100644 --- a/docker-compose-junit-jupiter/src/main/java/com/palantir/docker/compose/DockerComposeExtension.java +++ b/docker-compose-junit-jupiter/src/main/java/com/palantir/docker/compose/DockerComposeExtension.java @@ -32,7 +32,12 @@ public abstract class DockerComposeExtension extends DockerComposeManager @Override public void beforeAll(ExtensionContext unused) throws IOException, InterruptedException { - before(); + try { + before(); + } catch (RuntimeException e) { + after(); + throw e; + } } @Override diff --git a/docker-compose-rule-core/src/main/java/com/palantir/docker/compose/connection/Container.java b/docker-compose-rule-core/src/main/java/com/palantir/docker/compose/connection/Container.java index 1fa862ab0..6256680e8 100644 --- a/docker-compose-rule-core/src/main/java/com/palantir/docker/compose/connection/Container.java +++ b/docker-compose-rule-core/src/main/java/com/palantir/docker/compose/connection/Container.java @@ -88,7 +88,8 @@ public DockerPort port(int internalPort) { .stream() .filter(port -> port.getInternalPort() == internalPort) .findFirst() - .orElseThrow(() -> new IllegalArgumentException("No internal port '" + internalPort + "' for container '" + containerName + "': " + portMappings)); + .orElseThrow(() -> new IllegalArgumentException("No internal port '" + internalPort + "' " + + "for container '" + containerName + "': " + portMappings.get())); } public void start() throws IOException, InterruptedException { From 5405420f8e5afb280273a18dd62b857a128a2c39 Mon Sep 17 00:00:00 2001 From: Paul Nepywoda Date: Tue, 29 Oct 2019 15:23:20 -0700 Subject: [PATCH 2/8] mockito test --- .../compose/DockerComposeExtension.java | 7 +- .../compose/DockerComposeExtensionTest.java | 71 +++++++++++++++++++ 2 files changed, 77 insertions(+), 1 deletion(-) create mode 100644 docker-compose-junit-jupiter/src/test/java/com/palantir/docker/compose/DockerComposeExtensionTest.java diff --git a/docker-compose-junit-jupiter/src/main/java/com/palantir/docker/compose/DockerComposeExtension.java b/docker-compose-junit-jupiter/src/main/java/com/palantir/docker/compose/DockerComposeExtension.java index d9497cf1c..01ebddcd7 100644 --- a/docker-compose-junit-jupiter/src/main/java/com/palantir/docker/compose/DockerComposeExtension.java +++ b/docker-compose-junit-jupiter/src/main/java/com/palantir/docker/compose/DockerComposeExtension.java @@ -30,19 +30,24 @@ public abstract class DockerComposeExtension extends DockerComposeManager implements BeforeAllCallback, AfterAllCallback { + private boolean hasCalledAfterMethod; + @Override public void beforeAll(ExtensionContext unused) throws IOException, InterruptedException { try { before(); } catch (RuntimeException e) { after(); + hasCalledAfterMethod = true; throw e; } } @Override public void afterAll(ExtensionContext unused) { - after(); + if (!hasCalledAfterMethod) { + after(); + } } public static Builder builder() { diff --git a/docker-compose-junit-jupiter/src/test/java/com/palantir/docker/compose/DockerComposeExtensionTest.java b/docker-compose-junit-jupiter/src/test/java/com/palantir/docker/compose/DockerComposeExtensionTest.java new file mode 100644 index 000000000..22856a01b --- /dev/null +++ b/docker-compose-junit-jupiter/src/test/java/com/palantir/docker/compose/DockerComposeExtensionTest.java @@ -0,0 +1,71 @@ +/* + * (c) Copyright 2019 Palantir Technologies Inc. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.palantir.docker.compose; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import com.palantir.docker.compose.configuration.DockerComposeFiles; +import com.palantir.docker.compose.connection.waiting.ClusterWait; +import com.palantir.docker.compose.events.EventConsumer; +import java.io.IOException; +import java.util.List; +import java.util.concurrent.atomic.AtomicInteger; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtensionContext; +import org.mockito.Mockito; + +public class DockerComposeExtensionTest { + + @Test + public void calls_after_only_once() throws IOException, InterruptedException { + AtomicInteger count = new AtomicInteger(); + DockerComposeExtension dockerComposeExtension = new DockerComposeExtension() { + + @Override + public void before() { + throw new IllegalStateException("some error"); + } + + @Override + public void after() { + count.incrementAndGet(); + } + + @Override + public DockerComposeFiles files() { + return null; + } + + @Override + protected List clusterWaits() { + return null; + } + + @Override + protected List eventConsumers() { + return null; + } + }; + + ExtensionContext extensionContext = Mockito.mock(ExtensionContext.class); + assertThatThrownBy(() -> dockerComposeExtension.beforeAll(extensionContext)) + .isInstanceOf(IllegalStateException.class); + dockerComposeExtension.afterAll(extensionContext); + assertThat(count.get()).isEqualTo(1); + } +} From 00a42e76ee4ece1c3f8f604507894abe5a99ab1a Mon Sep 17 00:00:00 2001 From: Paul Nepywoda Date: Tue, 29 Oct 2019 15:23:55 -0700 Subject: [PATCH 3/8] remove integration tests --- ...sionIntegrationTestWithFailingCommand.java | 45 ------------------- .../docker-compose-with-failing-command.yaml | 5 --- 2 files changed, 50 deletions(-) delete mode 100644 docker-compose-junit-jupiter/src/integrationTest/java/com/palantir/docker/compose/DockerComposeExtensionIntegrationTestWithFailingCommand.java delete mode 100644 docker-compose-junit-jupiter/src/integrationTest/resources/docker-compose-with-failing-command.yaml diff --git a/docker-compose-junit-jupiter/src/integrationTest/java/com/palantir/docker/compose/DockerComposeExtensionIntegrationTestWithFailingCommand.java b/docker-compose-junit-jupiter/src/integrationTest/java/com/palantir/docker/compose/DockerComposeExtensionIntegrationTestWithFailingCommand.java deleted file mode 100644 index 378c65f97..000000000 --- a/docker-compose-junit-jupiter/src/integrationTest/java/com/palantir/docker/compose/DockerComposeExtensionIntegrationTestWithFailingCommand.java +++ /dev/null @@ -1,45 +0,0 @@ -/* - * (c) Copyright 2019 Palantir Technologies Inc. All rights reserved. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.palantir.docker.compose; - -import static org.assertj.core.api.Assertions.assertThat; - -import com.palantir.docker.compose.configuration.DockerComposeFiles; -import com.palantir.docker.compose.connection.State; -import com.palantir.docker.compose.connection.waiting.HealthChecks; -import java.io.IOException; -import org.joda.time.Duration; -import org.junit.jupiter.api.Order; -import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.extension.RegisterExtension; - -public class DockerComposeExtensionIntegrationTestWithFailingCommand { - - @RegisterExtension - public static final DockerComposeExtension docker = DockerComposeExtension.builder() - .files(DockerComposeFiles.from("src/integrationTest/resources/docker-compose-with-failing-command" - + ".yaml")) - .waitingForService("cmd", HealthChecks.toRespondOverHttp(9999, _$ -> "http://will-fail"), - Duration.millis(50)) - .build(); - - @Test - @Order(1) - public void calls_after_on_exception() throws IOException, InterruptedException { - assertThat(docker.containers().container("cmd").state()).isEqualTo(State.DOWN); - } -} diff --git a/docker-compose-junit-jupiter/src/integrationTest/resources/docker-compose-with-failing-command.yaml b/docker-compose-junit-jupiter/src/integrationTest/resources/docker-compose-with-failing-command.yaml deleted file mode 100644 index 1c52447e9..000000000 --- a/docker-compose-junit-jupiter/src/integrationTest/resources/docker-compose-with-failing-command.yaml +++ /dev/null @@ -1,5 +0,0 @@ -cmd: - image: appropriate/nc - command: /bin/sh -c 'sleep 1000' - ports: - - "9999:9999" From c2ce7461af16583ec86f373664b356a49acf203b Mon Sep 17 00:00:00 2001 From: Paul Nepywoda Date: Tue, 29 Oct 2019 15:47:46 -0700 Subject: [PATCH 4/8] checkstyle --- .../com/palantir/docker/compose/DockerComposeExtensionTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/docker-compose-junit-jupiter/src/test/java/com/palantir/docker/compose/DockerComposeExtensionTest.java b/docker-compose-junit-jupiter/src/test/java/com/palantir/docker/compose/DockerComposeExtensionTest.java index 22856a01b..d859df9cd 100644 --- a/docker-compose-junit-jupiter/src/test/java/com/palantir/docker/compose/DockerComposeExtensionTest.java +++ b/docker-compose-junit-jupiter/src/test/java/com/palantir/docker/compose/DockerComposeExtensionTest.java @@ -35,7 +35,6 @@ public class DockerComposeExtensionTest { public void calls_after_only_once() throws IOException, InterruptedException { AtomicInteger count = new AtomicInteger(); DockerComposeExtension dockerComposeExtension = new DockerComposeExtension() { - @Override public void before() { throw new IllegalStateException("some error"); From e0701782f14dbd33783829be727c5cfff825d06e Mon Sep 17 00:00:00 2001 From: Paul Nepywoda Date: Tue, 29 Oct 2019 22:47:46 +0000 Subject: [PATCH 5/8] Add generated changelog entries --- changelog/@unreleased/pr-384.v2.yml | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 changelog/@unreleased/pr-384.v2.yml diff --git a/changelog/@unreleased/pr-384.v2.yml b/changelog/@unreleased/pr-384.v2.yml new file mode 100644 index 000000000..6c0393728 --- /dev/null +++ b/changelog/@unreleased/pr-384.v2.yml @@ -0,0 +1,8 @@ +type: improvement +improvement: + description: |- + always copy logs to circle in finally + + doesnt work - jupiter always calls after anyway + links: + - https://github.com/palantir/docker-compose-rule/pull/384 From 9b0dac4896191257380a03fa2de8e2b516c1a1c4 Mon Sep 17 00:00:00 2001 From: pnepywoda Date: Wed, 30 Oct 2019 17:37:12 -0700 Subject: [PATCH 6/8] Update pr-384.v2.yml --- changelog/@unreleased/pr-384.v2.yml | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/changelog/@unreleased/pr-384.v2.yml b/changelog/@unreleased/pr-384.v2.yml index 6c0393728..9c520edc8 100644 --- a/changelog/@unreleased/pr-384.v2.yml +++ b/changelog/@unreleased/pr-384.v2.yml @@ -1,8 +1,6 @@ type: improvement improvement: description: |- - always copy logs to circle in finally - - doesnt work - jupiter always calls after anyway + copy logs to circle even if there's an error initializing in `before` links: - https://github.com/palantir/docker-compose-rule/pull/384 From e619465bdb662a121adb799c2c5c39424aec424b Mon Sep 17 00:00:00 2001 From: Paul Nepywoda Date: Wed, 30 Oct 2019 17:38:07 -0700 Subject: [PATCH 7/8] no integration test --- .../compose/DockerComposeExtensionIntegrationTest.java | 6 ------ 1 file changed, 6 deletions(-) diff --git a/docker-compose-junit-jupiter/src/integrationTest/java/com/palantir/docker/compose/DockerComposeExtensionIntegrationTest.java b/docker-compose-junit-jupiter/src/integrationTest/java/com/palantir/docker/compose/DockerComposeExtensionIntegrationTest.java index f620224bb..b107f67bf 100644 --- a/docker-compose-junit-jupiter/src/integrationTest/java/com/palantir/docker/compose/DockerComposeExtensionIntegrationTest.java +++ b/docker-compose-junit-jupiter/src/integrationTest/java/com/palantir/docker/compose/DockerComposeExtensionIntegrationTest.java @@ -49,10 +49,4 @@ public void an_external_port_exists() { public void container_stays_up_between_tests() { assertThat(docker.containers().container("db").port(5432).getExternalPort()).isEqualTo(port); } - - @Test - @Order(3) - public void calls_after_on_exception() { - assertThat(docker.containers().container("db").port(5432).getExternalPort()).isEqualTo(port); - } } From 3d98783aca632bf6aa4ad17c09294544ba2f92f8 Mon Sep 17 00:00:00 2001 From: Paul Nepywoda Date: Thu, 14 Nov 2019 15:33:07 -0800 Subject: [PATCH 8/8] only-call-after-once check in DockerComposeManager --- .../compose/DockerComposeExtension.java | 14 ++------------ .../compose/DockerComposeExtensionTest.java | 3 +-- .../docker/compose/DockerComposeManager.java | 19 +++++++++++++++---- 3 files changed, 18 insertions(+), 18 deletions(-) diff --git a/docker-compose-junit-jupiter/src/main/java/com/palantir/docker/compose/DockerComposeExtension.java b/docker-compose-junit-jupiter/src/main/java/com/palantir/docker/compose/DockerComposeExtension.java index 01ebddcd7..2b8834a2c 100644 --- a/docker-compose-junit-jupiter/src/main/java/com/palantir/docker/compose/DockerComposeExtension.java +++ b/docker-compose-junit-jupiter/src/main/java/com/palantir/docker/compose/DockerComposeExtension.java @@ -30,24 +30,14 @@ public abstract class DockerComposeExtension extends DockerComposeManager implements BeforeAllCallback, AfterAllCallback { - private boolean hasCalledAfterMethod; - @Override public void beforeAll(ExtensionContext unused) throws IOException, InterruptedException { - try { - before(); - } catch (RuntimeException e) { - after(); - hasCalledAfterMethod = true; - throw e; - } + before(); } @Override public void afterAll(ExtensionContext unused) { - if (!hasCalledAfterMethod) { - after(); - } + after(); } public static Builder builder() { diff --git a/docker-compose-junit-jupiter/src/test/java/com/palantir/docker/compose/DockerComposeExtensionTest.java b/docker-compose-junit-jupiter/src/test/java/com/palantir/docker/compose/DockerComposeExtensionTest.java index d859df9cd..2e10486b7 100644 --- a/docker-compose-junit-jupiter/src/test/java/com/palantir/docker/compose/DockerComposeExtensionTest.java +++ b/docker-compose-junit-jupiter/src/test/java/com/palantir/docker/compose/DockerComposeExtensionTest.java @@ -22,7 +22,6 @@ import com.palantir.docker.compose.configuration.DockerComposeFiles; import com.palantir.docker.compose.connection.waiting.ClusterWait; import com.palantir.docker.compose.events.EventConsumer; -import java.io.IOException; import java.util.List; import java.util.concurrent.atomic.AtomicInteger; import org.junit.jupiter.api.Test; @@ -32,7 +31,7 @@ public class DockerComposeExtensionTest { @Test - public void calls_after_only_once() throws IOException, InterruptedException { + public void calls_after_only_once() { AtomicInteger count = new AtomicInteger(); DockerComposeExtension dockerComposeExtension = new DockerComposeExtension() { @Override diff --git a/docker-compose-rule-core/src/main/java/com/palantir/docker/compose/DockerComposeManager.java b/docker-compose-rule-core/src/main/java/com/palantir/docker/compose/DockerComposeManager.java index d6d746078..589d70f54 100644 --- a/docker-compose-rule-core/src/main/java/com/palantir/docker/compose/DockerComposeManager.java +++ b/docker-compose-rule-core/src/main/java/com/palantir/docker/compose/DockerComposeManager.java @@ -76,6 +76,7 @@ public abstract class DockerComposeManager { public static final int DEFAULT_RETRY_ATTEMPTS = 2; private final RunRecorder runRecorder = RunRecorder.defaults(); + private boolean hasCalledAfterMethod; public DockerPort hostNetworkedPort(int port) { return new DockerPort(machine().getIp(), port, port); @@ -177,13 +178,19 @@ protected void setDescription(TestDescription testDescription) { } public void before() throws IOException, InterruptedException { - log.debug("Starting docker-compose cluster"); + try { + log.debug("Starting docker-compose cluster"); - runRecorder.before(() -> dockerCompose().config()); + runRecorder.before(() -> dockerCompose().config()); - pullBuildAndUp(); + pullBuildAndUp(); - emitEventsFor().waitingForServices(this::waitForServices); + emitEventsFor().waitingForServices(this::waitForServices); + } catch (RuntimeException e) { + after(); + hasCalledAfterMethod = true; + throw e; + } } private void pullBuildAndUp() throws IOException, InterruptedException { @@ -254,6 +261,10 @@ private void waitForAllClusterWaits(List allClusterWai } public void after() { + if (hasCalledAfterMethod) { + return; + } + try { emitEventsFor().shutdownStop(() -> shutdownStrategy().stop(this.dockerCompose()));