From 63443602441d3958365a300f916b8beb2abcb52f Mon Sep 17 00:00:00 2001 From: Mateusz Gajewski Date: Mon, 28 Sep 2020 17:33:37 +0200 Subject: [PATCH] Minor code cleanup --- .../product/launcher/cli/EnvironmentUp.java | 2 +- .../product/launcher/cli/SuiteDescribe.java | 4 ++-- .../tests/product/launcher/cli/SuiteRun.java | 4 +--- .../product/launcher/docker/DockerFiles.java | 2 +- .../product/launcher/env/Environment.java | 22 +++++++++---------- .../product/launcher/env/Environments.java | 2 +- .../tests/product/launcher/suite/Suites.java | 2 +- 7 files changed, 17 insertions(+), 21 deletions(-) diff --git a/presto-product-tests-launcher/src/main/java/io/prestosql/tests/product/launcher/cli/EnvironmentUp.java b/presto-product-tests-launcher/src/main/java/io/prestosql/tests/product/launcher/cli/EnvironmentUp.java index c1a3a39895467..26554c8ec90e3 100644 --- a/presto-product-tests-launcher/src/main/java/io/prestosql/tests/product/launcher/cli/EnvironmentUp.java +++ b/presto-product-tests-launcher/src/main/java/io/prestosql/tests/product/launcher/cli/EnvironmentUp.java @@ -158,7 +158,7 @@ public Integer call() return ExitCode.OK; } - private void killContainersReaperContainer() + private static void killContainersReaperContainer() { try (DockerClient dockerClient = DockerClientFactory.lazyClient()) { log.info("Killing the testcontainers reaper container (Ryuk) so that environment can stay alive"); diff --git a/presto-product-tests-launcher/src/main/java/io/prestosql/tests/product/launcher/cli/SuiteDescribe.java b/presto-product-tests-launcher/src/main/java/io/prestosql/tests/product/launcher/cli/SuiteDescribe.java index 82fbe8ce6e257..2705f3212080e 100644 --- a/presto-product-tests-launcher/src/main/java/io/prestosql/tests/product/launcher/cli/SuiteDescribe.java +++ b/presto-product-tests-launcher/src/main/java/io/prestosql/tests/product/launcher/cli/SuiteDescribe.java @@ -139,11 +139,11 @@ public Integer call() Suite suite = suiteFactory.getSuite(suiteName); EnvironmentConfig config = configFactory.getConfig(this.config); - out.println(format("Suite '%s' with configuration '%s' consists of following test runs: ", suiteName, this.config)); + out.printf("Suite '%s' with configuration '%s' consists of following test runs: \n", suiteName, this.config); for (SuiteTestRun testRun : suite.getTestRuns(config)) { TestRun.TestRunOptions runOptions = createTestRunOptions(suiteName, testRun, config); - out.println(format("\n%s test run %s\n", environmentOptions.launcherBin, OptionsPrinter.format(environmentOptions, runOptions))); + out.printf("\n%s test run %s\n\n", environmentOptions.launcherBin, OptionsPrinter.format(environmentOptions, runOptions)); } return OK; diff --git a/presto-product-tests-launcher/src/main/java/io/prestosql/tests/product/launcher/cli/SuiteRun.java b/presto-product-tests-launcher/src/main/java/io/prestosql/tests/product/launcher/cli/SuiteRun.java index 6de64bdb7fded..f15e268c500b8 100644 --- a/presto-product-tests-launcher/src/main/java/io/prestosql/tests/product/launcher/cli/SuiteRun.java +++ b/presto-product-tests-launcher/src/main/java/io/prestosql/tests/product/launcher/cli/SuiteRun.java @@ -173,7 +173,7 @@ public Integer call() return getFailedCount(testRunsResults) == 0 ? ExitCode.OK : ExitCode.SOFTWARE; } - private int printTestRunsSummary(String suiteName, List results) + private void printTestRunsSummary(String suiteName, List results) { long failedRuns = getFailedCount(results); @@ -191,8 +191,6 @@ private int printTestRunsSummary(String suiteName, List results) results.stream() .filter(TestRunResult::hasFailed) .forEach(Execution::printTestRunSummary); - - return failedRuns == 0L ? 0 : 1; } private static long getFailedCount(List results) diff --git a/presto-product-tests-launcher/src/main/java/io/prestosql/tests/product/launcher/docker/DockerFiles.java b/presto-product-tests-launcher/src/main/java/io/prestosql/tests/product/launcher/docker/DockerFiles.java index 67fdc14b60f27..8b51fb3994e61 100644 --- a/presto-product-tests-launcher/src/main/java/io/prestosql/tests/product/launcher/docker/DockerFiles.java +++ b/presto-product-tests-launcher/src/main/java/io/prestosql/tests/product/launcher/docker/DockerFiles.java @@ -72,7 +72,7 @@ public synchronized String getDockerFilesHostPath() public String getDockerFilesHostPath(String file) { - checkArgument(file != null && !file.isEmpty() && !file.startsWith("/"), "Invalid file: %s", file); + checkArgument(file != null && !file.isEmpty() && !(file.charAt(0) == '/'), "Invalid file: %s", file); Path filePath = Paths.get(getDockerFilesHostPath()).resolve(file); checkArgument(Files.exists(filePath), "'%s' resolves to '%s', but it does not exist", file, filePath); return filePath.toString(); diff --git a/presto-product-tests-launcher/src/main/java/io/prestosql/tests/product/launcher/env/Environment.java b/presto-product-tests-launcher/src/main/java/io/prestosql/tests/product/launcher/env/Environment.java index a5b59ae3d4d82..bc8bcbcd97fcc 100644 --- a/presto-product-tests-launcher/src/main/java/io/prestosql/tests/product/launcher/env/Environment.java +++ b/presto-product-tests-launcher/src/main/java/io/prestosql/tests/product/launcher/env/Environment.java @@ -19,7 +19,6 @@ import com.github.dockerjava.api.model.HostConfig; import com.github.dockerjava.api.model.Ulimit; import com.google.common.base.Joiner; -import com.google.common.base.Predicate; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Streams; @@ -50,6 +49,7 @@ import java.util.concurrent.ExecutionException; import java.util.function.Consumer; import java.util.function.Function; +import java.util.function.Predicate; import static com.google.common.base.MoreObjects.firstNonNull; import static com.google.common.base.Preconditions.checkState; @@ -99,7 +99,7 @@ public Environment start() return Failsafe .with(retryPolicy) - .get(() -> tryStart()); + .get(this::tryStart); } public List getContainerNames() @@ -169,8 +169,6 @@ public void awaitContainersStopped() } log.warn("Some of the containers are stopped or unhealthy"); - - return; } catch (InterruptedException e) { log.info("Interrupted"); @@ -374,8 +372,8 @@ public Builder containerDependsOnRest(String logicalName) containers.entrySet() .stream() .filter(entry -> !entry.getKey().equals(logicalName)) - .map(entry -> entry.getValue()) - .forEach(dependant -> container.dependsOn(dependant)); + .map(Map.Entry::getValue) + .forEach(container::dependsOn); return this; } @@ -449,11 +447,11 @@ private Environment build(Optional listener) switch (outputMode) { case DISCARD: log.warn("Containers logs are not printed to stdout"); - setContainerOutputConsumer(this::discardContainerLogs); + setContainerOutputConsumer(Environment.Builder::discardContainerLogs); break; case PRINT: - setContainerOutputConsumer(this::printContainerLogs); + setContainerOutputConsumer(Environment.Builder::printContainerLogs); break; case PRINT_WRITE: @@ -485,7 +483,7 @@ private Environment build(Optional listener) return new Environment(name, startupRetries, containers, listener); } - private Consumer writeContainerLogs(DockerContainer container, Path path) + private static Consumer writeContainerLogs(DockerContainer container, Path path) { Path containerLogFile = path.resolve(container.getLogicalName() + "/container.log"); log.info("Writing container %s logs to %s", container, containerLogFile); @@ -499,7 +497,7 @@ private Consumer writeContainerLogs(DockerContainer container, Path } } - private Consumer printContainerLogs(DockerContainer container) + private static Consumer printContainerLogs(DockerContainer container) { try { // write directly to System.out, bypassing logging & io.airlift.log.Logging#rewireStdStreams @@ -512,13 +510,13 @@ private Consumer printContainerLogs(DockerContainer container) } } - private Consumer discardContainerLogs(DockerContainer container) + private static Consumer discardContainerLogs(DockerContainer container) { // Discard log frames return outputFrame -> {}; } - private Consumer combineConsumers(Consumer... consumers) + private static Consumer combineConsumers(Consumer... consumers) { return outputFrame -> Arrays.stream(consumers).forEach(consumer -> consumer.accept(outputFrame)); } diff --git a/presto-product-tests-launcher/src/main/java/io/prestosql/tests/product/launcher/env/Environments.java b/presto-product-tests-launcher/src/main/java/io/prestosql/tests/product/launcher/env/Environments.java index 8ba1199749c08..33fd4080e8a2e 100644 --- a/presto-product-tests-launcher/src/main/java/io/prestosql/tests/product/launcher/env/Environments.java +++ b/presto-product-tests-launcher/src/main/java/io/prestosql/tests/product/launcher/env/Environments.java @@ -98,7 +98,7 @@ public static List> findConfigsByBasePackage( return ClassPath.from(Environments.class.getClassLoader()).getTopLevelClassesRecursive(packageName).stream() .map(ClassPath.ClassInfo::load) .filter(clazz -> !isAbstract(clazz.getModifiers())) - .filter(clazz -> EnvironmentConfig.class.isAssignableFrom(clazz)) + .filter(EnvironmentConfig.class::isAssignableFrom) .map(clazz -> (Class) clazz.asSubclass(EnvironmentConfig.class)) .collect(toImmutableList()); } diff --git a/presto-product-tests-launcher/src/main/java/io/prestosql/tests/product/launcher/suite/Suites.java b/presto-product-tests-launcher/src/main/java/io/prestosql/tests/product/launcher/suite/Suites.java index d4aa7c71c6324..0e8ebabf4c122 100644 --- a/presto-product-tests-launcher/src/main/java/io/prestosql/tests/product/launcher/suite/Suites.java +++ b/presto-product-tests-launcher/src/main/java/io/prestosql/tests/product/launcher/suite/Suites.java @@ -33,7 +33,7 @@ public static List> findSuitesByPackageName(String packag return ClassPath.from(Environments.class.getClassLoader()).getTopLevelClassesRecursive(packageName).stream() .map(ClassPath.ClassInfo::load) .filter(clazz -> !isAbstract(clazz.getModifiers())) - .filter(clazz -> Suite.class.isAssignableFrom(clazz)) + .filter(Suite.class::isAssignableFrom) .map(clazz -> (Class) clazz.asSubclass(Suite.class)) .collect(toImmutableList()); }