From d689641cde124a276e0a66377ef93d1678211150 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Wed, 27 Nov 2019 16:18:03 -0800 Subject: [PATCH 1/7] Simplify running tools in packaging tests Running tools requires a shell. This should be the shell setup by the base packaging tests, but currently tests must pass in their own shell. This commit begins to make running tools easier by eliminating the shell argument, instead keeping the shell as part of the Installation (which can eventually be passed through from the test itself on installation). The variable names for each tool are also simplified. --- .../packaging/test/ArchiveTests.java | 32 ++++++++-------- .../packaging/test/DockerTests.java | 16 ++++---- .../packaging/test/PasswordToolsTests.java | 6 +-- .../packaging/test/RpmPreservationTests.java | 2 +- .../packaging/test/SqlCliTests.java | 2 +- .../packaging/util/Archives.java | 2 +- .../elasticsearch/packaging/util/Docker.java | 2 +- .../packaging/util/Installation.java | 37 +++++++++++-------- .../packaging/util/Packages.java | 2 +- 9 files changed, 53 insertions(+), 48 deletions(-) diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/ArchiveTests.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/ArchiveTests.java index 37fd4448974af..3af745d7d8808 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/ArchiveTests.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/ArchiveTests.java @@ -69,7 +69,7 @@ public void test10Install() throws Exception { public void test20PluginsListWithNoPlugins() throws Exception { final Installation.Executables bin = installation.executables(); - final Result r = bin.elasticsearchPlugin.run(sh, "list"); + final Result r = bin.pluginTool.run("list"); assertThat(r.stdout, isEmptyString()); } @@ -109,26 +109,26 @@ public void test31BadJavaHome() throws Exception { public void test40CreateKeystoreManually() throws Exception { final Installation.Executables bin = installation.executables(); - Platforms.onLinux(() -> sh.run("sudo -u " + ARCHIVE_OWNER + " " + bin.elasticsearchKeystore + " create")); + Platforms.onLinux(() -> sh.run("sudo -u " + ARCHIVE_OWNER + " " + bin.keystoreTool + " create")); // this is a hack around the fact that we can't run a command in the same session as the same user but not as administrator. // 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.run(bin.elasticsearchKeystore + " create"); + sh.run(bin.keystoreTool + " create"); sh.chown(installation.config("elasticsearch.keystore")); }); assertThat(installation.config("elasticsearch.keystore"), file(File, ARCHIVE_OWNER, ARCHIVE_OWNER, p660)); Platforms.onLinux(() -> { - final Result r = sh.run("sudo -u " + ARCHIVE_OWNER + " " + bin.elasticsearchKeystore + " list"); + final Result r = sh.run("sudo -u " + ARCHIVE_OWNER + " " + bin.keystoreTool + " list"); assertThat(r.stdout, containsString("keystore.seed")); }); Platforms.onWindows(() -> { - final Result r = sh.run(bin.elasticsearchKeystore + " list"); + final Result r = sh.run(bin.keystoreTool + " list"); assertThat(r.stdout, containsString("keystore.seed")); }); } @@ -256,12 +256,12 @@ public void test60AutoCreateKeystore() throws Exception { final Installation.Executables bin = installation.executables(); Platforms.onLinux(() -> { - final Result result = sh.run("sudo -u " + ARCHIVE_OWNER + " " + bin.elasticsearchKeystore + " list"); + final Result result = sh.run("sudo -u " + ARCHIVE_OWNER + " " + bin.keystoreTool + " list"); assertThat(result.stdout, containsString("keystore.seed")); }); Platforms.onWindows(() -> { - final Result result = sh.run(bin.elasticsearchKeystore + " list"); + final Result result = sh.run(bin.keystoreTool + " list"); assertThat(result.stdout, containsString("keystore.seed")); }); } @@ -341,11 +341,11 @@ public void test90SecurityCliPackaging() throws Exception { if (distribution().isDefault()) { assertTrue(Files.exists(installation.lib.resolve("tools").resolve("security-cli"))); final Platforms.PlatformAction action = () -> { - Result result = sh.run(bin.elasticsearchCertutil + " --help"); + Result result = sh.run(bin.certutilTool + " --help"); assertThat(result.stdout, containsString("Simplifies certificate creation for use with the Elastic Stack")); // Ensure that the exit code from the java command is passed back up through the shell script - result = sh.runIgnoreExitCode(bin.elasticsearchCertutil + " invalid-command"); + result = sh.runIgnoreExitCode(bin.certutilTool + " invalid-command"); assertThat(result.exitCode, is(not(0))); assertThat(result.stderr, containsString("Unknown command [invalid-command]")); }; @@ -360,7 +360,7 @@ public void test91ElasticsearchShardCliPackaging() throws Exception { final Installation.Executables bin = installation.executables(); Platforms.PlatformAction action = () -> { - final Result result = sh.run(bin.elasticsearchShard + " -h"); + final Result result = sh.run(bin.shardTool + " -h"); assertThat(result.stdout, containsString("A CLI tool to remove corrupted parts of unrecoverable shards")); }; @@ -375,7 +375,7 @@ public void test92ElasticsearchNodeCliPackaging() throws Exception { final Installation.Executables bin = installation.executables(); Platforms.PlatformAction action = () -> { - final Result result = sh.run(bin.elasticsearchNode + " -h"); + 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")); }; @@ -396,7 +396,7 @@ public void test93ElasticsearchNodeCustomDataPathAndNotEsHomeWorkDir() throws Ex Archives.runElasticsearch(installation, sh); Archives.stopElasticsearch(installation); - Result result = sh.run("echo y | " + installation.executables().elasticsearchNode + " unsafe-bootstrap"); + Result result = sh.run("echo y | " + installation.executables().nodeTool + " unsafe-bootstrap"); assertThat(result.stdout, containsString("Master node was successfully bootstrapped")); } @@ -406,16 +406,16 @@ public void test94ElasticsearchNodeExecuteCliNotEsHomeWorkDir() throws Exception sh.setWorkingDirectory(getTempDir()); Platforms.PlatformAction action = () -> { - Result result = sh.run(bin.elasticsearchCertutil+ " -h"); + Result result = sh.run(bin.certutilTool + " -h"); assertThat(result.stdout, containsString("Simplifies certificate creation for use with the Elastic Stack")); - result = sh.run(bin.elasticsearchSyskeygen+ " -h"); + result = sh.run(bin.syskeygenTool + " -h"); assertThat(result.stdout, containsString("system key tool")); - result = sh.run(bin.elasticsearchSetupPasswords+ " -h"); + result = sh.run(bin.setupPasswordsTool + " -h"); assertThat(result.stdout, containsString("Sets the passwords for reserved users")); - result = sh.run(bin.elasticsearchUsers+ " -h"); + result = sh.run(bin.usersTool + " -h"); assertThat(result.stdout, containsString("Manages elasticsearch file users")); }; diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/DockerTests.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/DockerTests.java index e75bfc8ad38df..6a41e965fcdb4 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/DockerTests.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/DockerTests.java @@ -101,7 +101,7 @@ public void test10Install() { */ public void test20PluginsListWithNoPlugins() { final Installation.Executables bin = installation.executables(); - final Result r = sh.run(bin.elasticsearchPlugin + " list"); + final Result r = sh.run(bin.pluginTool + " list"); assertThat("Expected no plugins to be listed", r.stdout, emptyString()); } @@ -119,9 +119,9 @@ public void test40CreateKeystoreManually() throws InterruptedException { // Move the auto-created one out of the way, or else the CLI prompts asks us to confirm sh.run("mv " + keystorePath + " " + keystorePath + ".bak"); - sh.run(bin.elasticsearchKeystore + " create"); + sh.run(bin.keystoreTool + " create"); - final Result r = sh.run(bin.elasticsearchKeystore + " list"); + final Result r = sh.run(bin.keystoreTool + " list"); assertThat(r.stdout, containsString("keystore.seed")); } @@ -148,7 +148,7 @@ public void test60AutoCreateKeystore() throws Exception { assertPermissionsAndOwnership(keystorePath, p660); final Installation.Executables bin = installation.executables(); - final Result result = sh.run(bin.elasticsearchKeystore + " list"); + final Result result = sh.run(bin.keystoreTool + " list"); assertThat(result.stdout, containsString("keystore.seed")); } @@ -341,11 +341,11 @@ public void test90SecurityCliPackaging() { if (distribution().isDefault()) { assertTrue(existsInContainer(securityCli)); - Result result = sh.run(bin.elasticsearchCertutil + " --help"); + Result result = sh.run(bin.certutilTool + " --help"); assertThat(result.stdout, containsString("Simplifies certificate creation for use with the Elastic Stack")); // Ensure that the exit code from the java command is passed back up through the shell script - result = sh.runIgnoreExitCode(bin.elasticsearchCertutil + " invalid-command"); + result = sh.runIgnoreExitCode(bin.certutilTool + " invalid-command"); assertThat(result.isSuccess(), is(false)); assertThat(result.stdout, containsString("Unknown command [invalid-command]")); } else { @@ -359,7 +359,7 @@ public void test90SecurityCliPackaging() { public void test91ElasticsearchShardCliPackaging() { final Installation.Executables bin = installation.executables(); - final Result result = sh.run(bin.elasticsearchShard + " -h"); + final Result result = sh.run(bin.shardTool + " -h"); assertThat(result.stdout, containsString("A CLI tool to remove corrupted parts of unrecoverable shards")); } @@ -369,7 +369,7 @@ public void test91ElasticsearchShardCliPackaging() { public void test92ElasticsearchNodeCliPackaging() { final Installation.Executables bin = installation.executables(); - final Result result = sh.run(bin.elasticsearchNode + " -h"); + 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")); } } diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/PasswordToolsTests.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/PasswordToolsTests.java index 9082b19f0bd49..b08b3bfe52987 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/PasswordToolsTests.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/PasswordToolsTests.java @@ -59,7 +59,7 @@ public void test010Install() throws Exception { public void test20GeneratePasswords() throws Exception { assertWhileRunning(() -> { - Shell.Result result = installation.executables().elasticsearchSetupPasswords.run(sh, "auto --batch", null); + Shell.Result result = installation.executables().setupPasswordsTool.run("auto --batch", null); Map userpasses = parseUsersAndPasswords(result.stdout); for (Map.Entry userpass : userpasses.entrySet()) { String response = ServerUtils.makeRequest(Request.Get("http://localhost:9200"), userpass.getKey(), userpass.getValue()); @@ -106,7 +106,7 @@ public void test30AddBootstrapPassword() throws Exception { }); } - installation.executables().elasticsearchKeystore.run(sh, "add --stdin bootstrap.password", BOOTSTRAP_PASSWORD); + installation.executables().keystoreTool.run("add --stdin bootstrap.password", BOOTSTRAP_PASSWORD); assertWhileRunning(() -> { String response = ServerUtils.makeRequest( @@ -119,7 +119,7 @@ public void test30AddBootstrapPassword() throws Exception { public void test40GeneratePasswordsBootstrapAlreadySet() throws Exception { assertWhileRunning(() -> { - Shell.Result result = installation.executables().elasticsearchSetupPasswords.run(sh, "auto --batch", null); + Shell.Result result = installation.executables().setupPasswordsTool.run("auto --batch", null); Map userpasses = parseUsersAndPasswords(result.stdout); assertThat(userpasses, hasKey("elastic")); for (Map.Entry userpass : userpasses.entrySet()) { diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/RpmPreservationTests.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/RpmPreservationTests.java index 0509b1d244b1a..a9fcbbd7974cd 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/RpmPreservationTests.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/RpmPreservationTests.java @@ -75,7 +75,7 @@ public void test30PreserveConfig() throws Exception { assertInstalled(distribution()); verifyPackageInstallation(installation, distribution(), newShell()); - sh.run("echo foobar | " + installation.executables().elasticsearchKeystore + " add --stdin foo.bar"); + sh.run("echo foobar | " + installation.executables().keystoreTool + " add --stdin foo.bar"); Stream.of( "elasticsearch.yml", "jvm.options", diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/SqlCliTests.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/SqlCliTests.java index 62a00aab59d22..afcc7dd0b0919 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/SqlCliTests.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/SqlCliTests.java @@ -38,7 +38,7 @@ public void test010Install() throws Exception { } public void test020Help() throws Exception { - Shell.Result result = installation.executables().elasticsearchSqlCli.run(sh, "--help"); + Shell.Result result = installation.executables().sqlCli.run("--help"); assertThat(result.stdout, containsString("Elasticsearch SQL CLI")); } } diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/util/Archives.java b/qa/os/src/test/java/org/elasticsearch/packaging/util/Archives.java index 4303ca3bb6dd1..48b33733cf337 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/util/Archives.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/util/Archives.java @@ -111,7 +111,7 @@ public static Installation installArchive(Distribution distribution, Path fullIn sh.chown(fullInstallPath); - return Installation.ofArchive(distribution, fullInstallPath); + return Installation.ofArchive(sh, distribution, fullInstallPath); } private static void setupArchiveUsersLinux(Path installPath) { diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/util/Docker.java b/qa/os/src/test/java/org/elasticsearch/packaging/util/Docker.java index 8d5b7ce12c72f..1bc2671247785 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/util/Docker.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/util/Docker.java @@ -99,7 +99,7 @@ public static Installation runContainer(Distribution distribution, Map Date: Thu, 5 Dec 2019 12:52:40 -0800 Subject: [PATCH 2/7] Pass shell through --- .../elasticsearch/packaging/test/ArchiveTests.java | 2 +- .../packaging/test/DebPreservationTests.java | 2 +- .../elasticsearch/packaging/test/PackageTests.java | 14 +++++++------- .../packaging/test/PackagingTestCase.java | 4 ++-- .../packaging/test/RpmPreservationTests.java | 4 ++-- .../packaging/test/WindowsServiceTests.java | 2 +- .../org/elasticsearch/packaging/util/Archives.java | 8 +++----- .../org/elasticsearch/packaging/util/Packages.java | 3 +-- 8 files changed, 18 insertions(+), 21 deletions(-) diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/ArchiveTests.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/ArchiveTests.java index 3af745d7d8808..aebb4251a7053 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/ArchiveTests.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/ArchiveTests.java @@ -63,7 +63,7 @@ public static void filterDistros() { } public void test10Install() throws Exception { - installation = installArchive(distribution()); + installation = installArchive(sh, distribution()); verifyArchiveInstallation(installation, distribution()); } diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/DebPreservationTests.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/DebPreservationTests.java index ea4f5565a98a8..6603be247e738 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/DebPreservationTests.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/DebPreservationTests.java @@ -47,7 +47,7 @@ public static void filterDistros() { public void test10Install() throws Exception { assertRemoved(distribution()); - installation = installPackage(distribution()); + installation = installPackage(sh, distribution()); assertInstalled(distribution()); verifyPackageInstallation(installation, distribution(), newShell()); } diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/PackageTests.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/PackageTests.java index 155048be8a630..64a81cd41bd59 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/PackageTests.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/PackageTests.java @@ -75,7 +75,7 @@ public static void filterDistros() { public void test10InstallPackage() throws Exception { assertRemoved(distribution()); - installation = installPackage(distribution()); + installation = installPackage(sh, distribution()); assertInstalled(distribution()); verifyPackageInstallation(installation, distribution(), sh); } @@ -218,7 +218,7 @@ public void test50Remove() throws Exception { } public void test60Reinstall() throws Exception { - installation = installPackage(distribution()); + installation = installPackage(sh, distribution()); assertInstalled(distribution()); verifyPackageInstallation(installation, distribution(), sh); @@ -228,7 +228,7 @@ public void test60Reinstall() throws Exception { public void test70RestartServer() throws Exception { try { - installation = installPackage(distribution()); + installation = installPackage(sh, distribution()); assertInstalled(distribution()); startElasticsearch(sh, installation); @@ -243,7 +243,7 @@ public void test70RestartServer() throws Exception { public void test72TestRuntimeDirectory() throws Exception { try { - installation = installPackage(distribution()); + installation = installPackage(sh, distribution()); FileUtils.rm(installation.pidDir); startElasticsearch(sh, installation); assertPathsExist(installation.pidDir); @@ -254,7 +254,7 @@ public void test72TestRuntimeDirectory() throws Exception { } public void test73gcLogsExist() throws Exception { - installation = installPackage(distribution()); + installation = installPackage(sh, distribution()); startElasticsearch(sh, installation); // it can be gc.log or gc.log.0.current assertThat(installation.logs, fileWithGlobExist("gc.log*")); @@ -306,7 +306,7 @@ public void test82SystemdMask() throws Exception { sh.run("systemctl mask systemd-sysctl.service"); - installation = installPackage(distribution()); + installation = installPackage(sh, distribution()); sh.run("systemctl unmask systemd-sysctl.service"); } finally { @@ -318,7 +318,7 @@ public void test83serviceFileSetsLimits() throws Exception { // Limits are changed on systemd platforms only assumeTrue(isSystemd()); - installation = installPackage(distribution()); + installation = installPackage(sh, distribution()); startElasticsearch(sh, installation); diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java index cb95408b2a5bf..efabe8c929516 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java @@ -128,12 +128,12 @@ protected static void install() throws Exception { switch (distribution.packaging) { case TAR: case ZIP: - installation = Archives.installArchive(distribution); + installation = Archives.installArchive(newShell(), distribution); Archives.verifyArchiveInstallation(installation, distribution); break; case DEB: case RPM: - installation = Packages.installPackage(distribution); + installation = Packages.installPackage(newShell(), distribution); Packages.verifyPackageInstallation(installation, distribution, newShell()); break; case DOCKER: diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/RpmPreservationTests.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/RpmPreservationTests.java index a9fcbbd7974cd..f95520658515b 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/RpmPreservationTests.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/RpmPreservationTests.java @@ -50,7 +50,7 @@ public static void filterDistros() { public void test10Install() throws Exception { assertRemoved(distribution()); - installation = installPackage(distribution()); + installation = installPackage(sh, distribution()); assertInstalled(distribution()); verifyPackageInstallation(installation, distribution(), newShell()); } @@ -71,7 +71,7 @@ public void test20Remove() throws Exception { public void test30PreserveConfig() throws Exception { final Shell sh = new Shell(); - installation = installPackage(distribution()); + installation = installPackage(sh, distribution()); assertInstalled(distribution()); verifyPackageInstallation(installation, distribution(), newShell()); diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/WindowsServiceTests.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/WindowsServiceTests.java index 506a7313cb17c..5f4fac5f5352c 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/WindowsServiceTests.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/WindowsServiceTests.java @@ -96,7 +96,7 @@ private void assertExit(Result result, String script, int exitCode) { } public void test10InstallArchive() throws Exception { - installation = installArchive(distribution()); + installation = installArchive(sh, distribution()); verifyArchiveInstallation(installation, distribution()); serviceScript = installation.bin("elasticsearch-service.bat").toString(); } diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/util/Archives.java b/qa/os/src/test/java/org/elasticsearch/packaging/util/Archives.java index 48b33733cf337..c8036799020a6 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/util/Archives.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/util/Archives.java @@ -63,13 +63,11 @@ public class Archives { ? System.getenv("username") : "elasticsearch"; - public static Installation installArchive(Distribution distribution) throws Exception { - return installArchive(distribution, getDefaultArchiveInstallPath(), getCurrentVersion()); + public static Installation installArchive(Shell sh, Distribution distribution) throws Exception { + return installArchive(sh, distribution, getDefaultArchiveInstallPath(), getCurrentVersion()); } - public static Installation installArchive(Distribution distribution, Path fullInstallPath, String version) throws Exception { - final Shell sh = new Shell(); - + public static Installation installArchive(Shell sh, Distribution distribution, Path fullInstallPath, String version) throws Exception { final Path distributionFile = getDistributionFile(distribution); final Path baseInstallPath = fullInstallPath.getParent(); final Path extractedPath = baseInstallPath.resolve("elasticsearch-" + version); diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/util/Packages.java b/qa/os/src/test/java/org/elasticsearch/packaging/util/Packages.java index 24467ef2049ef..25b16b493103a 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/util/Packages.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/util/Packages.java @@ -95,8 +95,7 @@ public static Result packageStatus(Distribution distribution) { return result; } - public static Installation installPackage(Distribution distribution) throws IOException { - Shell sh = new Shell(); + public static Installation installPackage(Shell sh, Distribution distribution) throws IOException { String systemJavaHome = sh.run("echo $SYSTEM_JAVA_HOME").stdout.trim(); if (distribution.hasJdk == false) { sh.getEnv().put("JAVA_HOME", systemJavaHome); From 5932b10b9df11c0836a253834f7a343a78cc8d4f Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Mon, 9 Dec 2019 11:36:27 -0800 Subject: [PATCH 3/7] Reuse shell instance and reset between tests --- .../packaging/test/PackagingTestCase.java | 11 ++++++++--- .../java/org/elasticsearch/packaging/util/Shell.java | 9 +++++++++ 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java index efabe8c929516..a5e2d53f03c20 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java @@ -96,7 +96,7 @@ protected void failed(Throwable e, Description description) { }; // a shell to run system commands with - protected Shell sh; + protected static Shell sh; @Rule public final TestName testNameRule = new TestName(); @@ -112,11 +112,16 @@ public static void cleanup() throws Exception { cleanEverything(); } + @BeforeClass + public static void createShell() throws Exception { + sh = newShell(); + } + @Before public void setup() throws Exception { assumeFalse(failed); // skip rest of tests once one fails - sh = newShell(); + sh.reset(); } /** The {@link Distribution} that should be tested in this case */ @@ -195,7 +200,7 @@ protected void assertWhileRunning(Platforms.PlatformAction assertions) throws Ex } } - protected static Shell newShell() throws Exception { + static Shell newShell() throws Exception { Shell sh = new Shell(); if (distribution().hasJdk == false) { Platforms.onLinux(() -> { diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/util/Shell.java b/qa/os/src/test/java/org/elasticsearch/packaging/util/Shell.java index 55488522797c1..cef88ba0ed6e3 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/util/Shell.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/util/Shell.java @@ -52,6 +52,14 @@ public Shell() { this.workingDirectory = null; } + /** + * Reset the shell to its newly created state. + */ + public void reset() { + env.clear(); + workingDirectory = null; + } + public Map getEnv() { return env; } @@ -111,6 +119,7 @@ private static String[] powershellCommand(String script) { } private Result runScript(String[] command) { + logger.warn("Running command with env: " + env); Result result = runScriptIgnoreExitCode(command); if (result.isSuccess() == false) { throw new RuntimeException("Command was not successful: [" + String.join(" ", command) + "]\n result: " + result.toString()); From 69ba5e434367b6d3094f36e74a5c064694966667 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Mon, 9 Dec 2019 16:27:46 -0800 Subject: [PATCH 4/7] remove local shells --- .../java/org/elasticsearch/packaging/test/ArchiveTests.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/ArchiveTests.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/ArchiveTests.java index 55f3b43302e71..9691636f5ccda 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/ArchiveTests.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/ArchiveTests.java @@ -25,7 +25,6 @@ import org.elasticsearch.packaging.util.Installation; import org.elasticsearch.packaging.util.Platforms; import org.elasticsearch.packaging.util.ServerUtils; -import org.elasticsearch.packaging.util.Shell; import org.elasticsearch.packaging.util.Shell.Result; import org.junit.BeforeClass; @@ -202,7 +201,6 @@ public void test52BundledJdkRemoved() throws Exception { public void test53JavaHomeWithSpecialCharacters() throws Exception { Platforms.onWindows(() -> { - final Shell sh = new Shell(); String javaPath = "C:\\Program Files (x86)\\java"; try { // once windows 2012 is no longer supported and powershell 5.0 is always available we can change this command @@ -228,7 +226,6 @@ public void test53JavaHomeWithSpecialCharacters() throws Exception { }); Platforms.onLinux(() -> { - final Shell sh = newShell(); // Create temporary directory with a space and link to real java home String testJavaHome = Paths.get("/tmp", "java home").toString(); try { From 08076a2f5ba7772a2c553ab3741a50c45fa551e3 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Tue, 10 Dec 2019 08:41:44 -0800 Subject: [PATCH 5/7] properly reset env for each test --- .../packaging/test/DebPreservationTests.java | 2 +- .../packaging/test/PackagingTestCase.java | 23 ++++++++----------- .../packaging/test/RpmPreservationTests.java | 4 ++-- 3 files changed, 12 insertions(+), 17 deletions(-) diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/DebPreservationTests.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/DebPreservationTests.java index 6603be247e738..4ff08ad47acf6 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/DebPreservationTests.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/DebPreservationTests.java @@ -49,7 +49,7 @@ public void test10Install() throws Exception { assertRemoved(distribution()); installation = installPackage(sh, distribution()); assertInstalled(distribution()); - verifyPackageInstallation(installation, distribution(), newShell()); + verifyPackageInstallation(installation, distribution(), sh); } public void test20Remove() throws Exception { diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java index c71b94264954f..b77d2bab85625 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java @@ -116,7 +116,7 @@ public static void cleanup() throws Exception { @BeforeClass public static void createShell() throws Exception { - sh = newShell(); + sh = new Shell(); } @Before @@ -124,6 +124,14 @@ 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); + }); + } } /** The {@link Distribution} that should be tested in this case */ @@ -181,19 +189,6 @@ protected void assertWhileRunning(Platforms.PlatformAction assertions) throws Ex stopElasticsearch(); } - static Shell newShell() throws Exception { - Shell sh = new Shell(); - if (distribution().hasJdk == false) { - Platforms.onLinux(() -> { - sh.getEnv().put("JAVA_HOME", systemJavaHome); - }); - Platforms.onWindows(() -> { - sh.getEnv().put("JAVA_HOME", systemJavaHome); - }); - } - return sh; - } - /** * Run the command to start Elasticsearch, but don't wait or test for success. * This method is useful for testing failure conditions in startup. To await success, diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/RpmPreservationTests.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/RpmPreservationTests.java index f95520658515b..4448e02bd0692 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/RpmPreservationTests.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/RpmPreservationTests.java @@ -52,7 +52,7 @@ public void test10Install() throws Exception { assertRemoved(distribution()); installation = installPackage(sh, distribution()); assertInstalled(distribution()); - verifyPackageInstallation(installation, distribution(), newShell()); + verifyPackageInstallation(installation, distribution(), sh); } public void test20Remove() throws Exception { @@ -73,7 +73,7 @@ public void test30PreserveConfig() throws Exception { installation = installPackage(sh, distribution()); assertInstalled(distribution()); - verifyPackageInstallation(installation, distribution(), newShell()); + verifyPackageInstallation(installation, distribution(), sh); sh.run("echo foobar | " + installation.executables().keystoreTool + " add --stdin foo.bar"); Stream.of( From e7f3f606728e100309a6ccb5eca80f13f7892a37 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Tue, 10 Dec 2019 17:31:10 -0800 Subject: [PATCH 6/7] add some debug messages --- .../elasticsearch/packaging/test/PackagingTestCase.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java index b77d2bab85625..fa24f0e81c789 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java @@ -37,7 +37,6 @@ import org.junit.Assert; import org.junit.Before; import org.junit.BeforeClass; -import org.junit.ClassRule; import org.junit.Rule; import org.junit.rules.TestName; import org.junit.rules.TestWatcher; @@ -89,8 +88,8 @@ public abstract class PackagingTestCase extends Assert { private static boolean failed; - @ClassRule - public static final TestWatcher testFailureRule = new TestWatcher() { + @Rule + public final TestWatcher testFailureRule = new TestWatcher() { @Override protected void failed(Throwable e, Description description) { failed = true; @@ -124,6 +123,8 @@ public void setup() throws Exception { assumeFalse(failed); // skip rest of tests once one fails sh.reset(); + logger.info("Testing distribution: " + distribution().path); + logger.info(" hasJdk: " + distribution().hasJdk); if (distribution().hasJdk == false) { Platforms.onLinux(() -> { sh.getEnv().put("JAVA_HOME", systemJavaHome); From b12135b5be8e75c5bde239aa96455b4af6604c28 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Wed, 11 Dec 2019 13:29:52 -0800 Subject: [PATCH 7/7] remove debug --- .../org/elasticsearch/packaging/test/PackagingTestCase.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java index fa24f0e81c789..da3f55daae035 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java @@ -123,8 +123,6 @@ public void setup() throws Exception { assumeFalse(failed); // skip rest of tests once one fails sh.reset(); - logger.info("Testing distribution: " + distribution().path); - logger.info(" hasJdk: " + distribution().hasJdk); if (distribution().hasJdk == false) { Platforms.onLinux(() -> { sh.getEnv().put("JAVA_HOME", systemJavaHome);