Autoformat :qa:os and :benchmarks (#52816)

Add `:qa:os` and `:benchmarks` to the list of automatically formatted
projects, and apply some manual fix-ups to polish it up.

In particular, I noticed that `Files.write(...)` when passed a list will
automaticaly apply a UTF-8 encoding and write a newline after each line,
making it easier to use than FileUtils.append. It's even available from
1.8.

Also, in the Allocators class, a number of methods declared thrown exceptions that IntelliJ reported were never thrown, and as far as I could see this is true, so I removed the exceptions.

benchmarks/src/main/java/org/elasticsearch/benchmark/fs/AvailableIndexFoldersBenchmark.java

@@ -61,7 +61,8 @@ public class AvailableIndexFoldersBenchmark {
         LogConfigurator.setNodeName("test");
         Settings settings = Settings.builder()
             .put(Environment.PATH_HOME_SETTING.getKey(), path)
-            .putList(Environment.PATH_DATA_SETTING.getKey(), paths).build();
+            .putList(Environment.PATH_DATA_SETTING.getKey(), paths)
+            .build();
         nodeEnv = new NodeEnvironment(settings, new Environment(settings, null));
 
         Files.createDirectories(nodePath.indicesPath);
@@ -80,7 +81,6 @@ public class AvailableIndexFoldersBenchmark {
         }
     }
 
-
     @Benchmark
     public Set<String> availableIndexFolderNaive() throws IOException {
         return nodeEnv.availableIndexFoldersForPath(nodePath);

benchmarks/src/main/java/org/elasticsearch/benchmark/indices/breaker/MemoryStatsBenchmark.java

@@ -102,4 +102,3 @@ public class MemoryStatsBenchmark {
         return MEMORY_MX_BEAN.getHeapMemoryUsage().getUsed();
     }
 }
-

benchmarks/src/main/java/org/elasticsearch/benchmark/routing/allocation/AllocationBenchmark.java

@@ -106,8 +106,7 @@ public class AllocationBenchmark {
         "       10|     10|        2|    50",
         "      100|      1|        2|    50",
         "      100|      3|        2|    50",
-        "      100|     10|        2|    50"
+        "      100|     10|        2|    50" })
-    })
     public String indicesShardsReplicasNodes = "10|1|0|1";
 
     public int numTags = 2;
@@ -124,13 +123,14 @@ public class AllocationBenchmark {
         int numReplicas = toInt(params[2]);
         int numNodes = toInt(params[3]);
 
-        strategy = Allocators.createAllocationService(Settings.builder()
+        strategy = Allocators.createAllocationService(
-                .put("cluster.routing.allocation.awareness.attributes", "tag")
+            Settings.builder().put("cluster.routing.allocation.awareness.attributes", "tag").build()
-                .build());
+        );
 
         MetaData.Builder mb = MetaData.builder();
         for (int i = 1; i <= numIndices; i++) {
-            mb.put(IndexMetaData.builder("test_" + i)
+            mb.put(
+                IndexMetaData.builder("test_" + i)
                     .settings(Settings.builder().put("index.version.created", Version.CURRENT))
                     .numberOfShards(numShards)
                     .numberOfReplicas(numReplicas)
@@ -147,8 +147,10 @@ public class AllocationBenchmark {
             nb.add(Allocators.newNode("node" + i, Collections.singletonMap("tag", "tag_" + (i % numTags))));
         }
         initialClusterState = ClusterState.builder(ClusterName.CLUSTER_NAME_SETTING.getDefault(Settings.EMPTY))
-            .metaData(metaData).routingTable(routingTable).nodes
+            .metaData(metaData)
-                (nb).build();
+            .routingTable(routingTable)
+            .nodes(nb)
+            .build();
     }
 
     private int toInt(String v) {
@@ -159,8 +161,10 @@ public class AllocationBenchmark {
     public ClusterState measureAllocation() {
         ClusterState clusterState = initialClusterState;
         while (clusterState.getRoutingNodes().hasUnassignedShards()) {
-            clusterState = strategy.applyStartedShards(clusterState, clusterState.getRoutingNodes()
+            clusterState = strategy.applyStartedShards(
-                    .shardsWithState(ShardRoutingState.INITIALIZING));
+                clusterState,
+                clusterState.getRoutingNodes().shardsWithState(ShardRoutingState.INITIALIZING)
+            );
             clusterState = strategy.reroute(clusterState, "reroute");
         }
         return clusterState;

benchmarks/src/main/java/org/elasticsearch/benchmark/routing/allocation/Allocators.java

@@ -36,7 +36,6 @@ import org.elasticsearch.common.transport.TransportAddress;
 import org.elasticsearch.common.util.set.Sets;
 import org.elasticsearch.gateway.GatewayAllocator;
 
-import java.lang.reflect.InvocationTargetException;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
@@ -67,33 +66,34 @@ public final class Allocators {
         throw new AssertionError("Do not instantiate");
     }
 
-
+    public static AllocationService createAllocationService(Settings settings) {
-    public static AllocationService createAllocationService(Settings settings) throws NoSuchMethodException, InstantiationException,
+        return createAllocationService(settings, new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS));
-        IllegalAccessException, InvocationTargetException {
-        return createAllocationService(settings, new ClusterSettings(Settings.EMPTY, ClusterSettings
-            .BUILT_IN_CLUSTER_SETTINGS));
     }
 
-    public static AllocationService createAllocationService(Settings settings, ClusterSettings clusterSettings) throws
+    public static AllocationService createAllocationService(Settings settings, ClusterSettings clusterSettings) {
-        InvocationTargetException, NoSuchMethodException, InstantiationException, IllegalAccessException {
         return new AllocationService(
             defaultAllocationDeciders(settings, clusterSettings),
-            NoopGatewayAllocator.INSTANCE, new BalancedShardsAllocator(settings), EmptyClusterInfoService.INSTANCE);
+            NoopGatewayAllocator.INSTANCE,
+            new BalancedShardsAllocator(settings),
+            EmptyClusterInfoService.INSTANCE
+        );
     }
 
-    public static AllocationDeciders defaultAllocationDeciders(Settings settings, ClusterSettings clusterSettings) throws
+    public static AllocationDeciders defaultAllocationDeciders(Settings settings, ClusterSettings clusterSettings) {
-        IllegalAccessException, InvocationTargetException, InstantiationException, NoSuchMethodException {
+        Collection<AllocationDecider> deciders = ClusterModule.createAllocationDeciders(settings, clusterSettings, Collections.emptyList());
-        Collection<AllocationDecider> deciders =
-            ClusterModule.createAllocationDeciders(settings, clusterSettings, Collections.emptyList());
         return new AllocationDeciders(deciders);
-
     }
 
     private static final AtomicInteger portGenerator = new AtomicInteger();
 
     public static DiscoveryNode newNode(String nodeId, Map<String, String> attributes) {
-        return new DiscoveryNode("", nodeId, new TransportAddress(TransportAddress.META_ADDRESS,
+        return new DiscoveryNode(
-            portGenerator.incrementAndGet()), attributes, Sets.newHashSet(DiscoveryNodeRole.MASTER_ROLE,
+            "",
-            DiscoveryNodeRole.DATA_ROLE), Version.CURRENT);
+            nodeId,
+            new TransportAddress(TransportAddress.META_ADDRESS, portGenerator.incrementAndGet()),
+            attributes,
+            Sets.newHashSet(DiscoveryNodeRole.MASTER_ROLE, DiscoveryNodeRole.DATA_ROLE),
+            Version.CURRENT
+        );
     }
 }

benchmarks/src/main/java/org/elasticsearch/benchmark/time/RoundingBenchmark.java

@@ -56,10 +56,10 @@ public class RoundingBenchmark {
 
     private long timestamp = 1548879021354L;
 
-    private final org.elasticsearch.common.rounding.Rounding jodaRounding =
+    private final org.elasticsearch.common.rounding.Rounding jodaRounding = org.elasticsearch.common.rounding.Rounding.builder(
-        org.elasticsearch.common.rounding.Rounding.builder(DateTimeUnit.HOUR_OF_DAY).timeZone(timeZone).build();
+        DateTimeUnit.HOUR_OF_DAY
-    private final Rounding javaRounding = Rounding.builder(Rounding.DateTimeUnit.HOUR_OF_DAY)
+    ).timeZone(timeZone).build();
-        .timeZone(zoneId).build();
+    private final Rounding javaRounding = Rounding.builder(Rounding.DateTimeUnit.HOUR_OF_DAY).timeZone(zoneId).build();
 
     @Benchmark
     public long timeRoundingDateTimeUnitJoda() {
@@ -71,10 +71,10 @@ public class RoundingBenchmark {
         return javaRounding.round(timestamp);
     }
 
-    private final org.elasticsearch.common.rounding.Rounding jodaDayOfMonthRounding =
+    private final org.elasticsearch.common.rounding.Rounding jodaDayOfMonthRounding = org.elasticsearch.common.rounding.Rounding.builder(
-        org.elasticsearch.common.rounding.Rounding.builder(DateTimeUnit.DAY_OF_MONTH).timeZone(timeZone).build();
+        DateTimeUnit.DAY_OF_MONTH
-    private final Rounding javaDayOfMonthRounding = Rounding.builder(DAY_OF_MONTH)
+    ).timeZone(timeZone).build();
-        .timeZone(zoneId).build();
+    private final Rounding javaDayOfMonthRounding = Rounding.builder(DAY_OF_MONTH).timeZone(zoneId).build();
 
     @Benchmark
     public long timeRoundingDateTimeUnitDayOfMonthJoda() {
@@ -86,10 +86,10 @@ public class RoundingBenchmark {
         return javaDayOfMonthRounding.round(timestamp);
     }
 
-    private final org.elasticsearch.common.rounding.Rounding timeIntervalRoundingJoda =
+    private final org.elasticsearch.common.rounding.Rounding timeIntervalRoundingJoda = org.elasticsearch.common.rounding.Rounding.builder(
-        org.elasticsearch.common.rounding.Rounding.builder(TimeValue.timeValueMinutes(60)).timeZone(timeZone).build();
+        TimeValue.timeValueMinutes(60)
-    private final Rounding timeIntervalRoundingJava = Rounding.builder(TimeValue.timeValueMinutes(60))
+    ).timeZone(timeZone).build();
-        .timeZone(zoneId).build();
+    private final Rounding timeIntervalRoundingJava = Rounding.builder(TimeValue.timeValueMinutes(60)).timeZone(zoneId).build();
 
     @Benchmark
     public long timeIntervalRoundingJava() {
@@ -101,10 +101,11 @@ public class RoundingBenchmark {
         return timeIntervalRoundingJoda.round(timestamp);
     }
 
-    private final org.elasticsearch.common.rounding.Rounding timeUnitRoundingUtcDayOfMonthJoda =
+    private final org.elasticsearch.common.rounding.Rounding timeUnitRoundingUtcDayOfMonthJoda = org.elasticsearch.common.rounding.Rounding
-        org.elasticsearch.common.rounding.Rounding.builder(DateTimeUnit.DAY_OF_MONTH).timeZone(DateTimeZone.UTC).build();
+        .builder(DateTimeUnit.DAY_OF_MONTH)
-    private final Rounding timeUnitRoundingUtcDayOfMonthJava = Rounding.builder(DAY_OF_MONTH)
+        .timeZone(DateTimeZone.UTC)
-        .timeZone(ZoneOffset.UTC).build();
+        .build();
+    private final Rounding timeUnitRoundingUtcDayOfMonthJava = Rounding.builder(DAY_OF_MONTH).timeZone(ZoneOffset.UTC).build();
 
     @Benchmark
     public long timeUnitRoundingUtcDayOfMonthJava() {
@@ -118,8 +119,7 @@ public class RoundingBenchmark {
 
     private final org.elasticsearch.common.rounding.Rounding timeUnitRoundingUtcQuarterOfYearJoda =
         org.elasticsearch.common.rounding.Rounding.builder(DateTimeUnit.QUARTER).timeZone(DateTimeZone.UTC).build();
-    private final Rounding timeUnitRoundingUtcQuarterOfYearJava = Rounding.builder(QUARTER_OF_YEAR)
+    private final Rounding timeUnitRoundingUtcQuarterOfYearJava = Rounding.builder(QUARTER_OF_YEAR).timeZone(ZoneOffset.UTC).build();
-        .timeZone(ZoneOffset.UTC).build();
 
     @Benchmark
     public long timeUnitRoundingUtcQuarterOfYearJava() {
@@ -131,10 +131,11 @@ public class RoundingBenchmark {
         return timeUnitRoundingUtcQuarterOfYearJoda.round(timestamp);
     }
 
-    private final org.elasticsearch.common.rounding.Rounding timeUnitRoundingUtcMonthOfYearJoda =
+    private final org.elasticsearch.common.rounding.Rounding timeUnitRoundingUtcMonthOfYearJoda = org.elasticsearch.common.rounding.Rounding
-        org.elasticsearch.common.rounding.Rounding.builder(DateTimeUnit.MONTH_OF_YEAR).timeZone(DateTimeZone.UTC).build();
+        .builder(DateTimeUnit.MONTH_OF_YEAR)
-    private final Rounding timeUnitRoundingUtcMonthOfYearJava = Rounding.builder(MONTH_OF_YEAR)
+        .timeZone(DateTimeZone.UTC)
-        .timeZone(ZoneOffset.UTC).build();
+        .build();
+    private final Rounding timeUnitRoundingUtcMonthOfYearJava = Rounding.builder(MONTH_OF_YEAR).timeZone(ZoneOffset.UTC).build();
 
     @Benchmark
     public long timeUnitRoundingUtcMonthOfYearJava() {
@@ -146,11 +147,9 @@ public class RoundingBenchmark {
         return timeUnitRoundingUtcMonthOfYearJoda.round(timestamp);
     }
 
-
     private final org.elasticsearch.common.rounding.Rounding timeUnitRoundingUtcYearOfCenturyJoda =
         org.elasticsearch.common.rounding.Rounding.builder(DateTimeUnit.YEAR_OF_CENTURY).timeZone(DateTimeZone.UTC).build();
-    private final Rounding timeUnitRoundingUtcYearOfCenturyJava = Rounding.builder(YEAR_OF_CENTURY)
+    private final Rounding timeUnitRoundingUtcYearOfCenturyJava = Rounding.builder(YEAR_OF_CENTURY).timeZone(ZoneOffset.UTC).build();
-        .timeZone(ZoneOffset.UTC).build();
 
     @Benchmark
     public long timeUnitRoundingUtcYearOfCenturyJava() {

build.gradle

@@ -108,10 +108,12 @@ subprojects {
     // is greater than the number of unformatted projects, this can be
     // switched to an exclude list, and eventually removed completely.
     def projectPathsToFormat = [
+      ':benchmarks',
       ':build-tools',
       ':distribution:tools:java-version-checker',
       ':distribution:tools:launchers',
       ':distribution:tools:plugin-cli',
+      ':qa:os',
       ':x-pack:plugin:autoscaling',
       ':x-pack:plugin:enrich'
     ]

qa/os/src/test/java/org/elasticsearch/packaging/test/ArchiveTests.java

@@ -30,8 +30,12 @@ import org.junit.BeforeClass;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
+import java.util.ArrayList;
+import java.util.List;
 import java.util.stream.Stream;
 
+import static java.nio.file.StandardOpenOption.APPEND;
+import static java.nio.file.StandardOpenOption.CREATE;
 import static org.elasticsearch.packaging.util.Archives.installArchive;
 import static org.elasticsearch.packaging.util.Archives.verifyArchiveInstallation;
 import static org.elasticsearch.packaging.util.FileExistenceMatchers.fileDoesNotExist;
@@ -121,9 +125,7 @@ public class ArchiveTests extends PackagingTestCase {
             throw e;
         }
 
-        final String gcLogName = Platforms.LINUX && distribution().hasJdk == false
+        final String gcLogName = Platforms.LINUX && distribution().hasJdk == false ? "gc.log.0.current" : "gc.log";
-            ? "gc.log.0.current"
-            : "gc.log";
 
         assertThat(installation.logs.resolve(gcLogName), fileExists());
         ServerUtils.runElasticsearchTests();
@@ -146,8 +148,7 @@ public class ArchiveTests extends PackagingTestCase {
         stopElasticsearch();
 
         String systemJavaHome1 = sh.getEnv().get("JAVA_HOME");
-        assertThat(FileUtils.slurpAllLogs(installation.logs, "elasticsearch.log", "*.log.gz"),
+        assertThat(FileUtils.slurpAllLogs(installation.logs, "elasticsearch.log", "*.log.gz"), containsString(systemJavaHome1));
-            containsString(systemJavaHome1));
     }
 
     public void test52BundledJdkRemoved() throws Exception {
@@ -170,8 +171,7 @@ public class ArchiveTests extends PackagingTestCase {
             stopElasticsearch();
 
             String systemJavaHome1 = sh.getEnv().get("JAVA_HOME");
-            assertThat(FileUtils.slurpAllLogs(installation.logs, "elasticsearch.log", "*.log.gz"),
+            assertThat(FileUtils.slurpAllLogs(installation.logs, "elasticsearch.log", "*.log.gz"), containsString(systemJavaHome1));
-                containsString(systemJavaHome1));
         } finally {
             mv(relocatedJdk, installation.bundledJdk);
         }
@@ -237,11 +237,11 @@ public class ArchiveTests extends PackagingTestCase {
             // we have to disable Log4j from using JMX lest it will hit a security
             // manager exception before we have configured logging; this will fail
             // startup since we detect usages of logging before it is configured
-            final String jvmOptions =
+            final List<String> jvmOptions = new ArrayList<>();
-                "-Xms512m\n" +
+            jvmOptions.add("-Xms512m");
-                "-Xmx512m\n" +
+            jvmOptions.add("-Xmx512m");
-                "-Dlog4j2.disable.jmx=true\n";
+            jvmOptions.add("-Dlog4j2.disable.jmx=true");
-            append(tempConf.resolve("jvm.options"), jvmOptions);
+            Files.write(tempConf.resolve("jvm.options"), jvmOptions, CREATE, APPEND);
 
             sh.chown(tempConf);
 
@@ -324,11 +324,8 @@ public class ArchiveTests extends PackagingTestCase {
 
         try {
             mkdir(tempConf);
-            Stream.of(
+            Stream.of("elasticsearch.yml", "log4j2.properties", "jvm.options")
-                "elasticsearch.yml",
+                .forEach(file -> cp(installation.config(file), tempConf.resolve(file)));
-                "log4j2.properties",
-                "jvm.options"
-            ).forEach(file -> cp(installation.config(file), tempConf.resolve(file)));
 
             append(tempConf.resolve("elasticsearch.yml"), "node.name: relative");
 
@@ -389,8 +386,7 @@ public class ArchiveTests extends PackagingTestCase {
 
         Platforms.PlatformAction action = () -> {
             final Result result = sh.run(bin.nodeTool + " -h");
-            assertThat(result.stdout,
+            assertThat(result.stdout, containsString("A CLI tool to do unsafe cluster and index manipulations on current node"));
-                    containsString("A CLI tool to do unsafe cluster and index manipulations on current node"));
         };
 
         // TODO: this should be checked on all distributions
@@ -420,17 +416,13 @@ public class ArchiveTests extends PackagingTestCase {
 
         Platforms.PlatformAction action = () -> {
             Result result = sh.run(bin.certutilTool + " -h");
-            assertThat(result.stdout,
+            assertThat(result.stdout, containsString("Simplifies certificate creation for use with the Elastic Stack"));
-                containsString("Simplifies certificate creation for use with the Elastic Stack"));
             result = sh.run(bin.syskeygenTool + " -h");
-            assertThat(result.stdout,
+            assertThat(result.stdout, containsString("system key tool"));
-                containsString("system key tool"));
             result = sh.run(bin.setupPasswordsTool + " -h");
-            assertThat(result.stdout,
+            assertThat(result.stdout, containsString("Sets the passwords for reserved users"));
-                containsString("Sets the passwords for reserved users"));
             result = sh.run(bin.usersTool + " -h");
-            assertThat(result.stdout,
+            assertThat(result.stdout, containsString("Manages elasticsearch file users"));
-                containsString("Manages elasticsearch file users"));
         };
 
         // TODO: this should be checked on all distributions

qa/os/src/test/java/org/elasticsearch/packaging/test/CertGenCliTests.java

@@ -31,13 +31,15 @@ import org.junit.BeforeClass;
 
 import java.nio.file.Files;
 import java.nio.file.Path;
-import java.util.Arrays;
+import java.util.ArrayList;
+import java.util.List;
 
 import static com.carrotsearch.randomizedtesting.RandomizedTest.assumeFalse;
+import static java.nio.file.StandardOpenOption.APPEND;
+import static java.nio.file.StandardOpenOption.CREATE;
 import static org.elasticsearch.packaging.util.FileMatcher.Fileness.File;
 import static org.elasticsearch.packaging.util.FileMatcher.file;
 import static org.elasticsearch.packaging.util.FileMatcher.p600;
-import static org.elasticsearch.packaging.util.FileUtils.append;
 import static org.elasticsearch.packaging.util.FileUtils.escapePath;
 import static org.elasticsearch.packaging.util.FileUtils.getTempDir;
 import static org.hamcrest.CoreMatchers.containsString;
@@ -65,17 +67,18 @@ public class CertGenCliTests extends PackagingTestCase {
         install();
     }
 
-    public void test20Help() throws Exception {
+    public void test20Help() {
         Shell.Result result = installation.executables().certgenTool.run("--help");
         assertThat(result.stdout, containsString("Simplifies certificate creation"));
     }
 
     public void test30Generate() throws Exception {
-        Files.write(instancesFile, Arrays.asList(
+        final List<String> yaml = new ArrayList<>();
-            "instances:",
+        yaml.add("instances:");
-            "  - name: \"mynode\"",
+        yaml.add("  - name: \"mynode\"");
-            "    ip:",
+        yaml.add("    ip:");
-            "      - \"127.0.0.1\""));
+        yaml.add("      - \"127.0.0.1\"");
+        Files.write(instancesFile, yaml, CREATE, APPEND);
 
         installation.executables().certgenTool.run("--in " + instancesFile + " --out " + certificatesFile);
 
@@ -105,19 +108,25 @@ public class CertGenCliTests extends PackagingTestCase {
         // windows 2012 r2 has powershell 4.0, which lacks Expand-Archive
         assumeFalse(Platforms.OS_NAME.equals("Windows Server 2012 R2"));
 
-        append(installation.config("elasticsearch.yml"), String.join("\n",
+        final String keyPath = escapePath(installation.config("certs/mynode/mynode.key"));
-            "node.name: mynode",
+        final String certPath = escapePath(installation.config("certs/mynode/mynode.crt"));
-            "xpack.security.transport.ssl.key: " + escapePath(installation.config("certs/mynode/mynode.key")),
+        final String caCertPath = escapePath(installation.config("certs/ca/ca.crt"));
-            "xpack.security.transport.ssl.certificate: " + escapePath(installation.config("certs/mynode/mynode.crt")),
-            "xpack.security.transport.ssl.certificate_authorities: [\"" + escapePath(installation.config("certs/ca/ca.crt")) + "\"]",
-            "xpack.security.http.ssl.key: " + escapePath(installation.config("certs/mynode/mynode.key")),
-            "xpack.security.http.ssl.certificate: "+ escapePath(installation.config("certs/mynode/mynode.crt")),
-            "xpack.security.http.ssl.certificate_authorities: [\"" + escapePath(installation.config("certs/ca/ca.crt")) + "\"]",
-            "xpack.security.transport.ssl.enabled: true",
-            "xpack.security.http.ssl.enabled: true"));
 
-        assertWhileRunning(() -> {
+        List<String> yaml = new ArrayList<>();
-            ServerUtils.makeRequest(Request.Get("https://127.0.0.1:9200"), null, null, installation.config("certs/ca/ca.crt"));
+        yaml.add("node.name: mynode");
-        });
+        yaml.add("xpack.security.transport.ssl.key: " + keyPath);
+        yaml.add("xpack.security.transport.ssl.certificate: " + certPath);
+        yaml.add("xpack.security.transport.ssl.certificate_authorities: [\"" + caCertPath + "\"]");
+        yaml.add("xpack.security.http.ssl.key: " + keyPath);
+        yaml.add("xpack.security.http.ssl.certificate: " + certPath);
+        yaml.add("xpack.security.http.ssl.certificate_authorities: [\"" + caCertPath + "\"]");
+        yaml.add("xpack.security.transport.ssl.enabled: true");
+        yaml.add("xpack.security.http.ssl.enabled: true");
+
+        Files.write(installation.config("elasticsearch.yml"), yaml, CREATE, APPEND);
+
+        assertWhileRunning(
+            () -> ServerUtils.makeRequest(Request.Get("https://127.0.0.1:9200"), null, null, installation.config("certs/ca/ca.crt"))
+        );
     }
 }

qa/os/src/test/java/org/elasticsearch/packaging/test/DebPreservationTests.java

@@ -79,10 +79,7 @@ public class DebPreservationTests extends PackagingTestCase {
 
         // keystore was removed
 
-        assertPathsDoNotExist(
+        assertPathsDoNotExist(installation.config("elasticsearch.keystore"), installation.config(".elasticsearch.keystore.initial_md5sum"));
-            installation.config("elasticsearch.keystore"),
-            installation.config(".elasticsearch.keystore.initial_md5sum")
-        );
 
         // doc files were removed
 
@@ -105,11 +102,7 @@ public class DebPreservationTests extends PackagingTestCase {
 
         assertRemoved(distribution());
 
-        assertPathsDoNotExist(
+        assertPathsDoNotExist(installation.config, installation.envFile, SYSVINIT_SCRIPT);
-            installation.config,
-            installation.envFile,
-            SYSVINIT_SCRIPT
-        );
 
         assertThat(packageStatus(distribution()).exitCode, is(1));
     }

qa/os/src/test/java/org/elasticsearch/packaging/test/KeystoreManagementTests.java

@@ -30,15 +30,16 @@ import org.elasticsearch.packaging.util.Shell;
 import org.junit.Ignore;
 
 import java.io.IOException;
-import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
-import java.nio.file.StandardOpenOption;
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 
+import static java.util.Collections.singletonList;
 import static org.elasticsearch.packaging.util.Archives.ARCHIVE_OWNER;
 import static org.elasticsearch.packaging.util.Archives.installArchive;
 import static org.elasticsearch.packaging.util.Archives.verifyArchiveInstallation;
@@ -82,8 +83,7 @@ public class KeystoreManagementTests extends PackagingTestCase {
         final Installation.Executables bin = installation.executables();
         Shell.Result r = sh.runIgnoreExitCode(bin.keystoreTool.toString() + " has-passwd");
         assertFalse("has-passwd should fail", r.isSuccess());
-        assertThat("has-passwd should indicate missing keystore",
+        assertThat("has-passwd should indicate missing keystore", r.stderr, containsString(ERROR_KEYSTORE_NOT_FOUND));
-            r.stderr, containsString(ERROR_KEYSTORE_NOT_FOUND));
     }
 
     /** Test initial package state */
@@ -98,8 +98,7 @@ public class KeystoreManagementTests extends PackagingTestCase {
         final Installation.Executables bin = installation.executables();
         Shell.Result r = sh.runIgnoreExitCode(bin.keystoreTool.toString() + " has-passwd");
         assertFalse("has-passwd should fail", r.isSuccess());
-        assertThat("has-passwd should indicate unprotected keystore",
+        assertThat("has-passwd should indicate unprotected keystore", r.stderr, containsString(ERROR_KEYSTORE_NOT_PASSWORD_PROTECTED));
-            r.stderr, containsString(ERROR_KEYSTORE_NOT_PASSWORD_PROTECTED));
         Shell.Result r2 = bin.keystoreTool.run("list");
         assertThat(r2.stdout, containsString("keystore.seed"));
     }
@@ -119,8 +118,7 @@ public class KeystoreManagementTests extends PackagingTestCase {
         final Installation.Executables bin = installation.executables();
         Shell.Result r = sh.runIgnoreExitCode(bin.keystoreTool.toString() + " has-passwd");
         assertFalse("has-passwd should fail", r.isSuccess());
-        assertThat("has-passwd should indicate unprotected keystore",
+        assertThat("has-passwd should indicate unprotected keystore", r.stdout, containsString(ERROR_KEYSTORE_NOT_PASSWORD_PROTECTED));
-            r.stdout, containsString(ERROR_KEYSTORE_NOT_PASSWORD_PROTECTED));
         Shell.Result r2 = bin.keystoreTool.run("list");
         assertThat(r2.stdout, containsString("keystore.seed"));
     }
@@ -153,8 +151,7 @@ public class KeystoreManagementTests extends PackagingTestCase {
     }
 
     public void test40KeystorePasswordOnStandardInput() throws Exception {
-        assumeTrue("packages will use systemd, which doesn't handle stdin",
+        assumeTrue("packages will use systemd, which doesn't handle stdin", distribution.isArchive());
-            distribution.isArchive());
         assumeThat(installation, is(notNullValue()));
 
         String password = "^|<>\\&exit"; // code insertion on Windows if special characters are not escaped
@@ -171,8 +168,7 @@ public class KeystoreManagementTests extends PackagingTestCase {
     }
 
     public void test41WrongKeystorePasswordOnStandardInput() {
-        assumeTrue("packages will use systemd, which doesn't handle stdin",
+        assumeTrue("packages will use systemd, which doesn't handle stdin", distribution.isArchive());
-            distribution.isArchive());
         assumeThat(installation, is(notNullValue()));
 
         assertPasswordProtectedKeystore();
@@ -183,10 +179,8 @@ public class KeystoreManagementTests extends PackagingTestCase {
 
     @Ignore /* Ignored for feature branch, awaits fix: https://github.com/elastic/elasticsearch/issues/49340 */
     public void test42KeystorePasswordOnTty() throws Exception {
-        assumeTrue("expect command isn't on Windows",
+        assumeTrue("expect command isn't on Windows", distribution.platform != Distribution.Platform.WINDOWS);
-            distribution.platform != Distribution.Platform.WINDOWS);
+        assumeTrue("packages will use systemd, which doesn't handle stdin", distribution.isArchive());
-        assumeTrue("packages will use systemd, which doesn't handle stdin",
-            distribution.isArchive());
         assumeThat(installation, is(notNullValue()));
 
         String password = "keystorepass";
@@ -204,10 +198,8 @@ public class KeystoreManagementTests extends PackagingTestCase {
 
     @Ignore /* Ignored for feature branch, awaits fix: https://github.com/elastic/elasticsearch/issues/49340 */
     public void test43WrongKeystorePasswordOnTty() throws Exception {
-        assumeTrue("expect command isn't on Windows",
+        assumeTrue("expect command isn't on Windows", distribution.platform != Distribution.Platform.WINDOWS);
-            distribution.platform != Distribution.Platform.WINDOWS);
+        assumeTrue("packages will use systemd, which doesn't handle stdin", distribution.isArchive());
-        assumeTrue("packages will use systemd, which doesn't handle stdin",
-            distribution.isArchive());
         assumeThat(installation, is(notNullValue()));
 
         assertPasswordProtectedKeystore();
@@ -222,8 +214,7 @@ public class KeystoreManagementTests extends PackagingTestCase {
      * view help information.
      */
     public void test44EncryptedKeystoreAllowsHelpMessage() throws Exception {
-        assumeTrue("users call elasticsearch directly in archive case",
+        assumeTrue("users call elasticsearch directly in archive case", distribution.isArchive());
-            distribution.isArchive());
 
         String password = "keystorepass";
 
@@ -251,9 +242,7 @@ public class KeystoreManagementTests extends PackagingTestCase {
             sh.run("sudo systemctl set-environment ES_KEYSTORE_PASSPHRASE_FILE=" + esKeystorePassphraseFile);
 
             Files.createFile(esKeystorePassphraseFile);
-            Files.write(esKeystorePassphraseFile,
+            Files.write(esKeystorePassphraseFile, singletonList(password));
-                (password + System.lineSeparator()).getBytes(StandardCharsets.UTF_8),
-                StandardOpenOption.WRITE);
 
             startElasticsearch();
             ServerUtils.runElasticsearchTests();
@@ -277,9 +266,7 @@ public class KeystoreManagementTests extends PackagingTestCase {
             }
 
             Files.createFile(esKeystorePassphraseFile);
-            Files.write(esKeystorePassphraseFile,
+            Files.write(esKeystorePassphraseFile, singletonList("wrongpassword"));
-                ("wrongpassword" + System.lineSeparator()).getBytes(StandardCharsets.UTF_8),
-                StandardOpenOption.WRITE);
 
             Packages.JournaldWrapper journaldWrapper = new Packages.JournaldWrapper(sh);
             Shell.Result result = runElasticsearchStartCommand();
@@ -323,7 +310,7 @@ public class KeystoreManagementTests extends PackagingTestCase {
 
             String password = "password";
             String passwordFilename = "password.txt";
-            Files.write(tempDir.resolve(passwordFilename), (password + "\n").getBytes(StandardCharsets.UTF_8));
+            Files.write(tempDir.resolve(passwordFilename), singletonList(password));
             Files.setPosixFilePermissions(tempDir.resolve(passwordFilename), p600);
 
             Path dockerKeystore = installation.config("elasticsearch.keystore");
@@ -342,8 +329,7 @@ public class KeystoreManagementTests extends PackagingTestCase {
 
             waitForElasticsearch(installation);
             ServerUtils.runElasticsearchTests();
-        }
+        } finally {
-        finally {
             if (tempDir != null) {
                 rm(tempDir);
             }
@@ -387,9 +373,12 @@ public class KeystoreManagementTests extends PackagingTestCase {
         // It's very tricky to properly quote a pipeline that you're passing to
         // a docker exec command, so we're just going to put a small script in the
         // temp folder.
-        String setPasswordScript = "echo \"" + password + "\n" + password
+        List<String> setPasswordScript = new ArrayList<>();
-            + "\n\" | " + installation.executables().keystoreTool.toString() + " passwd";
+        setPasswordScript.add("echo \"" + password);
-        Files.write(tempDirectory.resolve("set-pass.sh"), setPasswordScript.getBytes(StandardCharsets.UTF_8));
+        setPasswordScript.add(password);
+        setPasswordScript.add("\" | " + installation.executables().keystoreTool.toString() + " passwd");
+
+        Files.write(tempDirectory.resolve("set-pass.sh"), setPasswordScript);
 
         runContainer(distribution(), volumes, null);
         try {
@@ -420,9 +409,7 @@ public class KeystoreManagementTests extends PackagingTestCase {
         // the keystore ends up being owned by the Administrators group, so we manually set it to be owned by the vagrant user here.
         // from the server's perspective the permissions aren't really different, this is just to reflect what we'd expect in the tests.
         // when we run these commands as a role user we won't have to do this
-        Platforms.onWindows(() -> {
+        Platforms.onWindows(() -> sh.chown(keystore));
-            sh.chown(keystore);
-        });
 
         if (distribution().isDocker()) {
             try {
@@ -455,14 +442,11 @@ public class KeystoreManagementTests extends PackagingTestCase {
         final Installation.Executables bin = installation.executables();
 
         // set the password by passing it to stdin twice
-        Platforms.onLinux(() -> {
+        Platforms.onLinux(() -> bin.keystoreTool.run("passwd", password + "\n" + password + "\n"));
-            bin.keystoreTool.run("passwd", password + "\n" + password + "\n");
-        });
 
-        Platforms.onWindows(() -> {
+        Platforms.onWindows(
-            sh.run("Invoke-Command -ScriptBlock {echo \'" + password + "\'; echo \'" + password + "\'} | "
+            () -> sh.run("Invoke-Command -ScriptBlock {echo '" + password + "'; echo '" + password + "'} | " + bin.keystoreTool + " passwd")
-                + bin.keystoreTool + " passwd");
+        );
-        });
     }
 
     private void assertPasswordProtectedKeystore() {

qa/os/src/test/java/org/elasticsearch/packaging/test/PackageTests.java

@@ -26,15 +26,15 @@ import org.elasticsearch.packaging.util.Packages;
 import org.elasticsearch.packaging.util.Shell.Result;
 import org.junit.BeforeClass;
 
-import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
-import java.nio.file.StandardOpenOption;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
 import static com.carrotsearch.randomizedtesting.RandomizedTest.getRandom;
+import static java.nio.file.StandardOpenOption.APPEND;
+import static java.util.Collections.singletonList;
 import static org.elasticsearch.packaging.util.FileExistenceMatchers.fileDoesNotExist;
 import static org.elasticsearch.packaging.util.FileExistenceMatchers.fileExists;
 import static org.elasticsearch.packaging.util.FileUtils.append;
@@ -98,8 +98,7 @@ public class PackageTests extends PackagingTestCase {
     private void assertRunsWithJavaHome() throws Exception {
         byte[] originalEnvFile = Files.readAllBytes(installation.envFile);
         try {
-            Files.write(installation.envFile, ("JAVA_HOME=" + systemJavaHome + "\n").getBytes(StandardCharsets.UTF_8),
+            Files.write(installation.envFile, singletonList("JAVA_HOME=" + systemJavaHome), APPEND);
-                StandardOpenOption.APPEND);
             startElasticsearch();
             runElasticsearchTests();
             stopElasticsearch();
@@ -107,8 +106,7 @@ public class PackageTests extends PackagingTestCase {
             Files.write(installation.envFile, originalEnvFile);
         }
 
-        assertThat(FileUtils.slurpAllLogs(installation.logs, "elasticsearch.log", "elasticsearch*.log.gz"),
+        assertThat(FileUtils.slurpAllLogs(installation.logs, "elasticsearch.log", "elasticsearch*.log.gz"), containsString(systemJavaHome));
-            containsString(systemJavaHome));
     }
 
     public void test32JavaHomeOverride() throws Exception {
@@ -170,9 +168,12 @@ public class PackageTests extends PackagingTestCase {
         String start = sh.runIgnoreExitCode("date ").stdout.trim();
         startElasticsearch();
 
-        String journalEntries = sh.runIgnoreExitCode("journalctl _SYSTEMD_UNIT=elasticsearch.service " +
+        String journalEntries = sh.runIgnoreExitCode(
-            "--since \"" + start + "\" --output cat | grep -v \"future versions of Elasticsearch will require Java 11\" | wc -l")
+            "journalctl _SYSTEMD_UNIT=elasticsearch.service "
-            .stdout.trim();
+                + "--since \""
+                + start
+                + "\" --output cat | grep -v \"future versions of Elasticsearch will require Java 11\" | wc -l"
+        ).stdout.trim();
         assertThat(journalEntries, equalTo("0"));
 
         assertPathsExist(installation.pidDir.resolve("elasticsearch.pid"));
@@ -212,9 +213,7 @@ public class PackageTests extends PackagingTestCase {
                 matcher.find();
                 final int version = Integer.parseInt(matcher.group(1));
 
-                statusExitCode = version < 231
+                statusExitCode = version < 231 ? 3 : 4;
-                    ? 3
-                    : 4;
             }
 
             assertThat(sh.runIgnoreExitCode("systemctl status elasticsearch.service").exitCode, is(statusExitCode));
@@ -257,7 +256,6 @@ public class PackageTests extends PackagingTestCase {
         }
     }
 
-
     public void test72TestRuntimeDirectory() throws Exception {
         try {
             install();
@@ -280,7 +278,6 @@ public class PackageTests extends PackagingTestCase {
 
     // TEST CASES FOR SYSTEMD ONLY
 
-
     /**
      * # Simulates the behavior of a system restart:
      * # the PID directory is deleted by the operating system

qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java

@@ -67,9 +67,7 @@ import static org.junit.Assume.assumeTrue;
  * Class that all packaging test cases should inherit from
  */
 @RunWith(RandomizedRunner.class)
-@TestMethodProviders({
+@TestMethodProviders({ JUnit3MethodProvider.class })
-    JUnit3MethodProvider.class
-})
 @Timeout(millis = 20 * 60 * 1000) // 20 min
 @TestCaseOrdering(TestCaseOrdering.AlphabeticOrder.class)
 public abstract class PackagingTestCase extends Assert {
@@ -142,19 +140,14 @@ public abstract class PackagingTestCase extends Assert {
         }
     }
 
-
     @Before
     public void setup() throws Exception {
         assumeFalse(failed); // skip rest of tests once one fails
 
         sh.reset();
         if (distribution().hasJdk == false) {
-            Platforms.onLinux(() -> {
+            Platforms.onLinux(() -> sh.getEnv().put("JAVA_HOME", systemJavaHome));
-                sh.getEnv().put("JAVA_HOME", systemJavaHome);
+            Platforms.onWindows(() -> sh.getEnv().put("JAVA_HOME", systemJavaHome));
-            });
-            Platforms.onWindows(() -> {
-                sh.getEnv().put("JAVA_HOME", systemJavaHome);
-            });
         }
     }
 
@@ -215,8 +208,7 @@ public abstract class PackagingTestCase extends Assert {
                 sh.runIgnoreExitCode("jstack " + pid);
             }
             if (Files.exists(installation.logs.resolve("elasticsearch.log"))) {
-                logger.warn("Elasticsearch log:\n" +
+                logger.warn("Elasticsearch log:\n" + FileUtils.slurpAllLogs(installation.logs, "elasticsearch.log", "*.log.gz"));
-                    FileUtils.slurpAllLogs(installation.logs, "elasticsearch.log", "*.log.gz"));
             }
             if (Files.exists(installation.logs.resolve("output.out"))) {
                 logger.warn("Stdout:\n" + FileUtils.slurpTxtorGz(installation.logs.resolve("output.out")));
@@ -230,8 +222,7 @@ public abstract class PackagingTestCase extends Assert {
         try {
             assertions.run();
         } catch (Exception e) {
-            logger.warn("Elasticsearch log:\n" +
+            logger.warn("Elasticsearch log:\n" + FileUtils.slurpAllLogs(installation.logs, "elasticsearch.log", "*.log.gz"));
-                FileUtils.slurpAllLogs(installation.logs, "elasticsearch.log", "*.log.gz"));
             throw e;
         }
         stopElasticsearch();
@@ -345,9 +336,11 @@ public abstract class PackagingTestCase extends Assert {
             // in the background
             String wrapperPid = result.stdout.trim();
             sh.runIgnoreExitCode("Wait-Process -Timeout " + Archives.ES_STARTUP_SLEEP_TIME_SECONDS + " -Id " + wrapperPid);
-            sh.runIgnoreExitCode("Get-EventSubscriber | " +
+            sh.runIgnoreExitCode(
-                "where {($_.EventName -eq 'OutputDataReceived' -Or $_.EventName -eq 'ErrorDataReceived' |" +
+                "Get-EventSubscriber | "
-                "Unregister-EventSubscriber -Force");
+                    + "where {($_.EventName -eq 'OutputDataReceived' -Or $_.EventName -eq 'ErrorDataReceived' |"
+                    + "Unregister-EventSubscriber -Force"
+            );
             assertThat(FileUtils.slurp(Archives.getPowershellErrorPath(installation)), anyOf(stringMatchers));
 
         } else {

qa/os/src/test/java/org/elasticsearch/packaging/test/PasswordToolsTests.java

@@ -28,13 +28,15 @@ import org.junit.Before;
 
 import java.nio.file.Files;
 import java.nio.file.Path;
+import java.nio.file.StandardOpenOption;
+import java.util.ArrayList;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 import java.util.stream.Stream;
 
-import static org.elasticsearch.packaging.util.FileUtils.append;
 import static org.hamcrest.CoreMatchers.containsString;
 import static org.hamcrest.collection.IsMapContaining.hasKey;
 import static org.junit.Assume.assumeTrue;
@@ -52,9 +54,12 @@ public class PasswordToolsTests extends PackagingTestCase {
 
     public void test010Install() throws Exception {
         install();
-        append(installation.config("elasticsearch.yml"),
+
-            "xpack.license.self_generated.type: trial\n" +
+        List<String> lines = new ArrayList<>();
-            "xpack.security.enabled: true");
+        lines.add("xpack.license.self_generated.type: trial");
+        lines.add("xpack.security.enabled: true");
+
+        Files.write(installation.config("elasticsearch.yml"), lines, StandardOpenOption.APPEND);
     }
 
     public void test20GeneratePasswords() throws Exception {
@@ -63,7 +68,11 @@ public class PasswordToolsTests extends PackagingTestCase {
             Map<String, String> userpasses = parseUsersAndPasswords(result.stdout);
             for (Map.Entry<String, String> userpass : userpasses.entrySet()) {
                 String response = ServerUtils.makeRequest(
-                    Request.Get("http://localhost:9200"), userpass.getKey(), userpass.getValue(), null);
+                    Request.Get("http://localhost:9200"),
+                    userpass.getKey(),
+                    userpass.getValue(),
+                    null
+                );
                 assertThat(response, containsString("You Know, for Search"));
             }
         });
@@ -112,7 +121,10 @@ public class PasswordToolsTests extends PackagingTestCase {
         assertWhileRunning(() -> {
             String response = ServerUtils.makeRequest(
                 Request.Get("http://localhost:9200/_cluster/health?wait_for_status=green&timeout=180s"),
-                "elastic", BOOTSTRAP_PASSWORD, null);
+                "elastic",
+                BOOTSTRAP_PASSWORD,
+                null
+            );
             assertThat(response, containsString("\"status\":\"green\""));
         });
     }
@@ -125,7 +137,11 @@ public class PasswordToolsTests extends PackagingTestCase {
             assertThat(userpasses, hasKey("elastic"));
             for (Map.Entry<String, String> userpass : userpasses.entrySet()) {
                 String response = ServerUtils.makeRequest(
-                    Request.Get("http://localhost:9200"), userpass.getKey(), userpass.getValue(), null);
+                    Request.Get("http://localhost:9200"),
+                    userpass.getKey(),
+                    userpass.getValue(),
+                    null
+                );
                 assertThat(response, containsString("You Know, for Search"));
             }
         });

qa/os/src/test/java/org/elasticsearch/packaging/test/RpmPreservationTests.java

@@ -78,21 +78,12 @@ public class RpmPreservationTests extends PackagingTestCase {
         verifyPackageInstallation(installation, distribution(), sh);
 
         sh.run("echo foobar | " + installation.executables().keystoreTool + " add --stdin foo.bar");
-        Stream.of(
+        Stream.of("elasticsearch.yml", "jvm.options", "log4j2.properties")
-            "elasticsearch.yml",
-            "jvm.options",
-            "log4j2.properties"
-        )
             .map(each -> installation.config(each))
             .forEach(path -> append(path, "# foo"));
         append(installation.config(Paths.get("jvm.options.d", "heap.options")), "# foo");
         if (distribution().isDefault()) {
-            Stream.of(
+            Stream.of("role_mapping.yml", "roles.yml", "users", "users_roles")
-                "role_mapping.yml",
-                "roles.yml",
-                "users",
-                "users_roles"
-            )
                 .map(each -> installation.config(each))
                 .forEach(path -> append(path, "# foo"));
         }
@@ -119,20 +110,11 @@ public class RpmPreservationTests extends PackagingTestCase {
         assertThat(installation.config, fileExists());
         assertThat(installation.config("elasticsearch.keystore"), fileExists());
 
-        Stream.of(
+        Stream.of("elasticsearch.yml", "jvm.options", "log4j2.properties").forEach(this::assertConfFilePreserved);
-            "elasticsearch.yml",
-            "jvm.options",
-            "log4j2.properties"
-        ).forEach(this::assertConfFilePreserved);
         assertThat(installation.config(Paths.get("jvm.options.d", "heap.options")), fileExists());
 
         if (distribution().isDefault()) {
-            Stream.of(
+            Stream.of("role_mapping.yml", "roles.yml", "users", "users_roles").forEach(this::assertConfFilePreserved);
-                "role_mapping.yml",
-                "roles.yml",
-                "users",
-                "users_roles"
-            ).forEach(this::assertConfFilePreserved);
         }
     }
 

qa/os/src/test/java/org/elasticsearch/packaging/test/WindowsServiceTests.java

@@ -82,12 +82,16 @@ public class WindowsServiceTests extends PackagingTestCase {
             logger.error("---- Unexpected exit code (expected " + exitCode + ", got " + result.exitCode + ") for script: " + script);
             logger.error(result);
             logger.error("Dumping log files\n");
-            Result logs = sh.run("$files = Get-ChildItem \"" + installation.logs + "\\elasticsearch.log\"; " +
+            Result logs = sh.run(
-                "Write-Output $files; " +
+                "$files = Get-ChildItem \""
-                "foreach ($file in $files) {" +
+                    + installation.logs
-                    "Write-Output \"$file\"; " +
+                    + "\\elasticsearch.log\"; "
-                    "Get-Content \"$file\" " +
+                    + "Write-Output $files; "
-                "}");
+                    + "foreach ($file in $files) {"
+                    + "    Write-Output \"$file\"; "
+                    + "    Get-Content \"$file\" "
+                    + "}"
+            );
             logger.error(logs.stdout);
             fail();
         } else {
@@ -167,28 +171,32 @@ public class WindowsServiceTests extends PackagingTestCase {
         assertCommand(serviceScript + " stop");
         assertService(DEFAULT_ID, "Stopped", DEFAULT_DISPLAY_NAME);
         // the process is stopped async, and can become a zombie process, so we poll for the process actually being gone
-        assertCommand("$p = Get-Service -Name \"elasticsearch-service-x64\" -ErrorAction SilentlyContinue;" +
+        assertCommand(
-            "$i = 0;" +
+            "$p = Get-Service -Name \"elasticsearch-service-x64\" -ErrorAction SilentlyContinue;"
-            "do {" +
+                + "$i = 0;"
-              "$p = Get-Process -Name \"elasticsearch-service-x64\" -ErrorAction SilentlyContinue;" +
+                + "do {"
-              "echo \"$p\";" +
+                + "  $p = Get-Process -Name \"elasticsearch-service-x64\" -ErrorAction SilentlyContinue;"
-              "if ($p -eq $Null) {" +
+                + "  echo \"$p\";"
-              "  Write-Host \"exited after $i seconds\";" +
+                + "  if ($p -eq $Null) {"
-              "  exit 0;" +
+                + "    Write-Host \"exited after $i seconds\";"
-              "}" +
+                + "    exit 0;"
-              "Start-Sleep -Seconds 1;" +
+                + "  }"
-              "$i += 1;" +
+                + "  Start-Sleep -Seconds 1;"
-            "} while ($i -lt 300);" +
+                + "  $i += 1;"
-            "exit 9;");
+                + "} while ($i -lt 300);"
+                + "exit 9;"
+        );
 
         assertCommand(serviceScript + " remove");
-        assertCommand("$p = Get-Service -Name \"elasticsearch-service-x64\" -ErrorAction SilentlyContinue;" +
+        assertCommand(
-            "echo \"$p\";" +
+            "$p = Get-Service -Name \"elasticsearch-service-x64\" -ErrorAction SilentlyContinue;"
-            "if ($p -eq $Null) {" +
+                + "echo \"$p\";"
-            "  exit 0;" +
+                + "if ($p -eq $Null) {"
-            "} else {" +
+                + "  exit 0;"
-            "  exit 1;" +
+                + "} else {"
-            "}");
+                + "  exit 1;"
+                + "}"
+        );
     }
 
     public void test30StartStop() throws Exception {

qa/os/src/test/java/org/elasticsearch/packaging/util/Archives.java

@@ -26,6 +26,7 @@ import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.util.List;
+import java.util.Locale;
 import java.util.stream.Stream;
 
 import static java.util.stream.Collectors.joining;
@@ -59,9 +60,7 @@ public class Archives {
     protected static final Logger logger = LogManager.getLogger(Archives.class);
 
     // in the future we'll run as a role user on Windows
-    public static final String ARCHIVE_OWNER = Platforms.WINDOWS
+    public static final String ARCHIVE_OWNER = Platforms.WINDOWS ? System.getenv("username") : "elasticsearch";
-        ? System.getenv("username")
-        : "elasticsearch";
 
     /** This is an arbitrarily chosen value that gives Elasticsearch time to log Bootstrap
      *  errors to the console if they occur before the logging framework is initialized. */
@@ -91,9 +90,12 @@ public class Archives {
             if (Platforms.WINDOWS == false) {
                 throw new IllegalStateException("Distribution " + distribution + " is not supported on linux");
             }
-            installCommand =
+            installCommand = String.format(
-                "Add-Type -AssemblyName 'System.IO.Compression.Filesystem'; " +
+                Locale.ROOT,
-                "[IO.Compression.ZipFile]::ExtractToDirectory('" + distributionFile + "', '" + baseInstallPath + "')";
+                "Add-Type -AssemblyName 'System.IO.Compression.Filesystem'; [IO.Compression.ZipFile]::ExtractToDirectory('%s', '%s')",
+                distributionFile,
+                baseInstallPath
+            );
 
         } else {
             throw new RuntimeException("Distribution " + distribution + " is not a known archive type");
@@ -129,22 +131,26 @@ public class Archives {
 
         if (sh.runIgnoreExitCode("id elasticsearch").isSuccess() == false) {
             if (isDPKG()) {
-                sh.run("adduser " +
+                sh.run(
-                    "--quiet " +
+                    "adduser "
-                    "--system " +
+                        + "--quiet "
-                    "--no-create-home " +
+                        + "--system "
-                    "--ingroup elasticsearch " +
+                        + "--no-create-home "
-                    "--disabled-password " +
+                        + "--ingroup elasticsearch "
-                    "--shell /bin/false " +
+                        + "--disabled-password "
-                    "elasticsearch");
+                        + "--shell /bin/false "
+                        + "elasticsearch"
+                );
             } else {
-                sh.run("useradd " +
+                sh.run(
-                    "--system " +
+                    "useradd "
-                    "-M " +
+                        + "--system "
-                    "--gid elasticsearch " +
+                        + "-M "
-                    "--shell /sbin/nologin " +
+                        + "--gid elasticsearch "
-                    "--comment 'elasticsearch user' " +
+                        + "--shell /sbin/nologin "
-                    "elasticsearch");
+                        + "--comment 'elasticsearch user' "
+                        + "elasticsearch"
+                );
             }
         }
     }
@@ -157,13 +163,7 @@ public class Archives {
     }
 
     private static void verifyOssInstallation(Installation es, Distribution distribution, String owner) {
-        Stream.of(
+        Stream.of(es.home, es.config, es.plugins, es.modules, es.logs).forEach(dir -> assertThat(dir, file(Directory, owner, owner, p755)));
-            es.home,
-            es.config,
-            es.plugins,
-            es.modules,
-            es.logs
-        ).forEach(dir -> assertThat(dir, file(Directory, owner, owner, p755)));
 
         assertThat(Files.exists(es.data), is(false));
 
@@ -188,24 +188,15 @@ public class Archives {
         });
 
         if (distribution.packaging == Distribution.Packaging.ZIP) {
-            Stream.of(
+            Stream.of("elasticsearch-service.bat", "elasticsearch-service-mgr.exe", "elasticsearch-service-x64.exe")
-                "elasticsearch-service.bat",
+                .forEach(executable -> assertThat(es.bin(executable), file(File, owner)));
-                "elasticsearch-service-mgr.exe",
-                "elasticsearch-service-x64.exe"
-            ).forEach(executable -> assertThat(es.bin(executable), file(File, owner)));
         }
 
-        Stream.of(
+        Stream.of("elasticsearch.yml", "jvm.options", "log4j2.properties")
-            "elasticsearch.yml",
+            .forEach(configFile -> assertThat(es.config(configFile), file(File, owner, owner, p660)));
-            "jvm.options",
-            "log4j2.properties"
-        ).forEach(configFile -> assertThat(es.config(configFile), file(File, owner, owner, p660)));
 
-        Stream.of(
+        Stream.of("NOTICE.txt", "LICENSE.txt", "README.asciidoc")
-            "NOTICE.txt",
+            .forEach(doc -> assertThat(es.home.resolve(doc), file(File, owner, owner, p644)));
-            "LICENSE.txt",
-            "README.asciidoc"
-        ).forEach(doc -> assertThat(es.home.resolve(doc), file(File, owner, owner, p644)));
     }
 
     private static void verifyDefaultInstallation(Installation es, Distribution distribution, String owner) {
@@ -236,13 +227,8 @@ public class Archives {
         // the version through here
         assertThat(es.bin("elasticsearch-sql-cli-" + getCurrentVersion() + ".jar"), file(File, owner, owner, p755));
 
-        Stream.of(
+        Stream.of("users", "users_roles", "roles.yml", "role_mapping.yml", "log4j2.properties")
-            "users",
+            .forEach(configFile -> assertThat(es.config(configFile), file(File, owner, owner, p660)));
-            "users_roles",
-            "roles.yml",
-            "role_mapping.yml",
-            "log4j2.properties"
-        ).forEach(configFile -> assertThat(es.config(configFile), file(File, owner, owner, p660)));
     }
 
     public static Shell.Result startElasticsearch(Installation installation, Shell sh) {
@@ -254,13 +240,20 @@ public class Archives {
         final Installation.Executables bin = installation.executables();
 
         // requires the "expect" utility to be installed
-        String script = "expect -c \"$(cat<<EXPECT\n" +
+        String script = String.format(
-            "spawn -ignore HUP sudo -E -u " + ARCHIVE_OWNER + " " + bin.elasticsearch + " -d -p " + pidFile + "\n" +
+            Locale.ROOT,
-            "expect \"Elasticsearch keystore password:\"\n" +
+            "expect -c \"$(cat<<EXPECT\n"
-            "send \"" + keystorePassword + "\\r\"\n" +
+                + "spawn -ignore HUP sudo -E -u %s %s -d -p %s \n"
-            "expect eof\n" +
+                + "expect \"Elasticsearch keystore password:\"\n"
-            "EXPECT\n" +
+                + "send \"%s\\r\"\n"
-            ")\"";
+                + "expect eof\n"
+                + "EXPECT\n"
+                + ")\"",
+            ARCHIVE_OWNER,
+            bin.elasticsearch,
+            pidFile,
+            keystorePassword
+        );
 
         sh.getEnv().put("ES_STARTUP_SLEEP_TIME", ES_STARTUP_SLEEP_TIME_SECONDS);
         return sh.runIgnoreExitCode(script);
@@ -283,8 +276,9 @@ public class Archives {
 
             // We need to give Elasticsearch enough time to print failures to stderr before exiting
             sh.getEnv().put("ES_STARTUP_SLEEP_TIME", ES_STARTUP_SLEEP_TIME_SECONDS);
-            return sh.runIgnoreExitCode("sudo -E -u " + ARCHIVE_OWNER + " " + bin.elasticsearch + " -d -p " + pidFile +
+            return sh.runIgnoreExitCode(
-                " <<<'" + keystorePassword + "'");
+                "sudo -E -u " + ARCHIVE_OWNER + " " + bin.elasticsearch + " -d -p " + pidFile + " <<<'" + keystorePassword + "'"
+            );
         }
         final Path stdout = getPowershellOutputPath(installation);
         final Path stderr = getPowershellErrorPath(installation);
@@ -294,44 +288,58 @@ public class Archives {
             // the tests will run as Administrator in vagrant.
             // we don't want to run the server as Administrator, so we provide the current user's
             // username and password to the process which has the effect of starting it not as Administrator.
-            powerShellProcessUserSetup =
+            powerShellProcessUserSetup = "$password = ConvertTo-SecureString 'vagrant' -AsPlainText -Force; "
-                "$password = ConvertTo-SecureString 'vagrant' -AsPlainText -Force; " +
+                + "$processInfo.Username = 'vagrant'; "
-                "$processInfo.Username = 'vagrant'; " +
+                + "$processInfo.Password = $password; ";
-                "$processInfo.Password = $password; ";
         } else {
             powerShellProcessUserSetup = "";
         }
 
         // this starts the server in the background. the -d flag is unsupported on windows
         return sh.run(
-            "$processInfo = New-Object System.Diagnostics.ProcessStartInfo; " +
+            "$processInfo = New-Object System.Diagnostics.ProcessStartInfo; "
-                "$processInfo.FileName = '" + bin.elasticsearch + "'; " +
+                + "$processInfo.FileName = '"
-                "$processInfo.Arguments = '-p " + installation.home.resolve("elasticsearch.pid") + "'; " +
+                + bin.elasticsearch
-                powerShellProcessUserSetup +
+                + "'; "
-                "$processInfo.RedirectStandardOutput = $true; " +
+                + "$processInfo.Arguments = '-p "
-                "$processInfo.RedirectStandardError = $true; " +
+                + installation.home.resolve("elasticsearch.pid")
-                "$processInfo.RedirectStandardInput = $true; " +
+                + "'; "
-                sh.env.entrySet().stream()
+                + powerShellProcessUserSetup
+                + "$processInfo.RedirectStandardOutput = $true; "
+                + "$processInfo.RedirectStandardError = $true; "
+                + "$processInfo.RedirectStandardInput = $true; "
+                + sh.env.entrySet()
+                    .stream()
                     .map(entry -> "$processInfo.Environment.Add('" + entry.getKey() + "', '" + entry.getValue() + "'); ")
-                    .collect(joining()) +
+                    .collect(joining())
-                "$processInfo.UseShellExecute = $false; " +
+                + "$processInfo.UseShellExecute = $false; "
-                "$process = New-Object System.Diagnostics.Process; " +
+                + "$process = New-Object System.Diagnostics.Process; "
-                "$process.StartInfo = $processInfo; " +
+                + "$process.StartInfo = $processInfo; "
+                +
 
                 // set up some asynchronous output handlers
-                "$outScript = { $EventArgs.Data | Out-File -Encoding UTF8 -Append '" + stdout + "' }; " +
+                "$outScript = { $EventArgs.Data | Out-File -Encoding UTF8 -Append '"
-                "$errScript = { $EventArgs.Data | Out-File -Encoding UTF8 -Append '" + stderr + "' }; " +
+                + stdout
-                "$stdOutEvent = Register-ObjectEvent -InputObject $process " +
+                + "' }; "
-                "-Action $outScript -EventName 'OutputDataReceived'; " +
+                + "$errScript = { $EventArgs.Data | Out-File -Encoding UTF8 -Append '"
-                "$stdErrEvent = Register-ObjectEvent -InputObject $process " +
+                + stderr
-                "-Action $errScript -EventName 'ErrorDataReceived'; " +
+                + "' }; "
+                + "$stdOutEvent = Register-ObjectEvent -InputObject $process "
+                + "-Action $outScript -EventName 'OutputDataReceived'; "
+                + "$stdErrEvent = Register-ObjectEvent -InputObject $process "
+                + "-Action $errScript -EventName 'ErrorDataReceived'; "
+                +
 
-                "$process.Start() | Out-Null; " +
+                "$process.Start() | Out-Null; "
-                "$process.BeginOutputReadLine(); " +
+                + "$process.BeginOutputReadLine(); "
-                "$process.BeginErrorReadLine(); " +
+                + "$process.BeginErrorReadLine(); "
-                "$process.StandardInput.WriteLine('" + keystorePassword + "'); " +
+                + "$process.StandardInput.WriteLine('"
-                "Wait-Process -Timeout " + ES_STARTUP_SLEEP_TIME_SECONDS + " -Id $process.Id; " +
+                + keystorePassword
-                "$process.Id;"
+                + "'); "
+                + "Wait-Process -Timeout "
+                + ES_STARTUP_SLEEP_TIME_SECONDS
+                + " -Id $process.Id; "
+                + "$process.Id;"
         );
     }
 
@@ -356,9 +364,11 @@ public class Archives {
             sh.run("Get-Process -Id " + pid + " | Stop-Process -Force; Wait-Process -Id " + pid);
 
             // Clear the asynchronous event handlers
-            sh.runIgnoreExitCode("Get-EventSubscriber | " +
+            sh.runIgnoreExitCode(
-                "where {($_.EventName -eq 'OutputDataReceived' -Or $_.EventName -eq 'ErrorDataReceived' |" +
+                "Get-EventSubscriber | "
-                "Unregister-EventSubscriber -Force");
+                    + "where {($_.EventName -eq 'OutputDataReceived' -Or $_.EventName -eq 'ErrorDataReceived' |"
+                    + "Unregister-EventSubscriber -Force"
+            );
         });
         if (Files.exists(pidFile)) {
             Files.delete(pidFile);

qa/os/src/test/java/org/elasticsearch/packaging/util/Cleanup.java

@@ -62,14 +62,16 @@ public class Cleanup {
             sh.runIgnoreExitCode("ps aux | grep -i 'org.elasticsearch.bootstrap.Elasticsearch' | awk {'print $2'} | xargs kill -9");
         });
 
-        Platforms.onWindows(() -> {
+        Platforms.onWindows(
+            () -> {
                 // the view of processes returned by Get-Process doesn't expose command line arguments, so we use WMI here
                 sh.runIgnoreExitCode(
-                "Get-WmiObject Win32_Process | " +
+                    "Get-WmiObject Win32_Process | "
-                "Where-Object { $_.CommandLine -Match 'org.elasticsearch.bootstrap.Elasticsearch' } | " +
+                        + "Where-Object { $_.CommandLine -Match 'org.elasticsearch.bootstrap.Elasticsearch' } | "
-                "ForEach-Object { $_.Terminate() }"
+                        + "ForEach-Object { $_.Terminate() }"
+                );
+            }
         );
-        });
 
         Platforms.onLinux(Cleanup::purgePackagesLinux);
 
@@ -85,10 +87,7 @@ public class Cleanup {
         final List<String> filesToDelete = Platforms.WINDOWS ? ELASTICSEARCH_FILES_WINDOWS : ELASTICSEARCH_FILES_LINUX;
         // windows needs leniency due to asinine releasing of file locking async from a process exiting
         Consumer<? super Path> rm = Platforms.WINDOWS ? FileUtils::rmWithRetries : FileUtils::rm;
-        filesToDelete.stream()
+        filesToDelete.stream().map(Paths::get).filter(Files::exists).forEach(rm);
-            .map(Paths::get)
-            .filter(Files::exists)
-            .forEach(rm);
 
         // disable elasticsearch service
         // todo add this for windows when adding tests for service intallation

qa/os/src/test/java/org/elasticsearch/packaging/util/Docker.java

@@ -79,10 +79,7 @@ public class Docker {
     public static void ensureImageIsLoaded(Distribution distribution) {
         Shell.Result result = sh.run("docker image ls --format '{{.Repository}}' " + distribution.flavor.name);
 
-        final long count = Arrays.stream(result.stdout.split("\n"))
+        final long count = Arrays.stream(result.stdout.split("\n")).map(String::trim).filter(s -> s.isEmpty() == false).count();
-            .map(String::trim)
-            .filter(s -> s.isEmpty() == false)
-            .count();
 
         if (count != 0) {
             return;

qa/os/src/test/java/org/elasticsearch/packaging/util/FileMatcher.java

@@ -43,7 +43,10 @@ import static org.elasticsearch.packaging.util.FileUtils.getPosixFileAttributes;
  */
 public class FileMatcher extends TypeSafeMatcher<Path> {
 
-    public enum Fileness { File, Directory }
+    public enum Fileness {
+        File,
+        Directory
+    }
 
     public static final Set<PosixFilePermission> p775 = fromString("rwxrwxr-x");
     public static final Set<PosixFilePermission> p770 = fromString("rwxrwx---");
@@ -126,10 +129,14 @@ public class FileMatcher extends TypeSafeMatcher<Path> {
 
     @Override
     public void describeTo(Description description) {
-        description.appendValue("file/directory: ").appendValue(fileness)
+        description.appendValue("file/directory: ")
-            .appendText(" with owner ").appendValue(owner)
+            .appendValue(fileness)
-            .appendText(" with group ").appendValue(group)
+            .appendText(" with owner ")
-            .appendText(" with posix permissions ").appendValueList("[", ",", "]", posixPermissions);
+            .appendValue(owner)
+            .appendText(" with group ")
+            .appendValue(group)
+            .appendText(" with posix permissions ")
+            .appendValueList("[", ",", "]", posixPermissions);
     }
 
     public static FileMatcher file(Fileness fileness, String owner) {

qa/os/src/test/java/org/elasticsearch/packaging/util/FileUtils.java

@@ -34,6 +34,7 @@ import java.nio.charset.StandardCharsets;
 import java.nio.file.DirectoryStream;
 import java.nio.file.Files;
 import java.nio.file.LinkOption;
+import java.nio.file.OpenOption;
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.nio.file.StandardOpenOption;
@@ -117,7 +118,6 @@ public class FileUtils {
         }
     }
 
-
     public static Path mkdir(Path path) {
         try {
             return Files.createDirectories(path);
@@ -142,9 +142,22 @@ public class FileUtils {
         }
     }
 
+    /**
+     * Creates or appends to the specified file, and writes the supplied string to it.
+     * No newline is written - if a trailing newline is required, it should be present
+     * in <code>text</code>, or use {@link Files#write(Path, Iterable, OpenOption...)}.
+     * @param file the file to create or append
+     * @param text the string to write
+     */
     public static void append(Path file, String text) {
-        try (BufferedWriter writer = Files.newBufferedWriter(file, StandardCharsets.UTF_8,
+        try (
-            StandardOpenOption.CREATE, StandardOpenOption.APPEND)) {
+            BufferedWriter writer = Files.newBufferedWriter(
+                file,
+                StandardCharsets.UTF_8,
+                StandardOpenOption.CREATE,
+                StandardOpenOption.APPEND
+            )
+        ) {
 
             writer.write(text);
         } catch (IOException e) {
@@ -284,7 +297,6 @@ public class FileUtils {
         return numericPathOwnership;
     }
 
-
     // vagrant creates /tmp for us in windows so we use that to avoid long paths
     public static Path getTempDir() {
         return Paths.get("/tmp").toAbsolutePath();
@@ -295,6 +307,7 @@ public class FileUtils {
     }
 
     private static final Pattern VERSION_REGEX = Pattern.compile("(\\d+\\.\\d+\\.\\d+(-SNAPSHOT)?)");
+
     public static String getCurrentVersion() {
         // TODO: just load this once
         String distroFile = System.getProperty("tests.distribution");

qa/os/src/test/java/org/elasticsearch/packaging/util/Installation.java

@@ -28,9 +28,7 @@ import java.nio.file.Paths;
 public class Installation {
 
     // in the future we'll run as a role user on Windows
-    public static final String ARCHIVE_OWNER = Platforms.WINDOWS
+    public static final String ARCHIVE_OWNER = Platforms.WINDOWS ? System.getenv("username") : "elasticsearch";
-        ? System.getenv("username")
-        : "elasticsearch";
 
     private final Shell sh;
     public final Distribution distribution;
@@ -46,8 +44,18 @@ public class Installation {
     public final Path pidDir;
     public final Path envFile;
 
-    private Installation(Shell sh, Distribution distribution, Path home, Path config, Path data, Path logs,
+    private Installation(
-                         Path plugins, Path modules, Path pidDir, Path envFile) {
+        Shell sh,
+        Distribution distribution,
+        Path home,
+        Path config,
+        Path data,
+        Path logs,
+        Path plugins,
+        Path modules,
+        Path pidDir,
+        Path envFile
+    ) {
         this.sh = sh;
         this.distribution = distribution;
         this.home = home;
@@ -147,9 +155,7 @@ public class Installation {
         public final Path path;
 
         private Executable(String name) {
-            final String platformExecutableName = Platforms.WINDOWS
+            final String platformExecutableName = Platforms.WINDOWS ? name + ".bat" : name;
-                ? name + ".bat"
-                : name;
             this.path = bin(platformExecutableName);
         }
 

qa/os/src/test/java/org/elasticsearch/packaging/util/Packages.java

@@ -24,7 +24,6 @@ import org.apache.logging.log4j.Logger;
 import org.elasticsearch.packaging.util.Shell.Result;
 
 import java.io.IOException;
-import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
@@ -32,6 +31,7 @@ import java.nio.file.StandardOpenOption;
 import java.util.regex.Pattern;
 import java.util.stream.Stream;
 
+import static java.util.Collections.singletonList;
 import static org.elasticsearch.packaging.util.FileExistenceMatchers.fileDoesNotExist;
 import static org.elasticsearch.packaging.util.FileMatcher.Fileness.Directory;
 import static org.elasticsearch.packaging.util.FileMatcher.Fileness.File;
@@ -73,9 +73,10 @@ public class Packages {
         Platforms.onDPKG(() -> {
             assertThat(status.exitCode, anyOf(is(0), is(1)));
             if (status.exitCode == 0) {
-                assertTrue("an uninstalled status should be indicated: " + status.stdout,
+                assertTrue(
-                    Pattern.compile("(?m)^Status:.+deinstall ok").matcher(status.stdout).find() ||
+                    "an uninstalled status should be indicated: " + status.stdout,
-                    Pattern.compile("(?m)^Status:.+ok not-installed").matcher(status.stdout).find()
+                    Pattern.compile("(?m)^Status:.+deinstall ok").matcher(status.stdout).find()
+                        || Pattern.compile("(?m)^Status:.+ok not-installed").matcher(status.stdout).find()
                 );
             }
         });
@@ -108,8 +109,7 @@ public class Packages {
         Installation installation = Installation.ofPackage(sh, distribution);
 
         if (distribution.hasJdk == false) {
-            Files.write(installation.envFile, ("JAVA_HOME=" + systemJavaHome + "\n").getBytes(StandardCharsets.UTF_8),
+            Files.write(installation.envFile, singletonList("JAVA_HOME=" + systemJavaHome), StandardOpenOption.APPEND);
-                StandardOpenOption.APPEND);
         }
         return installation;
     }
@@ -124,9 +124,7 @@ public class Packages {
             if (r.exitCode != 0) {
                 Result lockOF = sh.runIgnoreExitCode("lsof /var/lib/dpkg/lock");
                 if (lockOF.exitCode == 0) {
-                    throw new RuntimeException(
+                    throw new RuntimeException("dpkg failed and the lockfile still exists. " + "Failure:\n" + r + "\nLockfile:\n" + lockOF);
-                            "dpkg failed and the lockfile still exists. "
-                            + "Failure:\n" + r + "\nLockfile:\n" + lockOF);
                 }
             }
             return r;
@@ -157,7 +155,6 @@ public class Packages {
         }
     }
 
-
     private static void verifyOssInstallation(Installation es, Distribution distribution, Shell sh) {
 
         sh.run("id elasticsearch");
@@ -167,16 +164,9 @@ public class Packages {
         final Path homeDir = Paths.get(passwdResult.stdout.trim().split(":")[5]);
         assertThat("elasticsearch user home directory must not exist", homeDir, fileDoesNotExist());
 
-        Stream.of(
+        Stream.of(es.home, es.plugins, es.modules).forEach(dir -> assertThat(dir, file(Directory, "root", "root", p755)));
-            es.home,
-            es.plugins,
-            es.modules
-        ).forEach(dir -> assertThat(dir, file(Directory, "root", "root", p755)));
 
-        Stream.of(
+        Stream.of(es.data, es.logs).forEach(dir -> assertThat(dir, file(Directory, "elasticsearch", "elasticsearch", p750)));
-            es.data,
-            es.logs
-        ).forEach(dir -> assertThat(dir, file(Directory, "elasticsearch", "elasticsearch", p750)));
 
         // we shell out here because java's posix file permission view doesn't support special modes
         assertThat(es.config, file(Directory, "root", "elasticsearch", p750));
@@ -186,33 +176,18 @@ public class Packages {
         assertThat(jvmOptionsDirectory, file(Directory, "root", "elasticsearch", p750));
         assertThat(sh.run("find \"" + jvmOptionsDirectory + "\" -maxdepth 0 -printf \"%m\"").stdout, containsString("2750"));
 
-        Stream.of(
+        Stream.of("elasticsearch.keystore", "elasticsearch.yml", "jvm.options", "log4j2.properties")
-            "elasticsearch.keystore",
+            .forEach(configFile -> assertThat(es.config(configFile), file(File, "root", "elasticsearch", p660)));
-            "elasticsearch.yml",
-            "jvm.options",
-            "log4j2.properties"
-        ).forEach(configFile -> assertThat(es.config(configFile), file(File, "root", "elasticsearch", p660)));
         assertThat(es.config(".elasticsearch.keystore.initial_md5sum"), file(File, "root", "elasticsearch", p644));
 
         assertThat(sh.run("sudo -u elasticsearch " + es.bin("elasticsearch-keystore") + " list").stdout, containsString("keystore.seed"));
 
-        Stream.of(
+        Stream.of(es.bin, es.lib).forEach(dir -> assertThat(dir, file(Directory, "root", "root", p755)));
-            es.bin,
-            es.lib
-        ).forEach(dir -> assertThat(dir, file(Directory, "root", "root", p755)));
 
-        Stream.of(
+        Stream.of("elasticsearch", "elasticsearch-plugin", "elasticsearch-keystore", "elasticsearch-shard", "elasticsearch-node")
-            "elasticsearch",
+            .forEach(executable -> assertThat(es.bin(executable), file(File, "root", "root", p755)));
-            "elasticsearch-plugin",
-            "elasticsearch-keystore",
-            "elasticsearch-shard",
-            "elasticsearch-node"
-        ).forEach(executable -> assertThat(es.bin(executable), file(File, "root", "root", p755)));
 
-        Stream.of(
+        Stream.of("NOTICE.txt", "README.asciidoc").forEach(doc -> assertThat(es.home.resolve(doc), file(File, "root", "root", p644)));
-            "NOTICE.txt",
-            "README.asciidoc"
-        ).forEach(doc -> assertThat(es.home.resolve(doc), file(File, "root", "root", p644)));
 
         assertThat(es.envFile, file(File, "root", "elasticsearch", p660));
 
@@ -231,9 +206,7 @@ public class Packages {
                 Paths.get("/usr/lib/sysctl.d/elasticsearch.conf")
             ).forEach(confFile -> assertThat(confFile, file(File, "root", "root", p644)));
 
-            final String sysctlExecutable = (distribution.packaging == Distribution.Packaging.RPM)
+            final String sysctlExecutable = (distribution.packaging == Distribution.Packaging.RPM) ? "/usr/sbin/sysctl" : "/sbin/sysctl";
-                ? "/usr/sbin/sysctl"
-                : "/sbin/sysctl";
             assertThat(sh.run(sysctlExecutable + " vm.max_map_count").stdout, containsString("vm.max_map_count = 262144"));
         }
 
@@ -263,13 +236,8 @@ public class Packages {
         // the version through here
         assertThat(es.bin("elasticsearch-sql-cli-" + getCurrentVersion() + ".jar"), file(File, "root", "root", p755));
 
-        Stream.of(
+        Stream.of("users", "users_roles", "roles.yml", "role_mapping.yml", "log4j2.properties")
-            "users",
+            .forEach(configFile -> assertThat(es.config(configFile), file(File, "root", "elasticsearch", p660)));
-            "users_roles",
-            "roles.yml",
-            "role_mapping.yml",
-            "log4j2.properties"
-        ).forEach(configFile -> assertThat(es.config(configFile), file(File, "root", "elasticsearch", p660)));
     }
 
     /**
@@ -337,8 +305,8 @@ public class Packages {
          * for Elasticsearch logs and storing it in class state.
          */
         public void clear() {
-            cursor = sh.run("sudo journalctl --unit=elasticsearch.service --lines=0 --show-cursor -o cat" +
+            final String script = "sudo journalctl --unit=elasticsearch.service --lines=0 --show-cursor -o cat | sed -e 's/-- cursor: //'";
-                " | sed -e 's/-- cursor: //'").stdout.trim();
+            cursor = sh.run(script).stdout.trim();
         }
 
         /**

qa/os/src/test/java/org/elasticsearch/packaging/util/ServerUtils.java

@@ -124,9 +124,7 @@ public class ServerUtils {
                 connectionManager.setDefaultMaxPerRoute(100);
                 connectionManager.setMaxTotal(200);
                 connectionManager.setValidateAfterInactivity(1000);
-                executor = Executor.newInstance(HttpClientBuilder.create()
+                executor = Executor.newInstance(HttpClientBuilder.create().setConnectionManager(connectionManager).build());
-                    .setConnectionManager(connectionManager)
-                    .build());
             }
         } else {
             executor = Executor.newInstance();
@@ -159,13 +157,8 @@ public class ServerUtils {
         throw new RuntimeException("Elasticsearch (with x-pack) did not start");
     }
 
-    public static void waitForElasticsearch(
+    public static void waitForElasticsearch(String status, String index, Installation installation, String username, String password)
-        String status,
+        throws Exception {
-        String index,
-        Installation installation,
-        String username,
-        String password
-    ) throws Exception {
 
         Objects.requireNonNull(status);
 
@@ -186,8 +179,7 @@ public class ServerUtils {
                 try {
 
                     final HttpResponse response = execute(
-                        Request
+                        Request.Get("http://localhost:9200/_cluster/health")
-                            .Get("http://localhost:9200/_cluster/health")
                             .connectTimeout((int) timeoutLength)
                             .socketTimeout((int) timeoutLength),
                         username,
@@ -239,11 +231,13 @@ public class ServerUtils {
     public static void runElasticsearchTests() throws Exception {
         makeRequest(
             Request.Post("http://localhost:9200/library/book/1?refresh=true&pretty")
-                .bodyString("{ \"title\": \"Book #1\", \"pages\": 123 }", ContentType.APPLICATION_JSON));
+                .bodyString("{ \"title\": \"Book #1\", \"pages\": 123 }", ContentType.APPLICATION_JSON)
+        );
 
         makeRequest(
             Request.Post("http://localhost:9200/library/book/2?refresh=true&pretty")
-                .bodyString("{ \"title\": \"Book #2\", \"pages\": 456 }", ContentType.APPLICATION_JSON));
+                .bodyString("{ \"title\": \"Book #2\", \"pages\": 456 }", ContentType.APPLICATION_JSON)
+        );
 
         String count = makeRequest(Request.Get("http://localhost:9200/_count?pretty"));
         assertThat(count, containsString("\"count\" : 2"));

qa/os/src/test/java/org/elasticsearch/packaging/util/Shell.java

@@ -42,7 +42,6 @@ import java.util.stream.Stream;
  */
 public class Shell {
 
-    public static final int TAIL_WHEN_TOO_MUCH_OUTPUT = 1000;
     public static final Result NO_OP = new Shell.Result(0, "", "");
     protected final Logger logger = LogManager.getLogger(getClass());
 
@@ -86,20 +85,28 @@ public class Shell {
 
     public void chown(Path path) throws Exception {
         Platforms.onLinux(() -> run("chown -R elasticsearch:elasticsearch " + path));
-        Platforms.onWindows(() -> run(
+        Platforms.onWindows(
-            "$account = New-Object System.Security.Principal.NTAccount '" + System.getenv("username")  + "'; " +
+            () -> run(
-                "$pathInfo = Get-Item '" + path + "'; " +
+                String.format(
-                "$toChown = @(); " +
+                    Locale.ROOT,
-                "if ($pathInfo.PSIsContainer) { " +
+                    "$account = New-Object System.Security.Principal.NTAccount '%s'; "
-                "  $toChown += Get-ChildItem '" + path + "' -Recurse; " +
+                        + "$pathInfo = Get-Item '%s'; "
-                "}" +
+                        + "$toChown = @(); "
-                "$toChown += $pathInfo; " +
+                        + "if ($pathInfo.PSIsContainer) { "
-                "$toChown | ForEach-Object { " +
+                        + "  $toChown += Get-ChildItem '%s' -Recurse; "
-                "$acl = Get-Acl $_.FullName; " +
+                        + "}"
-                "$acl.SetOwner($account); " +
+                        + "$toChown += $pathInfo; "
-                "Set-Acl $_.FullName $acl " +
+                        + "$toChown | ForEach-Object { "
-                "}"
+                        + "  $acl = Get-Acl $_.FullName; "
-        ));
+                        + "  $acl.SetOwner($account); "
+                        + "  Set-Acl $_.FullName $acl "
+                        + "}",
+                    System.getenv("username"),
+                    path,
+                    path
+                )
+            )
+        );
     }
 
     public void extractZip(Path zipPath, Path destinationDir) throws Exception {
@@ -165,22 +172,13 @@ public class Shell {
                 if (process.isAlive()) {
                     process.destroyForcibly();
                 }
-                Result result = new Result(
+                Result result = new Result(-1, readFileIfExists(stdOut), readFileIfExists(stdErr));
-                    -1,
-                    readFileIfExists(stdOut),
-                    readFileIfExists(stdErr)
-                );
                 throw new IllegalStateException(
-                    "Timed out running shell command: " + Arrays.toString(command) + "\n" +
+                    "Timed out running shell command: " + Arrays.toString(command) + "\n" + "Result:\n" + result
-                    "Result:\n" + result
                 );
             }
 
-            Result result = new Result(
+            Result result = new Result(process.exitValue(), readFileIfExists(stdOut), readFileIfExists(stdErr));
-                process.exitValue(),
-                readFileIfExists(stdOut),
-                readFileIfExists(stdErr)
-            );
             logger.info("Ran: {} {}", Arrays.toString(command), result);
             return result;
 
@@ -225,15 +223,7 @@ public class Shell {
     }
 
     public String toString() {
-        return new StringBuilder()
+        return String.format(Locale.ROOT, " env = [%s] workingDirectory = [%s]", env, workingDirectory);
-            .append(" ")
-            .append("env = [")
-            .append(env)
-            .append("]")
-            .append("workingDirectory = [")
-            .append(workingDirectory)
-            .append("]")
-            .toString();
     }
 
     public static class Result {
@@ -252,17 +242,7 @@ public class Shell {
         }
 
         public String toString() {
-            return new StringBuilder()
+            return String.format(Locale.ROOT, "exitCode = [%d] stdout = [%s] stderr = [%s]", exitCode, stdout.trim(), stderr.trim());
-                .append("exitCode = [")
-                .append(exitCode)
-                .append("] ")
-                .append("stdout = [")
-                .append(stdout.trim())
-                .append("] ")
-                .append("stderr = [")
-                .append(stderr.trim())
-                .append("]")
-                .toString();
         }
     }