Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,17 @@ tasks.named("javaRestTest").configure {
}

testClusters.matching { it.name == "javaRestTest" }.configureEach {
systemProperty "die.with.dignity.test", "whatever"
setting 'xpack.security.enabled', 'true'
user username: 'admin', password: 'admin-password', role: 'superuser'
systemProperty "die.with.dignity.test", "true"
}

tasks.named("test").configure {
enabled = false
}

tasks.named("yamlRestTest").configure {
enabled = false
}

tasks.named('splitPackagesAudit').configure {
// these should be moved to an actual package, not the root package
ignoreClasses 'org.elasticsearch.DieWithDignityPlugin',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,6 @@

import org.elasticsearch.client.Request;
import org.elasticsearch.core.PathUtils;
import org.elasticsearch.common.settings.SecureString;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.util.concurrent.ThreadContext;
import org.elasticsearch.test.rest.ESRestTestCase;

import java.io.BufferedReader;
Expand All @@ -30,6 +27,24 @@
public class DieWithDignityIT extends ESRestTestCase {

public void testDieWithDignity() throws Exception {
// there should be an Elasticsearch process running with the die.with.dignity.test system property
assertBusy(() -> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this stall necessary? Doesn't the rest test infrastructure not run the tests until ES is up?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The simple explanation is that I copy/pasted the code below that checks that the die-with-dignity instance has been killed, with the intention of modifying it to remove the busy assert, and change it to a check that the die-with-dignity instance is up. Except, I forgot to remove the busy assert. 🤦‍♀️

I've removed this now in 3dd8db6.

final String jpsPath = PathUtils.get(System.getProperty("runtime.java.home"), "bin/jps").toString();
final Process process = new ProcessBuilder().command(jpsPath, "-v").start();

boolean found = false;
try (InputStream is = process.getInputStream(); BufferedReader in = new BufferedReader(new InputStreamReader(is, "UTF-8"))) {
String line;
while ((line = in.readLine()) != null) {
if (line.contains("-Ddie.with.dignity.test=true")) {
found = true;
break;
}
}
}
assertTrue(found);
});

expectThrows(IOException.class, () -> client().performRequest(new Request("GET", "/_die_with_dignity")));

// the Elasticsearch process should die and disappear from the output of jps
Expand All @@ -40,7 +55,7 @@ public void testDieWithDignity() throws Exception {
try (InputStream is = process.getInputStream(); BufferedReader in = new BufferedReader(new InputStreamReader(is, "UTF-8"))) {
String line;
while ((line = in.readLine()) != null) {
assertThat(line, line, not(containsString("-Ddie.with.dignity.test")));
assertThat(line, line, not(containsString("-Ddie.with.dignity.test=true")));
}
}
});
Expand Down Expand Up @@ -99,16 +114,4 @@ protected boolean preserveClusterUponCompletion() {
return true;
}

@Override
protected final Settings restClientSettings() {
String token = basicAuthHeaderValue("admin", new SecureString("admin-password".toCharArray()));
return Settings.builder()
.put(super.restClientSettings())
.put(ThreadContext.PREFIX + ".Authorization", token)
// increase the timeout here to 90 seconds to handle long waits for a green
// cluster health. the waits for green need to be longer than a minute to
// account for delayed shards
.put(ESRestTestCase.CLIENT_SOCKET_TIMEOUT, "1s")
.build();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,6 @@

public class DieWithDignityPlugin extends Plugin implements ActionPlugin {

public DieWithDignityPlugin() {
assert System.getProperty("die.with.dignity.test") != null : "test should pass the `die.with.dignity.test` property";
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because this module is now installed in all snapshot builds, this plugin is loaded in all other test suites where this system property would not be configured. This is why this assertion has to be dropped.

}

@Override
public List<RestHandler> getRestHandlers(
final Settings settings,
Expand Down