From d0ea876e0b9fb7760f2a5e914a9ee42af570a994 Mon Sep 17 00:00:00 2001 From: "Mateusz \"Serafin\" Gajewski" Date: Tue, 13 Jun 2023 15:24:37 +0200 Subject: [PATCH 1/5] Fix and use correct exit code constant --- .../trino/tests/product/launcher/cli/EnvironmentDescribe.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/testing/trino-product-tests-launcher/src/main/java/io/trino/tests/product/launcher/cli/EnvironmentDescribe.java b/testing/trino-product-tests-launcher/src/main/java/io/trino/tests/product/launcher/cli/EnvironmentDescribe.java index 73370e3c56f7..58c83042eec3 100644 --- a/testing/trino-product-tests-launcher/src/main/java/io/trino/tests/product/launcher/cli/EnvironmentDescribe.java +++ b/testing/trino-product-tests-launcher/src/main/java/io/trino/tests/product/launcher/cli/EnvironmentDescribe.java @@ -33,6 +33,7 @@ import io.trino.tests.product.launcher.util.ConsoleTable; import org.testcontainers.utility.MountableFile; import picocli.CommandLine; +import picocli.CommandLine.ExitCode; import picocli.CommandLine.Mixin; import picocli.CommandLine.Option; @@ -190,7 +191,7 @@ public Integer call() log.info("Environment '%s' file mounts:\n%s", environmentUpOptions.environment, mountsTable.render()); - return 1; + return ExitCode.OK; } private String simplifyPath(String path) From 2543c04cc270d6abaf58cf30d367cb1bcf2a6a51 Mon Sep 17 00:00:00 2001 From: "Mateusz \"Serafin\" Gajewski" Date: Tue, 13 Jun 2023 15:25:25 +0200 Subject: [PATCH 2/5] Make launcher print to stdout --- .../io/trino/tests/product/launcher/cli/EnvironmentList.java | 4 +--- .../io/trino/tests/product/launcher/cli/SuiteDescribe.java | 4 +--- .../java/io/trino/tests/product/launcher/cli/SuiteList.java | 4 +--- .../java/io/trino/tests/product/launcher/env/Environment.java | 4 +--- 4 files changed, 4 insertions(+), 12 deletions(-) diff --git a/testing/trino-product-tests-launcher/src/main/java/io/trino/tests/product/launcher/cli/EnvironmentList.java b/testing/trino-product-tests-launcher/src/main/java/io/trino/tests/product/launcher/cli/EnvironmentList.java index d2feae4eb139..6788582591fe 100644 --- a/testing/trino-product-tests-launcher/src/main/java/io/trino/tests/product/launcher/cli/EnvironmentList.java +++ b/testing/trino-product-tests-launcher/src/main/java/io/trino/tests/product/launcher/cli/EnvironmentList.java @@ -25,8 +25,6 @@ import picocli.CommandLine.ExitCode; import picocli.CommandLine.Option; -import java.io.FileDescriptor; -import java.io.FileOutputStream; import java.io.PrintStream; import java.io.UnsupportedEncodingException; import java.nio.charset.Charset; @@ -78,7 +76,7 @@ public Execution(EnvironmentFactory factory, EnvironmentConfigFactory configFact this.configFactory = requireNonNull(configFactory, "configFactory is null"); try { - this.out = new PrintStream(new FileOutputStream(FileDescriptor.out), true, Charset.defaultCharset().name()); + this.out = new PrintStream(System.out, true, Charset.defaultCharset().name()); } catch (UnsupportedEncodingException e) { throw new IllegalStateException("Could not create print stream", e); diff --git a/testing/trino-product-tests-launcher/src/main/java/io/trino/tests/product/launcher/cli/SuiteDescribe.java b/testing/trino-product-tests-launcher/src/main/java/io/trino/tests/product/launcher/cli/SuiteDescribe.java index 98339fa9d67b..87d7e15070dd 100644 --- a/testing/trino-product-tests-launcher/src/main/java/io/trino/tests/product/launcher/cli/SuiteDescribe.java +++ b/testing/trino-product-tests-launcher/src/main/java/io/trino/tests/product/launcher/cli/SuiteDescribe.java @@ -37,8 +37,6 @@ import picocli.CommandLine.Mixin; import java.io.File; -import java.io.FileDescriptor; -import java.io.FileOutputStream; import java.io.PrintStream; import java.io.UnsupportedEncodingException; import java.nio.charset.Charset; @@ -232,7 +230,7 @@ public Execution( this.outputBuilder = describeOptions.format.outputBuilderFactory.get(); try { - this.out = new PrintStream(new FileOutputStream(FileDescriptor.out), true, Charset.defaultCharset().name()); + this.out = new PrintStream(System.out, true, Charset.defaultCharset().name()); } catch (UnsupportedEncodingException e) { throw new IllegalStateException("Could not create print stream", e); diff --git a/testing/trino-product-tests-launcher/src/main/java/io/trino/tests/product/launcher/cli/SuiteList.java b/testing/trino-product-tests-launcher/src/main/java/io/trino/tests/product/launcher/cli/SuiteList.java index 6de434b9f563..f226f672a336 100644 --- a/testing/trino-product-tests-launcher/src/main/java/io/trino/tests/product/launcher/cli/SuiteList.java +++ b/testing/trino-product-tests-launcher/src/main/java/io/trino/tests/product/launcher/cli/SuiteList.java @@ -28,8 +28,6 @@ import picocli.CommandLine.ExitCode; import picocli.CommandLine.Option; -import java.io.FileDescriptor; -import java.io.FileOutputStream; import java.io.PrintStream; import java.io.UnsupportedEncodingException; import java.nio.charset.Charset; @@ -83,7 +81,7 @@ public Execution(SuiteFactory suiteFactory, EnvironmentConfigFactory configFacto this.suiteFactory = requireNonNull(suiteFactory, "suiteFactory is null"); try { - this.out = new PrintStream(new FileOutputStream(FileDescriptor.out), true, Charset.defaultCharset().name()); + this.out = new PrintStream(System.out, true, Charset.defaultCharset().name()); } catch (UnsupportedEncodingException e) { throw new IllegalStateException("Could not create print stream", e); diff --git a/testing/trino-product-tests-launcher/src/main/java/io/trino/tests/product/launcher/env/Environment.java b/testing/trino-product-tests-launcher/src/main/java/io/trino/tests/product/launcher/env/Environment.java index 30e7c139d724..9bf2834a489a 100644 --- a/testing/trino-product-tests-launcher/src/main/java/io/trino/tests/product/launcher/env/Environment.java +++ b/testing/trino-product-tests-launcher/src/main/java/io/trino/tests/product/launcher/env/Environment.java @@ -40,9 +40,7 @@ import org.testcontainers.utility.MountableFile; import java.io.File; -import java.io.FileDescriptor; import java.io.FileNotFoundException; -import java.io.FileOutputStream; import java.io.IOException; import java.io.PrintStream; import java.io.UncheckedIOException; @@ -635,7 +633,7 @@ private static Consumer printContainerLogs(DockerContainer containe { try { // write directly to System.out, bypassing logging & io.airlift.log.Logging#rewireStdStreams - PrintStream out = new PrintStream(new FileOutputStream(FileDescriptor.out), true, Charset.defaultCharset().name()); + PrintStream out = new PrintStream(System.out, true, Charset.defaultCharset().name()); return new PrintingLogConsumer(out, format("%-20s| ", container.getLogicalName())); } catch (UnsupportedEncodingException e) { From 7468873b034221da9753f35c7afda9267bd51f50 Mon Sep 17 00:00:00 2001 From: "Mateusz \"Serafin\" Gajewski" Date: Tue, 13 Jun 2023 15:26:39 +0200 Subject: [PATCH 3/5] Mark EnvironmentOptions.jdkDownloadPath as @Nullable This parameter is null by default and is only initialized by picocli when the command is invoked. --- .../tests/product/launcher/env/EnvironmentOptions.java | 2 ++ .../launcher/env/jdk/TarDownloadingJdkProvider.java | 8 +++++++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/testing/trino-product-tests-launcher/src/main/java/io/trino/tests/product/launcher/env/EnvironmentOptions.java b/testing/trino-product-tests-launcher/src/main/java/io/trino/tests/product/launcher/env/EnvironmentOptions.java index 5b3cbae9d3c0..1ade2cfd8cd2 100644 --- a/testing/trino-product-tests-launcher/src/main/java/io/trino/tests/product/launcher/env/EnvironmentOptions.java +++ b/testing/trino-product-tests-launcher/src/main/java/io/trino/tests/product/launcher/env/EnvironmentOptions.java @@ -13,6 +13,7 @@ */ package io.trino.tests.product.launcher.env; +import jakarta.annotation.Nullable; import picocli.CommandLine; import picocli.CommandLine.Model.CommandSpec; import picocli.CommandLine.Spec; @@ -60,6 +61,7 @@ public final class EnvironmentOptions public String jdkProvider = BUILT_IN_NAME; @Option(names = "--jdk-tmp-download-path", paramLabel = "", defaultValue = "${env:PTL_TMP_DOWNLOAD_PATH:-${sys:java.io.tmpdir}/ptl-tmp-download}", description = "Path to use to download JDK distributions " + DEFAULT_VALUE) + @Nullable public Path jdkDownloadPath; @Option(names = "--bind", description = "Bind exposed container ports to host ports, possible values: " + BIND_ON_HOST + ", " + DO_NOT_BIND + ", [port base number] " + DEFAULT_VALUE, defaultValue = BIND_ON_HOST, arity = "0..1", fallbackValue = BIND_ON_HOST) diff --git a/testing/trino-product-tests-launcher/src/main/java/io/trino/tests/product/launcher/env/jdk/TarDownloadingJdkProvider.java b/testing/trino-product-tests-launcher/src/main/java/io/trino/tests/product/launcher/env/jdk/TarDownloadingJdkProvider.java index a06c40c4d264..0fd4eb100486 100644 --- a/testing/trino-product-tests-launcher/src/main/java/io/trino/tests/product/launcher/env/jdk/TarDownloadingJdkProvider.java +++ b/testing/trino-product-tests-launcher/src/main/java/io/trino/tests/product/launcher/env/jdk/TarDownloadingJdkProvider.java @@ -36,6 +36,7 @@ import java.util.function.Consumer; import java.util.function.Function; +import static com.google.common.base.MoreObjects.firstNonNull; import static com.google.common.base.Strings.isNullOrEmpty; import static com.google.common.base.Verify.verify; import static io.trino.testing.containers.TestContainers.getDockerArchitectureInfo; @@ -53,7 +54,12 @@ public abstract class TarDownloadingJdkProvider public TarDownloadingJdkProvider(EnvironmentOptions environmentOptions) { - this.downloadPath = requireNonNull(environmentOptions.jdkDownloadPath, "environmentOptions.jdkDownloadPath is null"); + try { + this.downloadPath = firstNonNull(environmentOptions.jdkDownloadPath, Files.createTempDirectory("ptl-temp-path")); + } + catch (IOException e) { + throw new UncheckedIOException(e); + } } protected abstract String getDownloadUri(DockerArchitecture architecture); From 2eb1a80e1fe36e1ad84e934e3bc75bc9b60d033b Mon Sep 17 00:00:00 2001 From: "Mateusz \"Serafin\" Gajewski" Date: Tue, 13 Jun 2023 15:28:24 +0200 Subject: [PATCH 4/5] Refactor Launcher invocation to capture exit code --- .../io/trino/tests/product/launcher/cli/Launcher.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/testing/trino-product-tests-launcher/src/main/java/io/trino/tests/product/launcher/cli/Launcher.java b/testing/trino-product-tests-launcher/src/main/java/io/trino/tests/product/launcher/cli/Launcher.java index 60daca5961c6..0a649b88045b 100644 --- a/testing/trino-product-tests-launcher/src/main/java/io/trino/tests/product/launcher/cli/Launcher.java +++ b/testing/trino-product-tests-launcher/src/main/java/io/trino/tests/product/launcher/cli/Launcher.java @@ -59,18 +59,18 @@ public class Launcher public static void main(String[] args) { Launcher launcher = new Launcher(); - run(launcher, new LauncherBundle(), args); + System.exit(execute(launcher, new LauncherBundle(), args)); } - public static void run(Launcher launcher, ResourceBundle bundle, String[] args) + public static int execute(Launcher launcher, ResourceBundle bundle, String[] args) { IFactory factory = createFactory(launcher.getExtensions()); - System.exit(new CommandLine(launcher, factory) + return new CommandLine(launcher, factory) .setCaseInsensitiveEnumValuesAllowed(true) .registerConverter(Duration.class, Duration::valueOf) .registerConverter(Path.class, Paths::get) .setResourceBundle(bundle) - .execute(args)); + .execute(args); } private static IFactory createFactory(Extensions extensions) @@ -157,7 +157,7 @@ public String[] getVersion() } } - private static class LauncherBundle + static class LauncherBundle extends ListResourceBundle { @Override From d01c51371e9cd51813292e7a7af2e3cfe372c1a8 Mon Sep 17 00:00:00 2001 From: "Mateusz \"Serafin\" Gajewski" Date: Tue, 13 Jun 2023 15:28:43 +0200 Subject: [PATCH 5/5] Add basic product tests launcher tests This prevents regressions where some of the invocations are broken because of the lack of the dependencies or null derefences. --- .../product/launcher/cli/TestInvocations.java | 137 ++++++++++++++++++ 1 file changed, 137 insertions(+) create mode 100644 testing/trino-product-tests-launcher/src/test/java/io/trino/tests/product/launcher/cli/TestInvocations.java diff --git a/testing/trino-product-tests-launcher/src/test/java/io/trino/tests/product/launcher/cli/TestInvocations.java b/testing/trino-product-tests-launcher/src/test/java/io/trino/tests/product/launcher/cli/TestInvocations.java new file mode 100644 index 000000000000..d3e5b9095e2b --- /dev/null +++ b/testing/trino-product-tests-launcher/src/test/java/io/trino/tests/product/launcher/cli/TestInvocations.java @@ -0,0 +1,137 @@ +/* + * 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 io.trino.tests.product.launcher.cli; + +import org.testng.annotations.Test; + +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.PrintStream; +import java.nio.file.Files; + +import static io.trino.tests.product.launcher.cli.Launcher.execute; +import static java.nio.charset.StandardCharsets.UTF_8; +import static java.util.Objects.requireNonNull; +import static org.assertj.core.api.Assertions.assertThat; + +@Test(singleThreaded = true) +public class TestInvocations +{ + @Test + public void testEnvironmentList() + { + InvocationResult invocationResult = invokeLauncher("env", "list"); + + assertThat(invocationResult.exitCode()).isEqualTo(0); + assertThat(invocationResult.output()) + .contains("Available environments:") + .contains("multinode"); + } + + @Test + public void testSuiteList() + { + InvocationResult invocationResult = invokeLauncher("suite", "list"); + + assertThat(invocationResult.exitCode()).isEqualTo(0); + assertThat(invocationResult.output()) + .contains("Available suites:") + .contains("suite-1"); + } + + @Test + public void testDescribeEnvironment() + throws IOException + { + InvocationResult invocationResult = invokeLauncher( + "env", "describe", + "--server-package", Files.createTempFile("server", ".tar.gz").toString(), + // This is known to work for both arm and x86 + "--environment", "multinode-postgresql"); + + assertThat(invocationResult.exitCode()).isEqualTo(0); + assertThat(invocationResult.output()) + .contains("Environment 'multinode-postgresql' file mounts:"); + } + + @Test + public void testDescribeSuite() + throws IOException + { + InvocationResult invocationResult = invokeLauncher( + "suite", "describe", + "--server-package", Files.createTempFile("server", ".tar.gz").toString(), + "--suite", "suite-1"); + + assertThat(invocationResult.exitCode()).isEqualTo(0); + assertThat(invocationResult.output()) + .contains("Suite 'suite-1' with configuration 'config-default' consists of following test runs"); + } + + @Test + public void testEnvUpDown() + throws IOException + { + InvocationResult upResult = invokeLauncher( + "env", "up", + "--server-package", Files.createTempFile("server", ".tar.gz").toString(), + "--without-trino", + "--background", + "--bind", + "off", + // This is known to work for both arm and x86 + "--environment", "multinode-postgresql"); + + assertThat(upResult.exitCode()).isEqualTo(0); + InvocationResult downResult = invokeLauncher("env", "down"); + assertThat(downResult.exitCode()).isEqualTo(0); + } + + public static InvocationResult invokeLauncher(String... args) + { + Launcher launcher = new Launcher(); + ByteArrayOutputStream out = new ByteArrayOutputStream(); + + try (CaptureOutput ignored = new CaptureOutput(out)) { + int exitCode = execute(launcher, new Launcher.LauncherBundle(), args); + return new InvocationResult(exitCode, out.toString(UTF_8)); + } + } + + @SuppressWarnings("UnusedVariable") + private record InvocationResult(int exitCode, String output) {} + + private static class CaptureOutput + implements AutoCloseable + { + private final PrintStream originalOut; + private final PrintStream originalErr; + + public CaptureOutput(ByteArrayOutputStream out) + { + PrintStream stream = new PrintStream(requireNonNull(out, "out is null")); + this.originalOut = System.out; + this.originalErr = System.err; + System.setOut(stream); + System.setErr(stream); + } + + @Override + public void close() + { + System.setOut(originalOut); + System.setErr(originalErr); + } + } +}