Skip to content

Commit 54467b5

Browse files
rjernstrjernst
authored
Simplify running tools in packaging tests (#49665) (#50110)
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.
1 parent ff6ad58 commit 54467b5

File tree

14 files changed

+95
-90
lines changed

14 files changed

+95
-90
lines changed

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

Lines changed: 17 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525
import org.elasticsearch.packaging.util.Installation;
2626
import org.elasticsearch.packaging.util.Platforms;
2727
import org.elasticsearch.packaging.util.ServerUtils;
28-
import org.elasticsearch.packaging.util.Shell;
2928
import org.elasticsearch.packaging.util.Shell.Result;
3029
import org.junit.BeforeClass;
3130

@@ -63,13 +62,13 @@ public static void filterDistros() {
6362
}
6463

6564
public void test10Install() throws Exception {
66-
installation = installArchive(distribution());
65+
installation = installArchive(sh, distribution());
6766
verifyArchiveInstallation(installation, distribution());
6867
}
6968

7069
public void test20PluginsListWithNoPlugins() throws Exception {
7170
final Installation.Executables bin = installation.executables();
72-
final Result r = bin.elasticsearchPlugin.run(sh, "list");
71+
final Result r = bin.pluginTool.run("list");
7372

7473
assertThat(r.stdout, isEmptyString());
7574
}
@@ -109,26 +108,26 @@ public void test31BadJavaHome() throws Exception {
109108
public void test40CreateKeystoreManually() throws Exception {
110109
final Installation.Executables bin = installation.executables();
111110

112-
Platforms.onLinux(() -> sh.run("sudo -u " + ARCHIVE_OWNER + " " + bin.elasticsearchKeystore + " create"));
111+
Platforms.onLinux(() -> sh.run("sudo -u " + ARCHIVE_OWNER + " " + bin.keystoreTool + " create"));
113112

114113
// 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.
115114
// the keystore ends up being owned by the Administrators group, so we manually set it to be owned by the vagrant user here.
116115
// from the server's perspective the permissions aren't really different, this is just to reflect what we'd expect in the tests.
117116
// when we run these commands as a role user we won't have to do this
118117
Platforms.onWindows(() -> {
119-
sh.run(bin.elasticsearchKeystore + " create");
118+
sh.run(bin.keystoreTool + " create");
120119
sh.chown(installation.config("elasticsearch.keystore"));
121120
});
122121

123122
assertThat(installation.config("elasticsearch.keystore"), file(File, ARCHIVE_OWNER, ARCHIVE_OWNER, p660));
124123

125124
Platforms.onLinux(() -> {
126-
final Result r = sh.run("sudo -u " + ARCHIVE_OWNER + " " + bin.elasticsearchKeystore + " list");
125+
final Result r = sh.run("sudo -u " + ARCHIVE_OWNER + " " + bin.keystoreTool + " list");
127126
assertThat(r.stdout, containsString("keystore.seed"));
128127
});
129128

130129
Platforms.onWindows(() -> {
131-
final Result r = sh.run(bin.elasticsearchKeystore + " list");
130+
final Result r = sh.run(bin.keystoreTool + " list");
132131
assertThat(r.stdout, containsString("keystore.seed"));
133132
});
134133
}
@@ -206,7 +205,6 @@ public void test52BundledJdkRemoved() throws Exception {
206205

207206
public void test53JavaHomeWithSpecialCharacters() throws Exception {
208207
Platforms.onWindows(() -> {
209-
final Shell sh = new Shell();
210208
String javaPath = "C:\\Program Files (x86)\\java";
211209
try {
212210
// once windows 2012 is no longer supported and powershell 5.0 is always available we can change this command
@@ -232,7 +230,6 @@ public void test53JavaHomeWithSpecialCharacters() throws Exception {
232230
});
233231

234232
Platforms.onLinux(() -> {
235-
final Shell sh = newShell();
236233
// Create temporary directory with a space and link to real java home
237234
String testJavaHome = Paths.get("/tmp", "java home").toString();
238235
try {
@@ -260,12 +257,12 @@ public void test60AutoCreateKeystore() throws Exception {
260257

261258
final Installation.Executables bin = installation.executables();
262259
Platforms.onLinux(() -> {
263-
final Result result = sh.run("sudo -u " + ARCHIVE_OWNER + " " + bin.elasticsearchKeystore + " list");
260+
final Result result = sh.run("sudo -u " + ARCHIVE_OWNER + " " + bin.keystoreTool + " list");
264261
assertThat(result.stdout, containsString("keystore.seed"));
265262
});
266263

267264
Platforms.onWindows(() -> {
268-
final Result result = sh.run(bin.elasticsearchKeystore + " list");
265+
final Result result = sh.run(bin.keystoreTool + " list");
269266
assertThat(result.stdout, containsString("keystore.seed"));
270267
});
271268
}
@@ -343,11 +340,11 @@ public void test90SecurityCliPackaging() throws Exception {
343340
if (distribution().isDefault()) {
344341
assertTrue(Files.exists(installation.lib.resolve("tools").resolve("security-cli")));
345342
final Platforms.PlatformAction action = () -> {
346-
Result result = sh.run(bin.elasticsearchCertutil + " --help");
343+
Result result = sh.run(bin.certutilTool + " --help");
347344
assertThat(result.stdout, containsString("Simplifies certificate creation for use with the Elastic Stack"));
348345

349346
// Ensure that the exit code from the java command is passed back up through the shell script
350-
result = sh.runIgnoreExitCode(bin.elasticsearchCertutil + " invalid-command");
347+
result = sh.runIgnoreExitCode(bin.certutilTool + " invalid-command");
351348
assertThat(result.exitCode, is(not(0)));
352349
assertThat(result.stderr, containsString("Unknown command [invalid-command]"));
353350
};
@@ -362,7 +359,7 @@ public void test91ElasticsearchShardCliPackaging() throws Exception {
362359
final Installation.Executables bin = installation.executables();
363360

364361
Platforms.PlatformAction action = () -> {
365-
final Result result = sh.run(bin.elasticsearchShard + " -h");
362+
final Result result = sh.run(bin.shardTool + " -h");
366363
assertThat(result.stdout, containsString("A CLI tool to remove corrupted parts of unrecoverable shards"));
367364
};
368365

@@ -377,7 +374,7 @@ public void test92ElasticsearchNodeCliPackaging() throws Exception {
377374
final Installation.Executables bin = installation.executables();
378375

379376
Platforms.PlatformAction action = () -> {
380-
final Result result = sh.run(bin.elasticsearchNode + " -h");
377+
final Result result = sh.run(bin.nodeTool + " -h");
381378
assertThat(result.stdout,
382379
containsString("A CLI tool to do unsafe cluster and index manipulations on current node"));
383380
};
@@ -398,7 +395,7 @@ public void test93ElasticsearchNodeCustomDataPathAndNotEsHomeWorkDir() throws Ex
398395
startElasticsearch();
399396
Archives.stopElasticsearch(installation);
400397

401-
Result result = sh.run("echo y | " + installation.executables().elasticsearchNode + " unsafe-bootstrap");
398+
Result result = sh.run("echo y | " + installation.executables().nodeTool + " unsafe-bootstrap");
402399
assertThat(result.stdout, containsString("Master node was successfully bootstrapped"));
403400
}
404401

@@ -408,16 +405,16 @@ public void test94ElasticsearchNodeExecuteCliNotEsHomeWorkDir() throws Exception
408405
sh.setWorkingDirectory(getTempDir());
409406

410407
Platforms.PlatformAction action = () -> {
411-
Result result = sh.run(bin.elasticsearchCertutil+ " -h");
408+
Result result = sh.run(bin.certutilTool + " -h");
412409
assertThat(result.stdout,
413410
containsString("Simplifies certificate creation for use with the Elastic Stack"));
414-
result = sh.run(bin.elasticsearchSyskeygen+ " -h");
411+
result = sh.run(bin.syskeygenTool + " -h");
415412
assertThat(result.stdout,
416413
containsString("system key tool"));
417-
result = sh.run(bin.elasticsearchSetupPasswords+ " -h");
414+
result = sh.run(bin.setupPasswordsTool + " -h");
418415
assertThat(result.stdout,
419416
containsString("Sets the passwords for reserved users"));
420-
result = sh.run(bin.elasticsearchUsers+ " -h");
417+
result = sh.run(bin.usersTool + " -h");
421418
assertThat(result.stdout,
422419
containsString("Manages elasticsearch file users"));
423420
};

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,9 +47,9 @@ public static void filterDistros() {
4747

4848
public void test10Install() throws Exception {
4949
assertRemoved(distribution());
50-
installation = installPackage(distribution());
50+
installation = installPackage(sh, distribution());
5151
assertInstalled(distribution());
52-
verifyPackageInstallation(installation, distribution(), newShell());
52+
verifyPackageInstallation(installation, distribution(), sh);
5353
}
5454

5555
public void test20Remove() throws Exception {

‎qa/os/src/test/java/org/elasticsearch/packaging/test/DockerTests.java‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ public void test011PresenceOfXpack() throws Exception {
134134
*/
135135
public void test020PluginsListWithNoPlugins() {
136136
final Installation.Executables bin = installation.executables();
137-
final Result r = sh.run(bin.elasticsearchPlugin + " list");
137+
final Result r = sh.run(bin.pluginTool + " list");
138138

139139
assertThat("Expected no plugins to be listed", r.stdout, emptyString());
140140
}
@@ -152,9 +152,9 @@ public void test040CreateKeystoreManually() throws InterruptedException {
152152
// Move the auto-created one out of the way, or else the CLI prompts asks us to confirm
153153
sh.run("mv " + keystorePath + " " + keystorePath + ".bak");
154154

155-
sh.run(bin.elasticsearchKeystore + " create");
155+
sh.run(bin.keystoreTool + " create");
156156

157-
final Result r = sh.run(bin.elasticsearchKeystore + " list");
157+
final Result r = sh.run(bin.keystoreTool + " list");
158158
assertThat(r.stdout, containsString("keystore.seed"));
159159
}
160160

@@ -169,7 +169,7 @@ public void test041AutoCreateKeystore() throws Exception {
169169
assertPermissionsAndOwnership(keystorePath, p660);
170170

171171
final Installation.Executables bin = installation.executables();
172-
final Result result = sh.run(bin.elasticsearchKeystore + " list");
172+
final Result result = sh.run(bin.keystoreTool + " list");
173173
assertThat(result.stdout, containsString("keystore.seed"));
174174
}
175175

@@ -403,11 +403,11 @@ public void test090SecurityCliPackaging() {
403403
if (distribution().isDefault()) {
404404
assertTrue(existsInContainer(securityCli));
405405

406-
Result result = sh.run(bin.elasticsearchCertutil + " --help");
406+
Result result = sh.run(bin.certutilTool + " --help");
407407
assertThat(result.stdout, containsString("Simplifies certificate creation for use with the Elastic Stack"));
408408

409409
// Ensure that the exit code from the java command is passed back up through the shell script
410-
result = sh.runIgnoreExitCode(bin.elasticsearchCertutil + " invalid-command");
410+
result = sh.runIgnoreExitCode(bin.certutilTool + " invalid-command");
411411
assertThat(result.isSuccess(), is(false));
412412
assertThat(result.stdout, containsString("Unknown command [invalid-command]"));
413413
} else {
@@ -421,7 +421,7 @@ public void test090SecurityCliPackaging() {
421421
public void test091ElasticsearchShardCliPackaging() {
422422
final Installation.Executables bin = installation.executables();
423423

424-
final Result result = sh.run(bin.elasticsearchShard + " -h");
424+
final Result result = sh.run(bin.shardTool + " -h");
425425
assertThat(result.stdout, containsString("A CLI tool to remove corrupted parts of unrecoverable shards"));
426426
}
427427

@@ -431,7 +431,7 @@ public void test091ElasticsearchShardCliPackaging() {
431431
public void test092ElasticsearchNodeCliPackaging() {
432432
final Installation.Executables bin = installation.executables();
433433

434-
final Result result = sh.run(bin.elasticsearchNode + " -h");
434+
final Result result = sh.run(bin.nodeTool + " -h");
435435
assertThat(
436436
"Failed to find expected message about the elasticsearch-node CLI tool",
437437
result.stdout,

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ public static void filterDistros() {
7272

7373
public void test10InstallPackage() throws Exception {
7474
assertRemoved(distribution());
75-
installation = installPackage(distribution());
75+
installation = installPackage(sh, distribution());
7676
assertInstalled(distribution());
7777
verifyPackageInstallation(installation, distribution(), sh);
7878
}
@@ -303,7 +303,6 @@ public void test82SystemdMask() throws Exception {
303303
assumeTrue(isSystemd());
304304

305305
sh.run("systemctl mask systemd-sysctl.service");
306-
307306
install();
308307

309308
sh.run("systemctl unmask systemd-sysctl.service");

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

Lines changed: 20 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@
3737
import org.junit.Assert;
3838
import org.junit.Before;
3939
import org.junit.BeforeClass;
40-
import org.junit.ClassRule;
4140
import org.junit.Rule;
4241
import org.junit.rules.TestName;
4342
import org.junit.rules.TestWatcher;
@@ -89,16 +88,16 @@ public abstract class PackagingTestCase extends Assert {
8988

9089
private static boolean failed;
9190

92-
@ClassRule
93-
public static final TestWatcher testFailureRule = new TestWatcher() {
91+
@Rule
92+
public final TestWatcher testFailureRule = new TestWatcher() {
9493
@Override
9594
protected void failed(Throwable e, Description description) {
9695
failed = true;
9796
}
9897
};
9998

10099
// a shell to run system commands with
101-
protected Shell sh;
100+
protected static Shell sh;
102101

103102
@Rule
104103
public final TestName testNameRule = new TestName();
@@ -114,11 +113,24 @@ public static void cleanup() throws Exception {
114113
cleanEverything();
115114
}
116115

116+
@BeforeClass
117+
public static void createShell() throws Exception {
118+
sh = new Shell();
119+
}
120+
117121
@Before
118122
public void setup() throws Exception {
119123
assumeFalse(failed); // skip rest of tests once one fails
120124

121-
sh = newShell();
125+
sh.reset();
126+
if (distribution().hasJdk == false) {
127+
Platforms.onLinux(() -> {
128+
sh.getEnv().put("JAVA_HOME", systemJavaHome);
129+
});
130+
Platforms.onWindows(() -> {
131+
sh.getEnv().put("JAVA_HOME", systemJavaHome);
132+
});
133+
}
122134
}
123135

124136
/** The {@link Distribution} that should be tested in this case */
@@ -130,13 +142,13 @@ protected static void install() throws Exception {
130142
switch (distribution.packaging) {
131143
case TAR:
132144
case ZIP:
133-
installation = Archives.installArchive(distribution);
145+
installation = Archives.installArchive(sh, distribution);
134146
Archives.verifyArchiveInstallation(installation, distribution);
135147
break;
136148
case DEB:
137149
case RPM:
138-
installation = Packages.installPackage(distribution);
139-
Packages.verifyPackageInstallation(installation, distribution, newShell());
150+
installation = Packages.installPackage(sh, distribution);
151+
Packages.verifyPackageInstallation(installation, distribution, sh);
140152
break;
141153
case DOCKER:
142154
installation = Docker.runContainer(distribution);
@@ -176,19 +188,6 @@ protected void assertWhileRunning(Platforms.PlatformAction assertions) throws Ex
176188
stopElasticsearch();
177189
}
178190

179-
protected static Shell newShell() throws Exception {
180-
Shell sh = new Shell();
181-
if (distribution().hasJdk == false) {
182-
Platforms.onLinux(() -> {
183-
sh.getEnv().put("JAVA_HOME", systemJavaHome);
184-
});
185-
Platforms.onWindows(() -> {
186-
sh.getEnv().put("JAVA_HOME", systemJavaHome);
187-
});
188-
}
189-
return sh;
190-
}
191-
192191
/**
193192
* Run the command to start Elasticsearch, but don't wait or test for success.
194193
* This method is useful for testing failure conditions in startup. To await success,
@@ -290,7 +289,6 @@ public void assertElasticsearchFailure(Shell.Result result, String expectedMessa
290289

291290
// Otherwise, error should be on shell stderr
292291
assertThat(result.stderr, containsString(expectedMessage));
293-
294292
}
295293
}
296294
}

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ public void test010Install() throws Exception {
5959

6060
public void test20GeneratePasswords() throws Exception {
6161
assertWhileRunning(() -> {
62-
Shell.Result result = installation.executables().elasticsearchSetupPasswords.run(sh, "auto --batch", null);
62+
Shell.Result result = installation.executables().setupPasswordsTool.run("auto --batch", null);
6363
Map<String, String> userpasses = parseUsersAndPasswords(result.stdout);
6464
for (Map.Entry<String, String> userpass : userpasses.entrySet()) {
6565
String response = ServerUtils.makeRequest(Request.Get("http://localhost:9200"), userpass.getKey(), userpass.getValue());
@@ -106,7 +106,7 @@ public void test30AddBootstrapPassword() throws Exception {
106106
});
107107
}
108108

109-
installation.executables().elasticsearchKeystore.run(sh, "add --stdin bootstrap.password", BOOTSTRAP_PASSWORD);
109+
installation.executables().keystoreTool.run("add --stdin bootstrap.password", BOOTSTRAP_PASSWORD);
110110

111111
assertWhileRunning(() -> {
112112
String response = ServerUtils.makeRequest(
@@ -119,7 +119,7 @@ public void test30AddBootstrapPassword() throws Exception {
119119
public void test40GeneratePasswordsBootstrapAlreadySet() throws Exception {
120120
assertWhileRunning(() -> {
121121

122-
Shell.Result result = installation.executables().elasticsearchSetupPasswords.run(sh, "auto --batch", null);
122+
Shell.Result result = installation.executables().setupPasswordsTool.run("auto --batch", null);
123123
Map<String, String> userpasses = parseUsersAndPasswords(result.stdout);
124124
assertThat(userpasses, hasKey("elastic"));
125125
for (Map.Entry<String, String> userpass : userpasses.entrySet()) {

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -50,9 +50,9 @@ public static void filterDistros() {
5050

5151
public void test10Install() throws Exception {
5252
assertRemoved(distribution());
53-
installation = installPackage(distribution());
53+
installation = installPackage(sh, distribution());
5454
assertInstalled(distribution());
55-
verifyPackageInstallation(installation, distribution(), newShell());
55+
verifyPackageInstallation(installation, distribution(), sh);
5656
}
5757

5858
public void test20Remove() throws Exception {
@@ -71,11 +71,11 @@ public void test20Remove() throws Exception {
7171
public void test30PreserveConfig() throws Exception {
7272
final Shell sh = new Shell();
7373

74-
installation = installPackage(distribution());
74+
installation = installPackage(sh, distribution());
7575
assertInstalled(distribution());
76-
verifyPackageInstallation(installation, distribution(), newShell());
76+
verifyPackageInstallation(installation, distribution(), sh);
7777

78-
sh.run("echo foobar | " + installation.executables().elasticsearchKeystore + " add --stdin foo.bar");
78+
sh.run("echo foobar | " + installation.executables().keystoreTool + " add --stdin foo.bar");
7979
Stream.of(
8080
"elasticsearch.yml",
8181
"jvm.options",

0 commit comments

Comments
 (0)