Migrate setup passwords packaging test from bats #49337
Conversation
This commit moves the packaging tests for elasticsearch-setup-passwords to java from bats. The change also enables future tests to enable security in Elasticsearch and automatically have waitForElasticsearch work correctly, at least to the same extent it worked in bats, by waiting on the ES port instead of health check. relates elastic#46005
rjernst
added
>test
|
Pinging @elastic/es-core-infra (:Core/Infra/Packaging) |
| try (Socket s = new Socket(InetAddress.getLoopbackAddress(), 9200)) { | ||
| return; | ||
| } catch (IOException e) { | ||
| try { | ||
| // ignore, only want to establish a connection | ||
| Thread.sleep(1000); | ||
| } catch (InterruptedException interrupted) { | ||
| Thread.currentThread().interrupt(); | ||
| return; | ||
| } | ||
| } |
There was a problem hiding this comment.
this is just a preference, but I think it is easier to read like this:
try (Socket s = new Socket(InetAddress.getLoopbackAddress(), 9200)) {
return;
} catch (IOException e) {
// ignore, only want to establish a connection
}
try {
Thread.sleep(1000);
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
return;
}| return sh.run(path.toString() + " " + args); | ||
| } | ||
|
|
||
| protected void assertWhileRunning(Platforms.PlatformAction assertions) throws Exception { |
There was a problem hiding this comment.
I think it would be helpful to have some javadocs on this method indicating the method runs elasticsearch, performs the assertions, and then stops elasticsearch
| // nothing, "installing" docker image is running it | ||
| } | ||
|
|
||
| } catch (Exception e ){ |
There was a problem hiding this comment.
| } catch (Exception e ){ | |
| } catch (Exception e){ |
|
@jaymode I addressed your comments, and also migrated the bootstrap password tests as those were tightly couple with setup-passwords behavior. |
| public void test30AddBootstrapPassword() throws Exception { | ||
| installation.executables().elasticsearchKeystore.run(sh, "add --stdin bootstrap.password", BOOTSTRAP_PASSWORD); | ||
|
|
||
| FileUtils.rm(installation.data); // wipe auto generated passwords from previous test |
There was a problem hiding this comment.
minor nit, can this be the first line of the test?
|
@elasticmachine run elasticsearch-ci/bwc |
|
@elasticmachine run elasticsearch-ci/packaging-matrix |
| dataFiles.forEach(file -> { | ||
| if (distribution.platform != Distribution.Platform.WINDOWS) { | ||
| FileUtils.rm(file); | ||
| } |
There was a problem hiding this comment.
should we have an else here or a return?
There was a problem hiding this comment.
oops, yes, i meant for a return.
|
@elasticmachine run elasticsearch-ci/packaging-matrix |
This commit moves the packaging tests for elasticsearch-setup-passwords to java from bats. The change also enables future tests to enable security in Elasticsearch and automatically have waitForElasticsearch work correctly, at least to the same extent it worked in bats, by waiting on the ES port instead of health check. relates #46005
This commit moves the packaging tests for elasticsearch-setup-passwords
to java from bats. The change also enables future tests to enable
security in Elasticsearch and automatically have waitForElasticsearch
work correctly, at least to the same extent it worked in bats, by
waiting on the ES port instead of health check.
relates #46005