honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix environment-aware command tests 

This commit fixes tests for environment-aware commands. A previous
change added a check that es.path.conf is not null. The problem is that
this system property is not being set in tests so this check trips every
single time. To fix this, we move the check into a method that can be
overridden, and then override this method in relevant places in tests to
avoid having to set the property in tests. We also add a test that this
check works as expected.
This commit is contained in:
Jason Tedor 2017-07-28 14:08:24 +09:00
parent 7d2d6bd752
commit 1492ccd7ae
10 changed files with 114 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
core/src/main/java/org/elasticsearch/cli/EnvironmentAwareCommand.java
View File

@ -66,12 +66,16 @@ public abstract class EnvironmentAwareCommand extends Command {
putSystemPropertyIfSettingIsMissing(settings, "path.home", "es.path.home");
putSystemPropertyIfSettingIsMissing(settings, "path.logs", "es.path.logs");
final String pathConf = System.getProperty("es.path.conf");
if (pathConf == null) {
execute(terminal, options, createEnv(terminal, settings));
}
/** Create an {@link Environment} for the command to use. Overrideable for tests. */
protected Environment createEnv(final Terminal terminal, final Map<String, String> settings) throws UserException {
final String esPathConf = System.getProperty("es.path.conf");
if (esPathConf == null) {
throw new UserException(ExitCodes.CONFIG, "the system property es.path.conf must be set");
}
execute(terminal, options, createEnv(terminal, settings, getConfigPath(pathConf)));
return InternalSettingsPreparer.prepareEnvironment(Settings.EMPTY, terminal, settings, getConfigPath(esPathConf));
}
@SuppressForbidden(reason = "need path to construct environment")
@ -79,13 +83,8 @@ public abstract class EnvironmentAwareCommand extends Command {
return Paths.get(pathConf);
}
/** Create an {@link Environment} for the command to use. Overrideable for tests. */
protected Environment createEnv(Terminal terminal, Map<String, String> settings, Path configPath) {
return InternalSettingsPreparer.prepareEnvironment(Settings.EMPTY, terminal, settings, configPath);
}
/** Ensure the given setting exists, reading it from system properties if not already set. */
protected static void putSystemPropertyIfSettingIsMissing(final Map<String, String> settings, final String setting, final String key) {
private static void putSystemPropertyIfSettingIsMissing(final Map<String, String> settings, final String setting, final String key) {
final String value = System.getProperty(key);
if (value != null) {
if (settings.containsKey(setting)) {

2
core/src/test/java/org/elasticsearch/common/settings/AddFileKeyStoreCommandTests.java
View File

@ -37,7 +37,7 @@ public class AddFileKeyStoreCommandTests extends KeyStoreCommandTestCase {
protected Command newCommand() {
return new AddFileKeyStoreCommand() {
@Override
protected Environment createEnv(Terminal terminal, Map<String, String> settings, Path configPath) {
protected Environment createEnv(Terminal terminal, Map<String, String> settings) throws UserException {
return env;
}
};

3
core/src/test/java/org/elasticsearch/common/settings/AddStringKeyStoreCommandTests.java
View File

@ -22,7 +22,6 @@ package org.elasticsearch.common.settings;
import java.io.ByteArrayInputStream;
import java.io.InputStream;
import java.nio.charset.StandardCharsets;
import java.nio.file.Path;
import java.util.Map;
import org.elasticsearch.cli.Command;
@ -40,7 +39,7 @@ public class AddStringKeyStoreCommandTests extends KeyStoreCommandTestCase {
protected Command newCommand() {
return new AddStringKeyStoreCommand() {
@Override
protected Environment createEnv(Terminal terminal, Map<String, String> settings, Path configPath) {
protected Environment createEnv(Terminal terminal, Map<String, String> settings) throws UserException {
return env;
}
@Override

3
core/src/test/java/org/elasticsearch/common/settings/CreateKeyStoreCommandTests.java
View File

@ -26,6 +26,7 @@ import java.util.Map;
import org.elasticsearch.cli.Command;
import org.elasticsearch.cli.Terminal;
import org.elasticsearch.cli.UserException;
import org.elasticsearch.env.Environment;
public class CreateKeyStoreCommandTests extends KeyStoreCommandTestCase {
@ -34,7 +35,7 @@ public class CreateKeyStoreCommandTests extends KeyStoreCommandTestCase {
protected Command newCommand() {
return new CreateKeyStoreCommand() {
@Override
protected Environment createEnv(Terminal terminal, Map<String, String> settings, Path configPath) {
protected Environment createEnv(Terminal terminal, Map<String, String> settings) throws UserException {
return env;
}
};

3
core/src/test/java/org/elasticsearch/common/settings/ListKeyStoreCommandTests.java
View File

@ -19,7 +19,6 @@
package org.elasticsearch.common.settings;
import java.nio.file.Path;
import java.util.Map;
import org.elasticsearch.cli.Command;
@ -36,7 +35,7 @@ public class ListKeyStoreCommandTests extends KeyStoreCommandTestCase {
protected Command newCommand() {
return new ListKeyStoreCommand() {
@Override
protected Environment createEnv(Terminal terminal, Map<String, String> settings, Path configPath) {
protected Environment createEnv(Terminal terminal, Map<String, String> settings) throws UserException {
return env;
}
};

3
core/src/test/java/org/elasticsearch/common/settings/RemoveSettingKeyStoreCommandTests.java
View File

@ -25,7 +25,6 @@ import org.elasticsearch.cli.Terminal;
import org.elasticsearch.cli.UserException;
import org.elasticsearch.env.Environment;
import java.nio.file.Path;
import java.util.Map;
import java.util.Set;
@ -37,7 +36,7 @@ public class RemoveSettingKeyStoreCommandTests extends KeyStoreCommandTestCase {
protected Command newCommand() {
return new RemoveSettingKeyStoreCommand() {
@Override
protected Environment createEnv(Terminal terminal, Map<String, String> settings, Path configPath) {
protected Environment createEnv(Terminal terminal, Map<String, String> settings) throws UserException {
return env;
}
};

9
distribution/tools/plugin-cli/src/test/java/org/elasticsearch/plugins/ListPluginsCommandTests.java
View File

@ -26,12 +26,15 @@ import java.nio.file.NoSuchFileException;
import java.nio.file.Path;
import java.util.Arrays;
import java.util.Locale;
import java.util.Map;
import java.util.stream.Collectors;
import org.apache.lucene.util.LuceneTestCase;
import org.elasticsearch.Version;
import org.elasticsearch.cli.ExitCodes;
import org.elasticsearch.cli.MockTerminal;
import org.elasticsearch.cli.Terminal;
import org.elasticsearch.cli.UserException;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.env.Environment;
import org.elasticsearch.test.ESTestCase;
@ -64,6 +67,12 @@ public class ListPluginsCommandTests extends ESTestCase {
argsAndHome[args.length] = "-Epath.home=" + home;
MockTerminal terminal = new MockTerminal();
int status = new ListPluginsCommand() {
@Override
protected Environment createEnv(Terminal terminal, Map<String, String> settings) throws UserException {
final Settings realSettings = Settings.builder().put("path.home", home).put(settings).build();
return new Environment(realSettings, home.resolve("config"));
}
@Override
protected boolean addShutdownHook() {
return false;

23
distribution/tools/plugin-cli/src/test/java/org/elasticsearch/plugins/RemovePluginCommandTests.java
View File

@ -22,6 +22,7 @@ package org.elasticsearch.plugins;
import org.apache.lucene.util.LuceneTestCase;
import org.elasticsearch.cli.ExitCodes;
import org.elasticsearch.cli.MockTerminal;
import org.elasticsearch.cli.Terminal;
import org.elasticsearch.cli.UserException;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.env.Environment;
@ -34,6 +35,7 @@ import java.io.StringReader;
import java.nio.file.DirectoryStream;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.Map;
import static org.hamcrest.CoreMatchers.containsString;
import static org.hamcrest.CoreMatchers.not;
@ -45,6 +47,21 @@ public class RemovePluginCommandTests extends ESTestCase {
private Path home;
private Environment env;
static class MockRemovePluginCommand extends RemovePluginCommand {
final Environment env;
public MockRemovePluginCommand(final Environment env) {
this.env = env;
}
@Override
protected Environment createEnv(Terminal terminal, Map<String, String> settings) throws UserException {
return env;
}
}
@Override
@Before
public void setUp() throws Exception {
@ -62,7 +79,7 @@ public class RemovePluginCommandTests extends ESTestCase {
static MockTerminal removePlugin(String name, Path home, boolean purge) throws Exception {
Environment env = new Environment(Settings.builder().put("path.home", home).build());
MockTerminal terminal = new MockTerminal();
new RemovePluginCommand().execute(terminal, env, name, purge);
new MockRemovePluginCommand(env).execute(terminal, env, name, purge);
return terminal;
}
@ -175,8 +192,8 @@ public class RemovePluginCommandTests extends ESTestCase {
assertEquals("plugin [fake] not found; run 'elasticsearch-plugin list' to get list of installed plugins", e.getMessage());
MockTerminal terminal = new MockTerminal();
new RemovePluginCommand() {
@Override
new MockRemovePluginCommand(env) {
protected boolean addShutdownHook() {
return false;
}

65
qa/evil-tests/src/test/java/org/elasticsearch/cli/EvilEnvironmentAwareCommandTests.java Normal file
View File

@ -0,0 +1,65 @@
/*
* Licensed to Elasticsearch under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.elasticsearch.cli;
import joptsimple.OptionSet;
import org.apache.lucene.util.TestRuleRestoreSystemProperties;
import org.elasticsearch.common.SuppressForbidden;
import org.elasticsearch.env.Environment;
import org.elasticsearch.test.ESTestCase;
import org.junit.Rule;
import org.junit.rules.TestRule;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.hasToString;
public class EvilEnvironmentAwareCommandTests extends ESTestCase {
@Rule
public TestRule restoreSystemProperties = new TestRuleRestoreSystemProperties("es.path.conf");
public void testEsPathConfNotSet() throws Exception {
clearEsPathConf();
class TestEnvironmentAwareCommand extends EnvironmentAwareCommand {
private TestEnvironmentAwareCommand(String description) {
super(description);
}
@Override
protected void execute(Terminal terminal, OptionSet options, Environment env) throws Exception {
}
}
final TestEnvironmentAwareCommand command = new TestEnvironmentAwareCommand("test");
final UserException e =
expectThrows(UserException.class, () -> command.mainWithoutErrorHandling(new String[0], new MockTerminal()));
assertThat(e, hasToString(containsString("the system property es.path.conf must be set")));
}
@SuppressForbidden(reason = "clears system property es.path.conf as part of test setup")
private void clearEsPathConf() {
System.clearProperty("es.path.conf");
}
}

9
test/framework/src/main/java/org/elasticsearch/bootstrap/ESElasticsearchCliTestCase.java
View File

@ -21,6 +21,7 @@ package org.elasticsearch.bootstrap;
import org.elasticsearch.cli.MockTerminal;
import org.elasticsearch.cli.Terminal;
import org.elasticsearch.cli.UserException;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.env.Environment;
import org.elasticsearch.test.ESTestCase;
@ -43,16 +44,16 @@ abstract class ESElasticsearchCliTestCase extends ESTestCase {
final boolean expectedInit,
final Consumer<String> outputConsumer,
final InitConsumer initConsumer,
String... args) throws Exception {
final String... args) throws Exception {
final MockTerminal terminal = new MockTerminal();
Path home = createTempDir();
final Path home = createTempDir();
try {
final AtomicBoolean init = new AtomicBoolean();
final int status = Elasticsearch.main(args, new Elasticsearch() {
@Override
protected Environment createEnv(Terminal terminal, Map<String, String> settings, Path configPath) {
protected Environment createEnv(final Terminal terminal, final Map<String, String> settings) throws UserException {
final Settings realSettings = Settings.builder().put("path.home", home).put(settings).build();
return new Environment(realSettings, configPath);
return new Environment(realSettings, home.resolve("config"));
}
@Override
void init(final boolean daemonize, final Path pidFile, final boolean quiet, Environment initialEnv) {