Skip to content

Commit deba6ac

Browse files
rjernstrjernst
authored
Simplify running tools in packaging tests (#49665)
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 e97c38f commit deba6ac

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
}
@@ -202,7 +201,6 @@ public void test52BundledJdkRemoved() throws Exception {
202201

203202
public void test53JavaHomeWithSpecialCharacters() throws Exception {
204203
Platforms.onWindows(() -> {
205-
final Shell sh = new Shell();
206204
String javaPath = "C:\\Program Files (x86)\\java";
207205
try {
208206
// 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 {
228226
});
229227

230228
Platforms.onLinux(() -> {
231-
final Shell sh = newShell();
232229
// Create temporary directory with a space and link to real java home
233230
String testJavaHome = Paths.get("/tmp", "java home").toString();
234231
try {
@@ -256,12 +253,12 @@ public void test60AutoCreateKeystore() throws Exception {
256253

257254
final Installation.Executables bin = installation.executables();
258255
Platforms.onLinux(() -> {
259-
final Result result = sh.run("sudo -u " + ARCHIVE_OWNER + " " + bin.elasticsearchKeystore + " list");
256+
final Result result = sh.run("sudo -u " + ARCHIVE_OWNER + " " + bin.keystoreTool + " list");
260257
assertThat(result.stdout, containsString("keystore.seed"));
261258
});
262259

263260
Platforms.onWindows(() -> {
264-
final Result result = sh.run(bin.elasticsearchKeystore + " list");
261+
final Result result = sh.run(bin.keystoreTool + " list");
265262
assertThat(result.stdout, containsString("keystore.seed"));
266263
});
267264
}
@@ -339,11 +336,11 @@ public void test90SecurityCliPackaging() throws Exception {
339336
if (distribution().isDefault()) {
340337
assertTrue(Files.exists(installation.lib.resolve("tools").resolve("security-cli")));
341338
final Platforms.PlatformAction action = () -> {
342-
Result result = sh.run(bin.elasticsearchCertutil + " --help");
339+
Result result = sh.run(bin.certutilTool + " --help");
343340
assertThat(result.stdout, containsString("Simplifies certificate creation for use with the Elastic Stack"));
344341

345342
// Ensure that the exit code from the java command is passed back up through the shell script
346-
result = sh.runIgnoreExitCode(bin.elasticsearchCertutil + " invalid-command");
343+
result = sh.runIgnoreExitCode(bin.certutilTool + " invalid-command");
347344
assertThat(result.exitCode, is(not(0)));
348345
assertThat(result.stderr, containsString("Unknown command [invalid-command]"));
349346
};
@@ -358,7 +355,7 @@ public void test91ElasticsearchShardCliPackaging() throws Exception {
358355
final Installation.Executables bin = installation.executables();
359356

360357
Platforms.PlatformAction action = () -> {
361-
final Result result = sh.run(bin.elasticsearchShard + " -h");
358+
final Result result = sh.run(bin.shardTool + " -h");
362359
assertThat(result.stdout, containsString("A CLI tool to remove corrupted parts of unrecoverable shards"));
363360
};
364361

@@ -373,7 +370,7 @@ public void test92ElasticsearchNodeCliPackaging() throws Exception {
373370
final Installation.Executables bin = installation.executables();
374371

375372
Platforms.PlatformAction action = () -> {
376-
final Result result = sh.run(bin.elasticsearchNode + " -h");
373+
final Result result = sh.run(bin.nodeTool + " -h");
377374
assertThat(result.stdout,
378375
containsString("A CLI tool to do unsafe cluster and index manipulations on current node"));
379376
};
@@ -394,7 +391,7 @@ public void test93ElasticsearchNodeCustomDataPathAndNotEsHomeWorkDir() throws Ex
394391
startElasticsearch();
395392
Archives.stopElasticsearch(installation);
396393

397-
Result result = sh.run("echo y | " + installation.executables().elasticsearchNode + " unsafe-bootstrap");
394+
Result result = sh.run("echo y | " + installation.executables().nodeTool + " unsafe-bootstrap");
398395
assertThat(result.stdout, containsString("Master node was successfully bootstrapped"));
399396
}
400397

@@ -404,16 +401,16 @@ public void test94ElasticsearchNodeExecuteCliNotEsHomeWorkDir() throws Exception
404401
sh.setWorkingDirectory(getTempDir());
405402

406403
Platforms.PlatformAction action = () -> {
407-
Result result = sh.run(bin.elasticsearchCertutil+ " -h");
404+
Result result = sh.run(bin.certutilTool + " -h");
408405
assertThat(result.stdout,
409406
containsString("Simplifies certificate creation for use with the Elastic Stack"));
410-
result = sh.run(bin.elasticsearchSyskeygen+ " -h");
407+
result = sh.run(bin.syskeygenTool + " -h");
411408
assertThat(result.stdout,
412409
containsString("system key tool"));
413-
result = sh.run(bin.elasticsearchSetupPasswords+ " -h");
410+
result = sh.run(bin.setupPasswordsTool + " -h");
414411
assertThat(result.stdout,
415412
containsString("Sets the passwords for reserved users"));
416-
result = sh.run(bin.elasticsearchUsers+ " -h");
413+
result = sh.run(bin.usersTool + " -h");
417414
assertThat(result.stdout,
418415
containsString("Manages elasticsearch file users"));
419416
};

‎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
@@ -130,7 +130,7 @@ public void test011PresenceOfXpack() throws Exception {
130130
*/
131131
public void test020PluginsListWithNoPlugins() {
132132
final Installation.Executables bin = installation.executables();
133-
final Result r = sh.run(bin.elasticsearchPlugin + " list");
133+
final Result r = sh.run(bin.pluginTool + " list");
134134

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

151-
sh.run(bin.elasticsearchKeystore + " create");
151+
sh.run(bin.keystoreTool + " create");
152152

153-
final Result r = sh.run(bin.elasticsearchKeystore + " list");
153+
final Result r = sh.run(bin.keystoreTool + " list");
154154
assertThat(r.stdout, containsString("keystore.seed"));
155155
}
156156

@@ -165,7 +165,7 @@ public void test041AutoCreateKeystore() throws Exception {
165165
assertPermissionsAndOwnership(keystorePath, p660);
166166

167167
final Installation.Executables bin = installation.executables();
168-
final Result result = sh.run(bin.elasticsearchKeystore + " list");
168+
final Result result = sh.run(bin.keystoreTool + " list");
169169
assertThat(result.stdout, containsString("keystore.seed"));
170170
}
171171

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

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

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

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

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

433-
final Result result = sh.run(bin.elasticsearchNode + " -h");
433+
final Result result = sh.run(bin.nodeTool + " -h");
434434
assertThat(
435435
"Failed to find expected message about the elasticsearch-node CLI tool",
436436
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
}
@@ -302,7 +302,6 @@ public void test82SystemdMask() throws Exception {
302302
assumeTrue(isSystemd());
303303

304304
sh.run("systemctl mask systemd-sysctl.service");
305-
306305
install();
307306

308307
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)