Browse Source

Opt-in :qa:os to automatic formatting (#52750)

Add `:qa:os` 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.
Rory Hunter 5 years ago
parent
commit
214bbd25b1

+ 1 - 0
build.gradle

@@ -114,6 +114,7 @@ subprojects {
       ':distribution:tools:keystore-cli',
       ':distribution:tools:launchers',
       ':distribution:tools:plugin-cli',
+      ':qa:os',
       ':x-pack:plugin:autoscaling',
       ':x-pack:plugin:enrich'
     ]

+ 18 - 28
qa/os/src/test/java/org/elasticsearch/packaging/test/ArchiveTests.java

@@ -30,8 +30,11 @@ import org.junit.BeforeClass;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
+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;
@@ -108,7 +111,7 @@ public class ArchiveTests extends PackagingTestCase {
 
         try {
             startElasticsearch();
-        } catch (Exception e ){
+        } catch (Exception e) {
             if (Files.exists(installation.home.resolve("elasticsearch.pid"))) {
                 String pid = FileUtils.slurp(installation.home.resolve("elasticsearch.pid")).trim();
                 logger.info("Dumping jstack of elasticsearch processb ({}) that failed to start", pid);
@@ -138,8 +141,7 @@ public class ArchiveTests extends PackagingTestCase {
         stopElasticsearch();
 
         String systemJavaHome1 = sh.getEnv().get("JAVA_HOME");
-        assertThat(FileUtils.slurpAllLogs(installation.logs, "elasticsearch.log", "*.log.gz"),
-            containsString(systemJavaHome1));
+        assertThat(FileUtils.slurpAllLogs(installation.logs, "elasticsearch.log", "*.log.gz"), containsString(systemJavaHome1));
     }
 
     public void test52BundledJdkRemoved() throws Exception {
@@ -162,8 +164,7 @@ public class ArchiveTests extends PackagingTestCase {
             stopElasticsearch();
 
             String systemJavaHome1 = sh.getEnv().get("JAVA_HOME");
-            assertThat(FileUtils.slurpAllLogs(installation.logs, "elasticsearch.log", "*.log.gz"),
-                containsString(systemJavaHome1));
+            assertThat(FileUtils.slurpAllLogs(installation.logs, "elasticsearch.log", "*.log.gz"), containsString(systemJavaHome1));
         } finally {
             mv(relocatedJdk, installation.bundledJdk);
         }
@@ -178,7 +179,7 @@ public class ArchiveTests extends PackagingTestCase {
 
                 sh.getEnv().put("JAVA_HOME", "C:\\Program Files (x86)\\java");
 
-                //verify ES can start, stop and run plugin list
+                // verify ES can start, stop and run plugin list
                 startElasticsearch();
 
                 stopElasticsearch();
@@ -188,7 +189,7 @@ public class ArchiveTests extends PackagingTestCase {
                 assertThat(result.exitCode, equalTo(0));
 
             } finally {
-                //clean up sym link
+                // clean up sym link
                 if (Files.exists(Paths.get(javaPath))) {
                     sh.run("cmd /c rmdir '" + javaPath + "' ");
                 }
@@ -203,7 +204,7 @@ public class ArchiveTests extends PackagingTestCase {
                 sh.run("ln -s \"" + systemJavaHome + "\" \"" + testJavaHome + "\"");
                 sh.getEnv().put("JAVA_HOME", testJavaHome);
 
-                //verify ES can start, stop and run plugin list
+                // verify ES can start, stop and run plugin list
                 startElasticsearch();
 
                 stopElasticsearch();
@@ -229,11 +230,8 @@ 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 =
-                "-Xms512m\n" +
-                "-Xmx512m\n" +
-                "-Dlog4j2.disable.jmx=true\n";
-            append(tempConf.resolve("jvm.options"), jvmOptions);
+            final List<String> jvmOptions = List.of("-Xms512m", "-Xmx512m", "-Dlog4j2.disable.jmx=true");
+            Files.write(tempConf.resolve("jvm.options"), jvmOptions, CREATE, APPEND);
 
             sh.chown(tempConf);
 
@@ -316,11 +314,8 @@ public class ArchiveTests extends PackagingTestCase {
 
         try {
             mkdir(tempConf);
-            Stream.of(
-                "elasticsearch.yml",
-                "log4j2.properties",
-                "jvm.options"
-            ).forEach(file -> cp(installation.config(file), tempConf.resolve(file)));
+            Stream.of("elasticsearch.yml", "log4j2.properties", "jvm.options")
+                .forEach(file -> cp(installation.config(file), tempConf.resolve(file)));
 
             append(tempConf.resolve("elasticsearch.yml"), "node.name: relative");
 
@@ -381,8 +376,7 @@ public class ArchiveTests extends PackagingTestCase {
 
         Platforms.PlatformAction action = () -> {
             final Result result = sh.run(bin.nodeTool + " -h");
-            assertThat(result.stdout,
-                    containsString("A CLI tool to do unsafe cluster and index manipulations on current node"));
+            assertThat(result.stdout, containsString("A CLI tool to do unsafe cluster and index manipulations on current node"));
         };
 
         // TODO: this should be checked on all distributions
@@ -412,17 +406,13 @@ public class ArchiveTests extends PackagingTestCase {
 
         Platforms.PlatformAction action = () -> {
             Result result = sh.run(bin.certutilTool + " -h");
-            assertThat(result.stdout,
-                containsString("Simplifies certificate creation for use with the Elastic Stack"));
+            assertThat(result.stdout, containsString("Simplifies certificate creation for use with the Elastic Stack"));
             result = sh.run(bin.syskeygenTool + " -h");
-            assertThat(result.stdout,
-                containsString("system key tool"));
+            assertThat(result.stdout, containsString("system key tool"));
             result = sh.run(bin.setupPasswordsTool + " -h");
-            assertThat(result.stdout,
-                containsString("Sets the passwords for reserved users"));
+            assertThat(result.stdout, containsString("Sets the passwords for reserved users"));
             result = sh.run(bin.usersTool + " -h");
-            assertThat(result.stdout,
-                containsString("Manages elasticsearch file users"));
+            assertThat(result.stdout, containsString("Manages elasticsearch file users"));
         };
 
         // TODO: this should be checked on all distributions

+ 30 - 20
qa/os/src/test/java/org/elasticsearch/packaging/test/CertGenCliTests.java

@@ -30,13 +30,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;
@@ -61,17 +63,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(
-            "instances:",
-            "  - name: \"mynode\"",
-            "    ip:",
-            "      - \"127.0.0.1\""));
+        final List<String> lines = new ArrayList<>();
+        lines.add("instances:");
+        lines.add("  - name: \"mynode\"");
+        lines.add("    ip:");
+        lines.add("      - \"127.0.0.1\"");
+        Files.write(instancesFile, lines, CREATE, APPEND);
 
         installation.executables().certgenTool.run("--in " + instancesFile + " --out " + certificatesFile);
 
@@ -100,20 +103,27 @@ public class CertGenCliTests extends PackagingTestCase {
     public void test40RunWithCert() throws Exception {
         // 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"));
+        final String certPath = escapePath(installation.config("certs/mynode/mynode.crt"));
+        final String caCertPath = escapePath(installation.config("certs/ca/ca.crt"));
+
+        List<String> yaml = List.of(
             "node.name: mynode",
-            "xpack.security.transport.ssl.key: " + escapePath(installation.config("certs/mynode/mynode.key")),
-            "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.key: " + keyPath,
+            "xpack.security.transport.ssl.certificate: " + certPath,
+            "xpack.security.transport.ssl.certificate_authorities: [\"" + caCertPath + "\"]",
+            "xpack.security.http.ssl.key: " + keyPath,
+            "xpack.security.http.ssl.certificate: " + certPath,
+            "xpack.security.http.ssl.certificate_authorities: [\"" + caCertPath + "\"]",
             "xpack.security.transport.ssl.enabled: true",
-            "xpack.security.http.ssl.enabled: true"));
+            "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"));
-        });
+        assertWhileRunning(
+            () -> ServerUtils.makeRequest(Request.Get("https://127.0.0.1:9200"), null, null, installation.config("certs/ca/ca.crt"))
+        );
     }
 }

+ 2 - 9
qa/os/src/test/java/org/elasticsearch/packaging/test/DebPreservationTests.java

@@ -79,10 +79,7 @@ public class DebPreservationTests extends PackagingTestCase {
 
         // keystore was removed
 
-        assertPathsDoNotExist(
-            installation.config("elasticsearch.keystore"),
-            installation.config(".elasticsearch.keystore.initial_md5sum")
-        );
+        assertPathsDoNotExist(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(
-            installation.config,
-            installation.envFile,
-            SYSVINIT_SCRIPT
-        );
+        assertPathsDoNotExist(installation.config, installation.envFile, SYSVINIT_SCRIPT);
 
         assertThat(packageStatus(distribution()).exitCode, is(1));
     }

+ 27 - 44
qa/os/src/test/java/org/elasticsearch/packaging/test/KeystoreManagementTests.java

@@ -30,11 +30,10 @@ 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.StandardOpenOption;
 import java.util.Arrays;
+import java.util.List;
 import java.util.Map;
 
 import static org.elasticsearch.packaging.util.Archives.ARCHIVE_OWNER;
@@ -80,8 +79,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",
-            r.stderr, containsString(ERROR_KEYSTORE_NOT_FOUND));
+        assertThat("has-passwd should indicate missing keystore", r.stderr, containsString(ERROR_KEYSTORE_NOT_FOUND));
     }
 
     /** Test initial package state */
@@ -96,8 +94,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",
-            r.stderr, containsString(ERROR_KEYSTORE_NOT_PASSWORD_PROTECTED));
+        assertThat("has-passwd should indicate unprotected keystore", r.stderr, containsString(ERROR_KEYSTORE_NOT_PASSWORD_PROTECTED));
         Shell.Result r2 = bin.keystoreTool.run("list");
         assertThat(r2.stdout, containsString("keystore.seed"));
     }
@@ -117,8 +114,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",
-            r.stdout, containsString(ERROR_KEYSTORE_NOT_PASSWORD_PROTECTED));
+        assertThat("has-passwd should indicate unprotected keystore", r.stdout, containsString(ERROR_KEYSTORE_NOT_PASSWORD_PROTECTED));
         Shell.Result r2 = bin.keystoreTool.run("list");
         assertThat(r2.stdout, containsString("keystore.seed"));
     }
@@ -151,8 +147,7 @@ public class KeystoreManagementTests extends PackagingTestCase {
     }
 
     public void test40KeystorePasswordOnStandardInput() throws Exception {
-        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 = "^|<>\\&exit"; // code insertion on Windows if special characters are not escaped
@@ -169,8 +164,7 @@ public class KeystoreManagementTests extends PackagingTestCase {
     }
 
     public void test41WrongKeystorePasswordOnStandardInput() {
-        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();
@@ -181,10 +175,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",
-            distribution.platform != Distribution.Platform.WINDOWS);
-        assumeTrue("packages will use systemd, which doesn't handle stdin",
-            distribution.isArchive());
+        assumeTrue("expect command isn't on Windows", distribution.platform != Distribution.Platform.WINDOWS);
+        assumeTrue("packages will use systemd, which doesn't handle stdin", distribution.isArchive());
         assumeThat(installation, is(notNullValue()));
 
         String password = "keystorepass";
@@ -202,10 +194,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",
-            distribution.platform != Distribution.Platform.WINDOWS);
-        assumeTrue("packages will use systemd, which doesn't handle stdin",
-            distribution.isArchive());
+        assumeTrue("expect command isn't on Windows", distribution.platform != Distribution.Platform.WINDOWS);
+        assumeTrue("packages will use systemd, which doesn't handle stdin", distribution.isArchive());
         assumeThat(installation, is(notNullValue()));
 
         assertPasswordProtectedKeystore();
@@ -220,8 +210,7 @@ public class KeystoreManagementTests extends PackagingTestCase {
      * view help information.
      */
     public void test44EncryptedKeystoreAllowsHelpMessage() throws Exception {
-        assumeTrue("users call elasticsearch directly in archive case",
-            distribution.isArchive());
+        assumeTrue("users call elasticsearch directly in archive case", distribution.isArchive());
 
         String password = "keystorepass";
 
@@ -249,9 +238,7 @@ public class KeystoreManagementTests extends PackagingTestCase {
             sh.run("sudo systemctl set-environment ES_KEYSTORE_PASSPHRASE_FILE=" + esKeystorePassphraseFile);
 
             Files.createFile(esKeystorePassphraseFile);
-            Files.write(esKeystorePassphraseFile,
-                (password + System.lineSeparator()).getBytes(StandardCharsets.UTF_8),
-                StandardOpenOption.WRITE);
+            Files.write(esKeystorePassphraseFile, List.of(password));
 
             startElasticsearch();
             ServerUtils.runElasticsearchTests();
@@ -275,9 +262,7 @@ public class KeystoreManagementTests extends PackagingTestCase {
             }
 
             Files.createFile(esKeystorePassphraseFile);
-            Files.write(esKeystorePassphraseFile,
-                ("wrongpassword" + System.lineSeparator()).getBytes(StandardCharsets.UTF_8),
-                StandardOpenOption.WRITE);
+            Files.write(esKeystorePassphraseFile, List.of("wrongpassword"));
 
             Packages.JournaldWrapper journaldWrapper = new Packages.JournaldWrapper(sh);
             Shell.Result result = runElasticsearchStartCommand();
@@ -334,8 +319,7 @@ public class KeystoreManagementTests extends PackagingTestCase {
 
             waitForElasticsearch(installation);
             ServerUtils.runElasticsearchTests();
-        }
-        finally {
+        } finally {
             if (tempDir != null) {
                 rm(tempDir);
             }
@@ -376,9 +360,13 @@ 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
-            + "\n\" | " + installation.executables().keystoreTool.toString() + " passwd";
-        Files.writeString(tempDirectory.resolve("set-pass.sh"), setPasswordScript);
+        List<String> setPasswordScript = List.of(
+            "echo \"" + password,
+            password,
+            "\" | " + installation.executables().keystoreTool.toString() + " passwd"
+        );
+
+        Files.write(tempDirectory.resolve("set-pass.sh"), setPasswordScript);
 
         runContainer(distribution(), volumes, null);
         try {
@@ -409,9 +397,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(() -> {
-            sh.chown(keystore);
-        });
+        Platforms.onWindows(() -> sh.chown(keystore));
 
         if (distribution().isDocker()) {
             try {
@@ -444,14 +430,11 @@ public class KeystoreManagementTests extends PackagingTestCase {
         final Installation.Executables bin = installation.executables();
 
         // set the password by passing it to stdin twice
-        Platforms.onLinux(() -> {
-            bin.keystoreTool.run("passwd", password + "\n" + password + "\n");
-        });
-
-        Platforms.onWindows(() -> {
-            sh.run("Invoke-Command -ScriptBlock {echo \'" + password + "\'; echo \'" + password + "\'} | "
-                + bin.keystoreTool + " passwd");
-        });
+        Platforms.onLinux(() -> bin.keystoreTool.run("passwd", password + "\n" + password + "\n"));
+
+        Platforms.onWindows(
+            () -> sh.run("Invoke-Command -ScriptBlock {echo '" + password + "'; echo '" + password + "'} | " + bin.keystoreTool + " passwd")
+        );
     }
 
     private void assertPasswordProtectedKeystore() {

+ 8 - 13
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.List;
 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 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),
-                StandardOpenOption.APPEND);
+            Files.write(installation.envFile, List.of("JAVA_HOME=" + systemJavaHome), 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"),
-            containsString(systemJavaHome));
+        assertThat(FileUtils.slurpAllLogs(installation.logs, "elasticsearch.log", "elasticsearch*.log.gz"), containsString(systemJavaHome));
     }
 
     public void test32JavaHomeOverride() throws Exception {
@@ -170,8 +168,9 @@ public class PackageTests extends PackagingTestCase {
         String start = sh.runIgnoreExitCode("date ").stdout.trim();
         startElasticsearch();
 
-        String journalEntries = sh.runIgnoreExitCode("journalctl _SYSTEMD_UNIT=elasticsearch.service " +
-            "--since \"" + start + "\" --output cat | wc -l").stdout.trim();
+        String journalEntries = sh.runIgnoreExitCode(
+            "journalctl _SYSTEMD_UNIT=elasticsearch.service " + "--since \"" + start + "\" --output cat | wc -l"
+        ).stdout.trim();
         assertThat(journalEntries, equalTo("0"));
 
         assertPathsExist(installation.pidDir.resolve("elasticsearch.pid"));
@@ -211,9 +210,7 @@ public class PackageTests extends PackagingTestCase {
                 matcher.find();
                 final int version = Integer.parseInt(matcher.group(1));
 
-                statusExitCode = version < 231
-                    ? 3
-                    : 4;
+                statusExitCode = version < 231 ? 3 : 4;
             }
 
             assertThat(sh.runIgnoreExitCode("systemctl status elasticsearch.service").exitCode, is(statusExitCode));
@@ -256,7 +253,6 @@ public class PackageTests extends PackagingTestCase {
         }
     }
 
-
     public void test72TestRuntimeDirectory() throws Exception {
         try {
             install();
@@ -279,7 +275,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

+ 12 - 19
qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java

@@ -67,14 +67,12 @@ import static org.junit.Assume.assumeTrue;
  * Class that all packaging test cases should inherit from
  */
 @RunWith(RandomizedRunner.class)
-@TestMethodProviders({
-    JUnit3MethodProvider.class
-})
+@TestMethodProviders({ JUnit3MethodProvider.class })
 @Timeout(millis = 20 * 60 * 1000) // 20 min
 @TestCaseOrdering(TestCaseOrdering.AlphabeticOrder.class)
 public abstract class PackagingTestCase extends Assert {
 
-    protected final Logger logger =  LogManager.getLogger(getClass());
+    protected final Logger logger = LogManager.getLogger(getClass());
 
     // the distribution being tested
     protected static final Distribution distribution;
@@ -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(() -> {
-                sh.getEnv().put("JAVA_HOME", systemJavaHome);
-            });
-            Platforms.onWindows(() -> {
-                sh.getEnv().put("JAVA_HOME", systemJavaHome);
-            });
+            Platforms.onLinux(() -> sh.getEnv().put("JAVA_HOME", systemJavaHome));
+            Platforms.onWindows(() -> sh.getEnv().put("JAVA_HOME", systemJavaHome));
         }
     }
 
@@ -208,15 +201,14 @@ public abstract class PackagingTestCase extends Assert {
     protected void assertWhileRunning(Platforms.PlatformAction assertions) throws Exception {
         try {
             awaitElasticsearchStartup(runElasticsearchStartCommand());
-        } catch (Exception e ){
+        } catch (Exception e) {
             if (Files.exists(installation.home.resolve("elasticsearch.pid"))) {
                 String pid = FileUtils.slurp(installation.home.resolve("elasticsearch.pid")).trim();
                 logger.info("Dumping jstack of elasticsearch processb ({}) that failed to start", pid);
                 sh.runIgnoreExitCode("jstack " + pid);
             }
             if (Files.exists(installation.logs.resolve("elasticsearch.log"))) {
-                logger.warn("Elasticsearch log:\n" +
-                    FileUtils.slurpAllLogs(installation.logs, "elasticsearch.log", "*.log.gz"));
+                logger.warn("Elasticsearch log:\n" + 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" +
-                FileUtils.slurpAllLogs(installation.logs, "elasticsearch.log", "*.log.gz"));
+            logger.warn("Elasticsearch log:\n" + 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 | " +
-                "where {($_.EventName -eq 'OutputDataReceived' -Or $_.EventName -eq 'ErrorDataReceived' |" +
-                "Unregister-EventSubscriber -Force");
+            sh.runIgnoreExitCode(
+                "Get-EventSubscriber | "
+                    + "where {($_.EventName -eq 'OutputDataReceived' -Or $_.EventName -eq 'ErrorDataReceived' |"
+                    + "Unregister-EventSubscriber -Force"
+            );
             assertThat(FileUtils.slurp(Archives.getPowershellErrorPath(installation)), anyOf(stringMatchers));
 
         } else {

+ 21 - 7
qa/os/src/test/java/org/elasticsearch/packaging/test/PasswordToolsTests.java

@@ -28,13 +28,14 @@ import org.junit.Before;
 
 import java.nio.file.Files;
 import java.nio.file.Path;
+import java.nio.file.StandardOpenOption;
 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 +53,11 @@ public class PasswordToolsTests extends PackagingTestCase {
 
     public void test010Install() throws Exception {
         install();
-        append(installation.config("elasticsearch.yml"),
-            "xpack.license.self_generated.type: trial\n" +
-            "xpack.security.enabled: true");
+        Files.write(
+            installation.config("elasticsearch.yml"),
+            List.of("xpack.license.self_generated.type: trial", "xpack.security.enabled: true"),
+            StandardOpenOption.APPEND
+        );
     }
 
     public void test20GeneratePasswords() throws Exception {
@@ -63,7 +66,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 +119,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 +135,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"));
             }
         });

+ 5 - 23
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(
-            "elasticsearch.yml",
-            "jvm.options",
-            "log4j2.properties"
-        )
+        Stream.of("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(
-                "role_mapping.yml",
-                "roles.yml",
-                "users",
-                "users_roles"
-            )
+            Stream.of("role_mapping.yml", "roles.yml", "users", "users_roles")
                 .map(each -> installation.config(each))
                 .forEach(path -> append(path, "# foo"));
         }
@@ -119,27 +110,18 @@ public class RpmPreservationTests extends PackagingTestCase {
         assertThat(installation.config, fileExists());
         assertThat(installation.config("elasticsearch.keystore"), fileExists());
 
-        Stream.of(
-            "elasticsearch.yml",
-            "jvm.options",
-            "log4j2.properties"
-        ).forEach(this::assertConfFilePreserved);
+        Stream.of("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(
-                "role_mapping.yml",
-                "roles.yml",
-                "users",
-                "users_roles"
-            ).forEach(this::assertConfFilePreserved);
+            Stream.of("role_mapping.yml", "roles.yml", "users", "users_roles").forEach(this::assertConfFilePreserved);
         }
     }
 
     private void assertConfFilePreserved(String configFile) {
         final Path original = installation.config(configFile);
         final Path saved = installation.config(configFile + ".rpmsave");
-        assertConfFilePreserved(original ,saved);
+        assertConfFilePreserved(original, saved);
     }
 
     private void assertConfFilePreserved(final Path original, final Path saved) {

+ 37 - 29
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\"; " +
-                "Write-Output $files; " +
-                "foreach ($file in $files) {" +
-                    "Write-Output \"$file\"; " +
-                    "Get-Content \"$file\" " +
-                "}");
+            Result logs = sh.run(
+                "$files = Get-ChildItem \""
+                    + installation.logs
+                    + "\\elasticsearch.log\"; "
+                    + "Write-Output $files; "
+                    + "foreach ($file in $files) {"
+                    + "    Write-Output \"$file\"; "
+                    + "    Get-Content \"$file\" "
+                    + "}"
+            );
             logger.error(logs.stdout);
             fail();
         } else {
@@ -105,7 +109,7 @@ public class WindowsServiceTests extends PackagingTestCase {
         Path serviceExe = installation.bin("elasticsearch-service-x64.exe");
         Path tmpServiceExe = serviceExe.getParent().resolve(serviceExe.getFileName() + ".tmp");
         Files.move(serviceExe, tmpServiceExe);
-        Result result =  sh.runIgnoreExitCode(serviceScript + " install");
+        Result result = sh.runIgnoreExitCode(serviceScript + " install");
         assertThat(result.exitCode, equalTo(1));
         assertThat(result.stdout, containsString("elasticsearch-service-x64.exe was not found..."));
         Files.move(tmpServiceExe, serviceExe);
@@ -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;" +
-            "$i = 0;" +
-            "do {" +
-              "$p = Get-Process -Name \"elasticsearch-service-x64\" -ErrorAction SilentlyContinue;" +
-              "echo \"$p\";" +
-              "if ($p -eq $Null) {" +
-              "  Write-Host \"exited after $i seconds\";" +
-              "  exit 0;" +
-              "}" +
-              "Start-Sleep -Seconds 1;" +
-              "$i += 1;" +
-            "} while ($i -lt 300);" +
-            "exit 9;");
+        assertCommand(
+            "$p = Get-Service -Name \"elasticsearch-service-x64\" -ErrorAction SilentlyContinue;"
+                + "$i = 0;"
+                + "do {"
+                + "  $p = Get-Process -Name \"elasticsearch-service-x64\" -ErrorAction SilentlyContinue;"
+                + "  echo \"$p\";"
+                + "  if ($p -eq $Null) {"
+                + "    Write-Host \"exited after $i seconds\";"
+                + "    exit 0;"
+                + "  }"
+                + "  Start-Sleep -Seconds 1;"
+                + "  $i += 1;"
+                + "} while ($i -lt 300);"
+                + "exit 9;"
+        );
 
         assertCommand(serviceScript + " remove");
-        assertCommand("$p = Get-Service -Name \"elasticsearch-service-x64\" -ErrorAction SilentlyContinue;" +
-            "echo \"$p\";" +
-            "if ($p -eq $Null) {" +
-            "  exit 0;" +
-            "} else {" +
-            "  exit 1;" +
-            "}");
+        assertCommand(
+            "$p = Get-Service -Name \"elasticsearch-service-x64\" -ErrorAction SilentlyContinue;"
+                + "echo \"$p\";"
+                + "if ($p -eq $Null) {"
+                + "  exit 0;"
+                + "} else {"
+                + "  exit 1;"
+                + "}"
+        );
     }
 
     public void test30StartStop() throws Exception {
@@ -231,12 +239,12 @@ public class WindowsServiceTests extends PackagingTestCase {
         Path fakeServiceMgr = serviceMgr.getParent().resolve("elasticsearch-service-mgr.bat");
         Files.write(fakeServiceMgr, Arrays.asList("echo \"Fake Service Manager GUI\""));
         Shell sh = new Shell();
-        Result result =  sh.run(serviceScript + " manager");
+        Result result = sh.run(serviceScript + " manager");
         assertThat(result.stdout, containsString("Fake Service Manager GUI"));
 
         // check failure too
         Files.write(fakeServiceMgr, Arrays.asList("echo \"Fake Service Manager GUI Failure\"", "exit 1"));
-        result =  sh.runIgnoreExitCode(serviceScript + " manager");
+        result = sh.runIgnoreExitCode(serviceScript + " manager");
         TestCase.assertEquals(1, result.exitCode);
         TestCase.assertTrue(result.stdout, result.stdout.contains("Fake Service Manager GUI Failure"));
         Files.move(tmpServiceMgr, serviceMgr);

+ 103 - 93
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;
@@ -56,12 +57,10 @@ import static org.hamcrest.core.IsNot.not;
  */
 public class Archives {
 
-    protected static final Logger logger =  LogManager.getLogger(Archives.class);
+    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
-        ? System.getenv("username")
-        : "elasticsearch";
+    public static final String ARCHIVE_OWNER = Platforms.WINDOWS ? 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 =
-                "Add-Type -AssemblyName 'System.IO.Compression.Filesystem'; " +
-                "[IO.Compression.ZipFile]::ExtractToDirectory('" + distributionFile + "', '" + baseInstallPath + "')";
+            installCommand = String.format(
+                Locale.ROOT,
+                "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 " +
-                    "--quiet " +
-                    "--system " +
-                    "--no-create-home " +
-                    "--ingroup elasticsearch " +
-                    "--disabled-password " +
-                    "--shell /bin/false " +
-                    "elasticsearch");
+                sh.run(
+                    "adduser "
+                        + "--quiet "
+                        + "--system "
+                        + "--no-create-home "
+                        + "--ingroup elasticsearch "
+                        + "--disabled-password "
+                        + "--shell /bin/false "
+                        + "elasticsearch"
+                );
             } else {
-                sh.run("useradd " +
-                    "--system " +
-                    "-M " +
-                    "--gid elasticsearch " +
-                    "--shell /sbin/nologin " +
-                    "--comment 'elasticsearch user' " +
-                    "elasticsearch");
+                sh.run(
+                    "useradd "
+                        + "--system "
+                        + "-M "
+                        + "--gid elasticsearch "
+                        + "--shell /sbin/nologin "
+                        + "--comment 'elasticsearch user' "
+                        + "elasticsearch"
+                );
             }
         }
     }
@@ -157,13 +163,7 @@ public class Archives {
     }
 
     private static void verifyOssInstallation(Installation es, Distribution distribution, String owner) {
-        Stream.of(
-            es.home,
-            es.config,
-            es.plugins,
-            es.modules,
-            es.logs
-        ).forEach(dir -> assertThat(dir, file(Directory, owner, owner, p755)));
+        Stream.of(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(
-                "elasticsearch-service.bat",
-                "elasticsearch-service-mgr.exe",
-                "elasticsearch-service-x64.exe"
-            ).forEach(executable -> assertThat(es.bin(executable), file(File, owner)));
+            Stream.of("elasticsearch-service.bat", "elasticsearch-service-mgr.exe", "elasticsearch-service-x64.exe")
+                .forEach(executable -> assertThat(es.bin(executable), file(File, owner)));
         }
 
-        Stream.of(
-            "elasticsearch.yml",
-            "jvm.options",
-            "log4j2.properties"
-        ).forEach(configFile -> assertThat(es.config(configFile), file(File, owner, owner, p660)));
+        Stream.of("elasticsearch.yml", "jvm.options", "log4j2.properties")
+            .forEach(configFile -> assertThat(es.config(configFile), file(File, owner, owner, p660)));
 
-        Stream.of(
-            "NOTICE.txt",
-            "LICENSE.txt",
-            "README.asciidoc"
-        ).forEach(doc -> assertThat(es.home.resolve(doc), file(File, owner, owner, p644)));
+        Stream.of("NOTICE.txt", "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) {
@@ -235,13 +226,8 @@ public class Archives {
         // the version through here
         assertThat(es.bin("elasticsearch-sql-cli-" + getCurrentVersion() + ".jar"), file(File, owner, owner, p755));
 
-        Stream.of(
-            "users",
-            "users_roles",
-            "roles.yml",
-            "role_mapping.yml",
-            "log4j2.properties"
-        ).forEach(configFile -> assertThat(es.config(configFile), file(File, owner, owner, p660)));
+        Stream.of("users", "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) {
@@ -253,13 +239,20 @@ public class Archives {
         final Installation.Executables bin = installation.executables();
 
         // requires the "expect" utility to be installed
-        String script = "expect -c \"$(cat<<EXPECT\n" +
-            "spawn -ignore HUP sudo -E -u " + ARCHIVE_OWNER + " " + bin.elasticsearch + " -d -p " + pidFile + "\n" +
-            "expect \"Elasticsearch keystore password:\"\n" +
-            "send \"" + keystorePassword + "\\r\"\n" +
-            "expect eof\n" +
-            "EXPECT\n" +
-            ")\"";
+        String script = String.format(
+            Locale.ROOT,
+            "expect -c \"$(cat<<EXPECT\n"
+                + "spawn -ignore HUP sudo -E -u %s %s -d -p %s \n"
+                + "expect \"Elasticsearch keystore password:\"\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);
@@ -282,8 +275,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 +
-                " <<<'" + keystorePassword + "'");
+            return sh.runIgnoreExitCode(
+                "sudo -E -u " + ARCHIVE_OWNER + " " + bin.elasticsearch + " -d -p " + pidFile + " <<<'" + keystorePassword + "'"
+            );
         }
         final Path stdout = getPowershellOutputPath(installation);
         final Path stderr = getPowershellErrorPath(installation);
@@ -293,45 +287,59 @@ 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 =
-                "$password = ConvertTo-SecureString 'vagrant' -AsPlainText -Force; " +
-                "$processInfo.Username = 'vagrant'; " +
-                "$processInfo.Password = $password; ";
+            powerShellProcessUserSetup = "$password = ConvertTo-SecureString 'vagrant' -AsPlainText -Force; "
+                + "$processInfo.Username = 'vagrant'; "
+                + "$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.FileName = '" + bin.elasticsearch + "'; " +
-                "$processInfo.Arguments = '-p " + installation.home.resolve("elasticsearch.pid") + "'; " +
-                powerShellProcessUserSetup +
-                "$processInfo.RedirectStandardOutput = $true; " +
-                "$processInfo.RedirectStandardError = $true; " +
-                "$processInfo.RedirectStandardInput = $true; " +
-                sh.env.entrySet().stream()
+            "$processInfo = New-Object System.Diagnostics.ProcessStartInfo; "
+                + "$processInfo.FileName = '"
+                + bin.elasticsearch
+                + "'; "
+                + "$processInfo.Arguments = '-p "
+                + installation.home.resolve("elasticsearch.pid")
+                + "'; "
+                + 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()) +
-                "$processInfo.UseShellExecute = $false; " +
-                "$process = New-Object System.Diagnostics.Process; " +
-                "$process.StartInfo = $processInfo; " +
+                    .collect(joining())
+                + "$processInfo.UseShellExecute = $false; "
+                + "$process = New-Object System.Diagnostics.Process; "
+                + "$process.StartInfo = $processInfo; "
+                +
 
                 // set up some asynchronous output handlers
-                "$outScript = { $EventArgs.Data | Out-File -Encoding UTF8 -Append '" + stdout + "' }; " +
-                "$errScript = { $EventArgs.Data | Out-File -Encoding UTF8 -Append '" + stderr + "' }; " +
-                "$stdOutEvent = Register-ObjectEvent -InputObject $process " +
-                "-Action $outScript -EventName 'OutputDataReceived'; " +
-                "$stdErrEvent = Register-ObjectEvent -InputObject $process " +
-                "-Action $errScript -EventName 'ErrorDataReceived'; " +
-
-                "$process.Start() | Out-Null; " +
-                "$process.BeginOutputReadLine(); " +
-                "$process.BeginErrorReadLine(); " +
-                "$process.StandardInput.WriteLine('" + keystorePassword + "'); " +
-                "Wait-Process -Timeout " + ES_STARTUP_SLEEP_TIME_SECONDS + " -Id $process.Id; " +
-                "$process.Id;"
-            );
+                "$outScript = { $EventArgs.Data | Out-File -Encoding UTF8 -Append '"
+                + stdout
+                + "' }; "
+                + "$errScript = { $EventArgs.Data | Out-File -Encoding UTF8 -Append '"
+                + stderr
+                + "' }; "
+                + "$stdOutEvent = Register-ObjectEvent -InputObject $process "
+                + "-Action $outScript -EventName 'OutputDataReceived'; "
+                + "$stdErrEvent = Register-ObjectEvent -InputObject $process "
+                + "-Action $errScript -EventName 'ErrorDataReceived'; "
+                +
+
+                "$process.Start() | Out-Null; "
+                + "$process.BeginOutputReadLine(); "
+                + "$process.BeginErrorReadLine(); "
+                + "$process.StandardInput.WriteLine('"
+                + keystorePassword
+                + "'); "
+                + "Wait-Process -Timeout "
+                + ES_STARTUP_SLEEP_TIME_SECONDS
+                + " -Id $process.Id; "
+                + "$process.Id;"
+        );
     }
 
     public static void assertElasticsearchStarted(Installation installation) throws Exception {
@@ -355,9 +363,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 | " +
-                "where {($_.EventName -eq 'OutputDataReceived' -Or $_.EventName -eq 'ErrorDataReceived' |" +
-                "Unregister-EventSubscriber -Force");
+            sh.runIgnoreExitCode(
+                "Get-EventSubscriber | "
+                    + "where {($_.EventName -eq 'OutputDataReceived' -Or $_.EventName -eq 'ErrorDataReceived' |"
+                    + "Unregister-EventSubscriber -Force"
+            );
         });
         if (Files.exists(pidFile)) {
             Files.delete(pidFile);

+ 11 - 12
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(() -> {
-            // 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 | " +
-                "Where-Object { $_.CommandLine -Match 'org.elasticsearch.bootstrap.Elasticsearch' } | " +
-                "ForEach-Object { $_.Terminate() }"
-            );
-        });
+        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 | "
+                        + "Where-Object { $_.CommandLine -Match 'org.elasticsearch.bootstrap.Elasticsearch' } | "
+                        + "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()
-            .map(Paths::get)
-            .filter(Files::exists)
-            .forEach(rm);
+        filesToDelete.stream().map(Paths::get).filter(Files::exists).forEach(rm);
 
         // disable elasticsearch service
         // todo add this for windows when adding tests for service intallation

+ 12 - 5
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)
-            .appendText(" with owner ").appendValue(owner)
-            .appendText(" with group ").appendValue(group)
-            .appendText(" with posix permissions ").appendValueList("[", ",", "]", posixPermissions);
+        description.appendValue("file/directory: ")
+            .appendValue(fileness)
+            .appendText(" with owner ")
+            .appendValue(owner)
+            .appendText(" with group ")
+            .appendValue(group)
+            .appendText(" with posix permissions ")
+            .appendValueList("[", ",", "]", posixPermissions);
     }
 
     public static FileMatcher file(Fileness fileness, String owner) {

+ 33 - 20
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;
@@ -111,22 +112,21 @@ public class FileUtils {
 
     public static Path mktempDir(Path path) {
         try {
-            return Files.createTempDirectory(path,"tmp");
+            return Files.createTempDirectory(path, "tmp");
         } catch (IOException e) {
             throw new RuntimeException(e);
         }
     }
 
-
     public static Path mkdir(Path path) {
         try {
             return Files.createDirectories(path);
-         } catch (IOException e) {
+        } catch (IOException e) {
             throw new RuntimeException(e);
-         }
-     }
+        }
+    }
 
-     public static Path cp(Path source, Path target) {
+    public static Path cp(Path source, Path target) {
         try {
             return Files.copy(source, target);
         } catch (IOException e) {
@@ -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,
-            StandardOpenOption.CREATE, StandardOpenOption.APPEND)) {
+        try (
+            BufferedWriter writer = Files.newBufferedWriter(
+                file,
+                StandardCharsets.UTF_8,
+                StandardOpenOption.CREATE,
+                StandardOpenOption.APPEND
+            )
+        ) {
 
             writer.write(text);
         } catch (IOException e) {
@@ -204,7 +217,7 @@ public class FileUtils {
             for (Path rotatedLogFile : FileUtils.lsGlob(logPath, rotatedLogFilesGlob)) {
                 logFileJoiner.add(FileUtils.slurpTxtorGz(rotatedLogFile));
             }
-            return(logFileJoiner.toString());
+            return (logFileJoiner.toString());
         } catch (IOException e) {
             throw new RuntimeException(e);
         }
@@ -221,14 +234,14 @@ public class FileUtils {
                 // gc logs are verbose and not useful in this context
                 .filter(file -> file.getFileName().toString().startsWith("gc.log") == false)
                 .forEach(file -> {
-                logger.info("=== Contents of `{}` ({}) ===", file, file.toAbsolutePath());
-                try (Stream<String> stream = Files.lines(file)) {
-                    stream.forEach(logger::info);
-                } catch (IOException e) {
-                    logger.error("Can't show contents", e);
-                }
-                logger.info("=== End of contents of `{}`===", file);
-            });
+                    logger.info("=== Contents of `{}` ({}) ===", file, file.toAbsolutePath());
+                    try (Stream<String> stream = Files.lines(file)) {
+                        stream.forEach(logger::info);
+                    } catch (IOException e) {
+                        logger.error("Can't show contents", e);
+                    }
+                    logger.info("=== End of contents of `{}`===", file);
+                });
         } catch (IOException e) {
             logger.error("Can't list log files", 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");
@@ -314,12 +327,12 @@ public class FileUtils {
     }
 
     public static Matcher<Path> fileWithGlobExist(String glob) throws IOException {
-        return new FeatureMatcher<Path,Iterable<Path>>(not(emptyIterable()),"File with pattern exist", "file with pattern"){
+        return new FeatureMatcher<Path, Iterable<Path>>(not(emptyIterable()), "File with pattern exist", "file with pattern") {
 
             @Override
             protected Iterable<Path> featureValueOf(Path actual) {
                 try {
-                    return Files.newDirectoryStream(actual,glob);
+                    return Files.newDirectoryStream(actual, glob);
                 } catch (IOException e) {
                     return Collections.emptyList();
                 }

+ 14 - 8
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
-        ? System.getenv("username")
-        : "elasticsearch";
+    public static final String ARCHIVE_OWNER = Platforms.WINDOWS ? 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,
-                         Path plugins, Path modules, Path pidDir, Path envFile) {
+    private Installation(
+        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
-                ? name + ".bat"
-                : name;
+            final String platformExecutableName = Platforms.WINDOWS ? name + ".bat" : name;
             this.path = bin(platformExecutableName);
         }
 

+ 21 - 53
qa/os/src/test/java/org/elasticsearch/packaging/util/Packages.java

@@ -24,11 +24,11 @@ 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;
 import java.nio.file.StandardOpenOption;
+import java.util.List;
 import java.util.regex.Pattern;
 import java.util.stream.Stream;
 
@@ -53,7 +53,7 @@ import static org.junit.Assert.assertTrue;
 
 public class Packages {
 
-    private static final Logger logger =  LogManager.getLogger(Packages.class);
+    private static final Logger logger = LogManager.getLogger(Packages.class);
 
     public static final Path SYSVINIT_SCRIPT = Paths.get("/etc/init.d/elasticsearch");
     public static final Path SYSTEMD_SERVICE = Paths.get("/usr/lib/systemd/system/elasticsearch.service");
@@ -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,
-                    Pattern.compile("(?m)^Status:.+deinstall ok").matcher(status.stdout).find() ||
-                    Pattern.compile("(?m)^Status:.+ok not-installed").matcher(status.stdout).find()
+                assertTrue(
+                    "an uninstalled status should be indicated: " + status.stdout,
+                    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),
-                StandardOpenOption.APPEND);
+            Files.write(installation.envFile, List.of("JAVA_HOME=" + systemJavaHome), 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(
-                            "dpkg failed and the lockfile still exists. "
-                            + "Failure:\n" + r + "\nLockfile:\n" + lockOF);
+                    throw new RuntimeException("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(
-            es.home,
-            es.plugins,
-            es.modules
-        ).forEach(dir -> assertThat(dir, file(Directory, "root", "root", p755)));
+        Stream.of(es.home, es.plugins, es.modules).forEach(dir -> assertThat(dir, file(Directory, "root", "root", p755)));
 
-        Stream.of(
-            es.data,
-            es.logs
-        ).forEach(dir -> assertThat(dir, file(Directory, "elasticsearch", "elasticsearch", p750)));
+        Stream.of(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(
-            "elasticsearch.keystore",
-            "elasticsearch.yml",
-            "jvm.options",
-            "log4j2.properties"
-        ).forEach(configFile -> assertThat(es.config(configFile), file(File, "root", "elasticsearch", p660)));
+        Stream.of("elasticsearch.keystore", "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(
-            es.bin,
-            es.lib
-        ).forEach(dir -> assertThat(dir, file(Directory, "root", "root", p755)));
+        Stream.of(es.bin, es.lib).forEach(dir -> assertThat(dir, file(Directory, "root", "root", p755)));
 
-        Stream.of(
-            "elasticsearch",
-            "elasticsearch-plugin",
-            "elasticsearch-keystore",
-            "elasticsearch-shard",
-            "elasticsearch-node"
-        ).forEach(executable -> assertThat(es.bin(executable), file(File, "root", "root", p755)));
+        Stream.of("elasticsearch", "elasticsearch-plugin", "elasticsearch-keystore", "elasticsearch-shard", "elasticsearch-node")
+            .forEach(executable -> assertThat(es.bin(executable), file(File, "root", "root", p755)));
 
-        Stream.of(
-            "NOTICE.txt",
-            "README.asciidoc"
-        ).forEach(doc -> assertThat(es.home.resolve(doc), file(File, "root", "root", p644)));
+        Stream.of("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)
-                ? "/usr/sbin/sysctl"
-                : "/sbin/sysctl";
+            final String sysctlExecutable = (distribution.packaging == Distribution.Packaging.RPM) ? "/usr/sbin/sysctl" : "/sbin/sysctl";
             assertThat(sh.run(sysctlExecutable + " vm.max_map_count").stdout, containsString("vm.max_map_count = 262144"));
         }
 
@@ -262,13 +235,8 @@ public class Packages {
         // the version through here
         assertThat(es.bin("elasticsearch-sql-cli-" + getCurrentVersion() + ".jar"), file(File, "root", "root", p755));
 
-        Stream.of(
-            "users",
-            "users_roles",
-            "roles.yml",
-            "role_mapping.yml",
-            "log4j2.properties"
-        ).forEach(configFile -> assertThat(es.config(configFile), file(File, "root", "elasticsearch", p660)));
+        Stream.of("users", "users_roles", "roles.yml", "role_mapping.yml", "log4j2.properties")
+            .forEach(configFile -> assertThat(es.config(configFile), file(File, "root", "elasticsearch", p660)));
     }
 
     /**
@@ -336,8 +304,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" +
-                " | sed -e 's/-- cursor: //'").stdout.trim();
+            final String script = "sudo journalctl --unit=elasticsearch.service --lines=0 --show-cursor -o cat | sed -e 's/-- cursor: //'";
+            cursor = sh.run(script).stdout.trim();
         }
 
         /**

+ 10 - 16
qa/os/src/test/java/org/elasticsearch/packaging/util/ServerUtils.java

@@ -57,12 +57,12 @@ import static org.hamcrest.Matchers.containsString;
 
 public class ServerUtils {
 
-    private static final Logger logger =  LogManager.getLogger(ServerUtils.class);
+    private static final Logger logger = LogManager.getLogger(ServerUtils.class);
 
     private static String SECURITY_ENABLED = "xpack.security.enabled: true";
     private static String SSL_ENABLED = "xpack.security.http.ssl.enabled: true";
 
-    // generous timeout  as nested virtualization can be quite slow ...
+    // generous timeout as nested virtualization can be quite slow ...
     private static final long waitTime = TimeUnit.MINUTES.toMillis(3);
     private static final long timeoutLength = TimeUnit.SECONDS.toMillis(30);
     private static final long requestInterval = TimeUnit.SECONDS.toMillis(5);
@@ -122,9 +122,7 @@ public class ServerUtils {
                 connectionManager.setDefaultMaxPerRoute(100);
                 connectionManager.setMaxTotal(200);
                 connectionManager.setValidateAfterInactivity(1000);
-                executor = Executor.newInstance(HttpClientBuilder.create()
-                    .setConnectionManager(connectionManager)
-                    .build());
+                executor = Executor.newInstance(HttpClientBuilder.create().setConnectionManager(connectionManager).build());
             }
         } else {
             executor = Executor.newInstance();
@@ -157,13 +155,8 @@ public class ServerUtils {
         throw new RuntimeException("Elasticsearch (with x-pack) did not start");
     }
 
-    public static void waitForElasticsearch(
-        String status,
-        String index,
-        Installation installation,
-        String username,
-        String password
-    ) throws Exception {
+    public static void waitForElasticsearch(String status, String index, Installation installation, String username, String password)
+        throws Exception {
 
         Objects.requireNonNull(status);
 
@@ -184,8 +177,7 @@ public class ServerUtils {
                 try {
 
                     final HttpResponse response = execute(
-                        Request
-                            .Get("http://localhost:9200/_cluster/health")
+                        Request.Get("http://localhost:9200/_cluster/health")
                             .connectTimeout((int) timeoutLength)
                             .socketTimeout((int) timeoutLength),
                         username,
@@ -237,11 +229,13 @@ public class ServerUtils {
     public static void runElasticsearchTests() throws Exception {
         makeRequest(
             Request.Post("http://localhost:9200/library/_doc/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/_doc/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"));

+ 30 - 50
qa/os/src/test/java/org/elasticsearch/packaging/util/Shell.java

@@ -42,9 +42,8 @@ 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());
+    public static final Result NO_OP = new Shell.Result(0, "", "");
+    protected final Logger logger = LogManager.getLogger(getClass());
 
     final Map<String, String> env = new HashMap<>();
     Path workingDirectory;
@@ -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(
-            "$account = New-Object System.Security.Principal.NTAccount '" + System.getenv("username")  + "'; " +
-                "$pathInfo = Get-Item '" + path + "'; " +
-                "$toChown = @(); " +
-                "if ($pathInfo.PSIsContainer) { " +
-                "  $toChown += Get-ChildItem '" + path + "' -Recurse; " +
-                "}" +
-                "$toChown += $pathInfo; " +
-                "$toChown | ForEach-Object { " +
-                "$acl = Get-Acl $_.FullName; " +
-                "$acl.SetOwner($account); " +
-                "Set-Acl $_.FullName $acl " +
-                "}"
-        ));
+        Platforms.onWindows(
+            () -> run(
+                String.format(
+                    Locale.ROOT,
+                    "$account = New-Object System.Security.Principal.NTAccount '%s'; "
+                        + "$pathInfo = Get-Item '%s'; "
+                        + "$toChown = @(); "
+                        + "if ($pathInfo.PSIsContainer) { "
+                        + "  $toChown += Get-ChildItem '%s' -Recurse; "
+                        + "}"
+                        + "$toChown += $pathInfo; "
+                        + "$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(
-                    -1,
-                    readFileIfExists(stdOut),
-                    readFileIfExists(stdErr)
-                );
+                Result result = new Result(-1, readFileIfExists(stdOut), readFileIfExists(stdErr));
                 throw new IllegalStateException(
-                    "Timed out running shell command: " + Arrays.toString(command) + "\n" +
-                    "Result:\n" + result
+                    "Timed out running shell command: " + Arrays.toString(command) + "\n" + "Result:\n" + result
                 );
             }
 
-            Result result = new Result(
-                process.exitValue(),
-                readFileIfExists(stdOut),
-                readFileIfExists(stdErr)
-            );
+            Result result = new Result(process.exitValue(), readFileIfExists(stdOut), readFileIfExists(stdErr));
             logger.info("Ran: {} {}", Arrays.toString(command), result);
             return result;
 
@@ -203,7 +201,7 @@ public class Shell {
         if (Files.exists(path)) {
             long size = Files.size(path);
             if (size > 100 * 1024) {
-                return "<<Too large to read: " + size  + " bytes>>";
+                return "<<Too large to read: " + size + " bytes>>";
             }
             try (Stream<String> lines = Files.lines(path, StandardCharsets.UTF_8)) {
                 return lines.collect(Collectors.joining("\n"));
@@ -225,15 +223,7 @@ public class Shell {
     }
 
     public String toString() {
-        return new StringBuilder()
-            .append(" ")
-            .append("env = [")
-            .append(env)
-            .append("]")
-            .append("workingDirectory = [")
-            .append(workingDirectory)
-            .append("]")
-            .toString();
+        return String.format(Locale.ROOT, " env = [%s] workingDirectory = [%s]", env, workingDirectory);
     }
 
     public static class Result {
@@ -252,17 +242,7 @@ public class Shell {
         }
 
         public String toString() {
-            return new StringBuilder()
-                .append("exitCode = [")
-                .append(exitCode)
-                .append("] ")
-                .append("stdout = [")
-                .append(stdout.trim())
-                .append("] ")
-                .append("stderr = [")
-                .append(stderr.trim())
-                .append("]")
-                .toString();
+            return String.format(Locale.ROOT, "exitCode = [%d] stdout = [%s] stderr = [%s]", exitCode, stdout.trim(), stderr.trim());
         }
     }