From 45b5ab24fec2fae36e0eb205ccd00d8040ed9c7e Mon Sep 17 00:00:00 2001
From: Ryan Ernst <ryan@iernst.net>
Date: Mon, 7 Mar 2016 12:42:15 -0800
Subject: [PATCH] Moved MockTerminal and created a base test case for cli
commands.
---
.../java/org/elasticsearch/cli/Command.java | 6 +-
.../common/cli/CheckFileCommand.java | 138 ------------------
.../common/cli/TerminalTests.java | 7 +-
.../logging/LoggingConfigurationTests.java | 2 +-
.../InternalSettingsPreparerTests.java | 2 +-
.../elasticsearch/plugins/PluginCliTests.java | 4 +-
.../bootstrap/BootstrapCliParserTests.java | 10 +-
.../plugins/InstallPluginCommandTests.java | 7 +-
.../plugins/ListPluginsCommandTests.java | 7 +-
.../plugins/RemovePluginCommandTests.java | 4 +-
.../elasticsearch/cli/CommandTestCase.java | 48 ++++++
.../{common => }/cli/MockTerminal.java | 10 +-
.../common/cli/CliToolTestCase.java | 1 +
13 files changed, 75 insertions(+), 171 deletions(-)
delete mode 100644 core/src/main/java/org/elasticsearch/common/cli/CheckFileCommand.java
create mode 100644 test/framework/src/main/java/org/elasticsearch/cli/CommandTestCase.java
rename test/framework/src/main/java/org/elasticsearch/{common => }/cli/MockTerminal.java (92%)
diff --git a/core/src/main/java/org/elasticsearch/cli/Command.java b/core/src/main/java/org/elasticsearch/cli/Command.java
index d688347099d..6e57905b5b2 100644
--- a/core/src/main/java/org/elasticsearch/cli/Command.java
+++ b/core/src/main/java/org/elasticsearch/cli/Command.java
@@ -49,14 +49,14 @@ public abstract class Command {
}
/** Parses options for this command from args and executes it. */
- public final int main(String[] args, Terminal terminal) throws Exception {
+ protected final int main(String[] args, Terminal terminal) throws Exception {
final OptionSet options;
try {
options = parser.parse(args);
} catch (OptionException e) {
printHelp(terminal);
- terminal.println("ERROR: " + e.getMessage());
+ terminal.println(Terminal.Verbosity.SILENT, "ERROR: " + e.getMessage());
return ExitCodes.USAGE;
}
@@ -69,7 +69,7 @@ public abstract class Command {
if (options.has(verboseOption)) {
// mutually exclusive, we can remove this with jopt-simple 5.0, which natively supports it
printHelp(terminal);
- terminal.println("ERROR: Cannot specify -s and -v together");
+ terminal.println(Terminal.Verbosity.SILENT, "ERROR: Cannot specify -s and -v together");
return ExitCodes.USAGE;
}
terminal.setVerbosity(Terminal.Verbosity.SILENT);
diff --git a/core/src/main/java/org/elasticsearch/common/cli/CheckFileCommand.java b/core/src/main/java/org/elasticsearch/common/cli/CheckFileCommand.java
deleted file mode 100644
index e2fcbe89df8..00000000000
--- a/core/src/main/java/org/elasticsearch/common/cli/CheckFileCommand.java
+++ /dev/null
@@ -1,138 +0,0 @@
-/*
- * 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.common.cli;
-
-import org.elasticsearch.common.settings.Settings;
-import org.elasticsearch.env.Environment;
-
-import java.io.IOException;
-import java.nio.file.Files;
-import java.nio.file.Path;
-import java.nio.file.attribute.PosixFileAttributeView;
-import java.nio.file.attribute.PosixFileAttributes;
-import java.nio.file.attribute.PosixFilePermission;
-import java.nio.file.attribute.PosixFilePermissions;
-import java.util.HashMap;
-import java.util.Map;
-import java.util.Set;
-
-/**
- * A helper command that checks if configured paths have been changed when running a CLI command.
- * It is only executed in case of specified paths by the command and if the paths underlying filesystem
- * supports posix permissions.
- *
- * If this is the case, a warn message is issued whenever an owner, a group or the file permissions is changed by
- * the command being executed and not configured back to its prior state, which should be the task of the command
- * being executed.
- *
- */
-public abstract class CheckFileCommand extends CliTool.Command {
-
- public CheckFileCommand(Terminal terminal) {
- super(terminal);
- }
-
- /**
- * abstract method, which should implement the same logic as CliTool.Command.execute(), but is wrapped
- */
- public abstract CliTool.ExitStatus doExecute(Settings settings, Environment env) throws Exception;
-
- /**
- * Returns the array of paths, that should be checked if the permissions, user or groups have changed
- * before and after execution of the command
- *
- */
- protected abstract Path[] pathsForPermissionsCheck(Settings settings, Environment env) throws Exception;
-
- @Override
- public CliTool.ExitStatus execute(Settings settings, Environment env) throws Exception {
- Path[] paths = pathsForPermissionsCheck(settings, env);
-
- if (paths == null || paths.length == 0) {
- return doExecute(settings, env);
- }
-
- Map<Path, Set<PosixFilePermission>> permissions = new HashMap<>(paths.length);
- Map<Path, String> owners = new HashMap<>(paths.length);
- Map<Path, String> groups = new HashMap<>(paths.length);
-
- if (paths != null && paths.length > 0) {
- for (Path path : paths) {
- try {
- boolean supportsPosixPermissions = Environment.getFileStore(path).supportsFileAttributeView(PosixFileAttributeView.class);
- if (supportsPosixPermissions) {
- PosixFileAttributes attributes = Files.readAttributes(path, PosixFileAttributes.class);
- permissions.put(path, attributes.permissions());
- owners.put(path, attributes.owner().getName());
- groups.put(path, attributes.group().getName());
- }
- } catch (IOException e) {
- // silently swallow if not supported, no need to log things
- }
- }
- }
-
- CliTool.ExitStatus status = doExecute(settings, env);
-
- // check if permissions differ
- for (Map.Entry<Path, Set<PosixFilePermission>> entry : permissions.entrySet()) {
- if (!Files.exists(entry.getKey())) {
- continue;
- }
-
- Set<PosixFilePermission> permissionsBeforeWrite = entry.getValue();
- Set<PosixFilePermission> permissionsAfterWrite = Files.getPosixFilePermissions(entry.getKey());
- if (!permissionsBeforeWrite.equals(permissionsAfterWrite)) {
- terminal.println(Terminal.Verbosity.SILENT, "WARNING: The file permissions of [" + entry.getKey() + "] have changed "
- + "from [" + PosixFilePermissions.toString(permissionsBeforeWrite) + "] "
- + "to [" + PosixFilePermissions.toString(permissionsAfterWrite) + "]");
- terminal.println(Terminal.Verbosity.SILENT, "Please ensure that the user account running Elasticsearch has read access to this file!");
- }
- }
-
- // check if owner differs
- for (Map.Entry<Path, String> entry : owners.entrySet()) {
- if (!Files.exists(entry.getKey())) {
- continue;
- }
-
- String ownerBeforeWrite = entry.getValue();
- String ownerAfterWrite = Files.getOwner(entry.getKey()).getName();
- if (!ownerAfterWrite.equals(ownerBeforeWrite)) {
- terminal.println(Terminal.Verbosity.SILENT, "WARNING: Owner of file [" + entry.getKey() + "] used to be [" + ownerBeforeWrite + "], but now is [" + ownerAfterWrite + "]");
- }
- }
-
- // check if group differs
- for (Map.Entry<Path, String> entry : groups.entrySet()) {
- if (!Files.exists(entry.getKey())) {
- continue;
- }
-
- String groupBeforeWrite = entry.getValue();
- String groupAfterWrite = Files.readAttributes(entry.getKey(), PosixFileAttributes.class).group().getName();
- if (!groupAfterWrite.equals(groupBeforeWrite)) {
- terminal.println(Terminal.Verbosity.SILENT, "WARNING: Group of file [" + entry.getKey() + "] used to be [" + groupBeforeWrite + "], but now is [" + groupAfterWrite + "]");
- }
- }
-
- return status;
- }
-}
diff --git a/core/src/test/java/org/elasticsearch/common/cli/TerminalTests.java b/core/src/test/java/org/elasticsearch/common/cli/TerminalTests.java
index deb64e906b4..12fc4cb77e4 100644
--- a/core/src/test/java/org/elasticsearch/common/cli/TerminalTests.java
+++ b/core/src/test/java/org/elasticsearch/common/cli/TerminalTests.java
@@ -19,7 +19,10 @@
package org.elasticsearch.common.cli;
-public class TerminalTests extends CliToolTestCase {
+import org.elasticsearch.cli.MockTerminal;
+import org.elasticsearch.test.ESTestCase;
+
+public class TerminalTests extends ESTestCase {
public void testVerbosity() throws Exception {
MockTerminal terminal = new MockTerminal();
terminal.setVerbosity(Terminal.Verbosity.SILENT);
@@ -48,7 +51,7 @@ public class TerminalTests extends CliToolTestCase {
logTerminal.println(verbosity, text);
String output = logTerminal.getOutput();
assertTrue(output, output.contains(text));
- logTerminal.resetOutput();
+ logTerminal.reset();
}
private void assertNotPrinted(MockTerminal logTerminal, Terminal.Verbosity verbosity, String text) throws Exception {
diff --git a/core/src/test/java/org/elasticsearch/common/logging/LoggingConfigurationTests.java b/core/src/test/java/org/elasticsearch/common/logging/LoggingConfigurationTests.java
index 0cca19d33bf..5c812cca0a7 100644
--- a/core/src/test/java/org/elasticsearch/common/logging/LoggingConfigurationTests.java
+++ b/core/src/test/java/org/elasticsearch/common/logging/LoggingConfigurationTests.java
@@ -27,7 +27,7 @@ import java.util.Arrays;
import org.apache.log4j.Appender;
import org.apache.log4j.Logger;
-import org.elasticsearch.common.cli.MockTerminal;
+import org.elasticsearch.cli.MockTerminal;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.env.Environment;
import org.elasticsearch.node.internal.InternalSettingsPreparer;
diff --git a/core/src/test/java/org/elasticsearch/node/internal/InternalSettingsPreparerTests.java b/core/src/test/java/org/elasticsearch/node/internal/InternalSettingsPreparerTests.java
index 33876ef61ad..c979b2f4013 100644
--- a/core/src/test/java/org/elasticsearch/node/internal/InternalSettingsPreparerTests.java
+++ b/core/src/test/java/org/elasticsearch/node/internal/InternalSettingsPreparerTests.java
@@ -24,7 +24,7 @@ import java.io.InputStream;
import java.nio.file.Files;
import java.nio.file.Path;
-import org.elasticsearch.common.cli.MockTerminal;
+import org.elasticsearch.cli.MockTerminal;
import org.elasticsearch.cluster.ClusterName;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.settings.SettingsException;
diff --git a/core/src/test/java/org/elasticsearch/plugins/PluginCliTests.java b/core/src/test/java/org/elasticsearch/plugins/PluginCliTests.java
index 7a4590f8e25..708cefd91b2 100644
--- a/core/src/test/java/org/elasticsearch/plugins/PluginCliTests.java
+++ b/core/src/test/java/org/elasticsearch/plugins/PluginCliTests.java
@@ -20,10 +20,8 @@
package org.elasticsearch.plugins;
import org.elasticsearch.common.cli.CliToolTestCase;
-import org.elasticsearch.common.cli.MockTerminal;
+import org.elasticsearch.cli.MockTerminal;
-import static org.elasticsearch.common.cli.CliTool.ExitStatus.OK_AND_EXIT;
-import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.hasItem;
import static org.hamcrest.Matchers.is;
diff --git a/qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/BootstrapCliParserTests.java b/qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/BootstrapCliParserTests.java
index 12b5ab9eb39..18d6e0ac3c9 100644
--- a/qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/BootstrapCliParserTests.java
+++ b/qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/BootstrapCliParserTests.java
@@ -25,7 +25,7 @@ import org.elasticsearch.common.SuppressForbidden;
import org.elasticsearch.common.cli.CliTool.ExitStatus;
import org.elasticsearch.common.cli.CliToolTestCase;
import org.elasticsearch.cli.UserError;
-import org.elasticsearch.common.cli.MockTerminal;
+import org.elasticsearch.cli.MockTerminal;
import org.elasticsearch.common.collect.Tuple;
import org.elasticsearch.monitor.jvm.JvmInfo;
import org.junit.After;
@@ -89,7 +89,7 @@ public class BootstrapCliParserTests extends CliToolTestCase {
assertTrue(output, output.contains(Build.CURRENT.date()));
assertTrue(output, output.contains(JvmInfo.jvmInfo().version()));
- terminal.resetOutput();
+ terminal.reset();
parser = new BootstrapCLIParser(terminal);
status = parser.execute(args("start --version"));
assertStatus(status, OK_AND_EXIT);
@@ -177,7 +177,7 @@ public class BootstrapCliParserTests extends CliToolTestCase {
String output = terminal.getOutput();
assertTrue(output, output.contains("Parameter [network.host] needs value"));
- terminal.resetOutput();
+ terminal.reset();
status = parser.execute(args("start --network.host --foo"));
assertStatus(status, USAGE);
output = terminal.getOutput();
@@ -194,7 +194,7 @@ public class BootstrapCliParserTests extends CliToolTestCase {
assertTrue(output, output.contains("Unrecognized option: --unknown-param"));
// single dash in extra params
- terminal.resetOutput();
+ terminal.reset();
parser = new BootstrapCLIParser(terminal);
status = parser.execute(args("start -network.host 127.0.0.1"));
assertStatus(status, USAGE);
@@ -228,7 +228,7 @@ public class BootstrapCliParserTests extends CliToolTestCase {
tuples.add(new Tuple<>("-h", "elasticsearch.help"));
for (Tuple<String, String> tuple : tuples) {
- terminal.resetOutput();
+ terminal.reset();
BootstrapCLIParser parser = new BootstrapCLIParser(terminal);
ExitStatus status = parser.execute(args(tuple.v1()));
assertStatus(status, OK_AND_EXIT);
diff --git a/qa/evil-tests/src/test/java/org/elasticsearch/plugins/InstallPluginCommandTests.java b/qa/evil-tests/src/test/java/org/elasticsearch/plugins/InstallPluginCommandTests.java
index 342b579a175..514090d9869 100644
--- a/qa/evil-tests/src/test/java/org/elasticsearch/plugins/InstallPluginCommandTests.java
+++ b/qa/evil-tests/src/test/java/org/elasticsearch/plugins/InstallPluginCommandTests.java
@@ -36,7 +36,6 @@ import java.nio.file.attribute.BasicFileAttributes;
import java.nio.file.attribute.PosixFileAttributeView;
import java.nio.file.attribute.PosixFileAttributes;
import java.nio.file.attribute.PosixFilePermission;
-import java.util.Arrays;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
@@ -45,11 +44,7 @@ import java.util.zip.ZipOutputStream;
import org.apache.lucene.util.LuceneTestCase;
import org.elasticsearch.Version;
-import org.elasticsearch.cli.ExitCodes;
-import org.elasticsearch.common.cli.CliTool;
-import org.elasticsearch.common.cli.CliToolTestCase;
-import org.elasticsearch.common.cli.MockTerminal;
-import org.elasticsearch.common.cli.Terminal;
+import org.elasticsearch.cli.MockTerminal;
import org.elasticsearch.cli.UserError;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.env.Environment;
diff --git a/qa/evil-tests/src/test/java/org/elasticsearch/plugins/ListPluginsCommandTests.java b/qa/evil-tests/src/test/java/org/elasticsearch/plugins/ListPluginsCommandTests.java
index aed1696898a..cbdd031dea1 100644
--- a/qa/evil-tests/src/test/java/org/elasticsearch/plugins/ListPluginsCommandTests.java
+++ b/qa/evil-tests/src/test/java/org/elasticsearch/plugins/ListPluginsCommandTests.java
@@ -22,15 +22,10 @@ package org.elasticsearch.plugins;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
-import java.util.Collections;
-import java.util.List;
import org.apache.lucene.util.LuceneTestCase;
import org.elasticsearch.cli.ExitCodes;
-import org.elasticsearch.common.cli.CliTool;
-import org.elasticsearch.common.cli.CliToolTestCase;
-import org.elasticsearch.common.cli.MockTerminal;
-import org.elasticsearch.common.cli.Terminal;
+import org.elasticsearch.cli.MockTerminal;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.env.Environment;
import org.elasticsearch.test.ESTestCase;
diff --git a/qa/evil-tests/src/test/java/org/elasticsearch/plugins/RemovePluginCommandTests.java b/qa/evil-tests/src/test/java/org/elasticsearch/plugins/RemovePluginCommandTests.java
index a678d4f25f6..d9d5661b834 100644
--- a/qa/evil-tests/src/test/java/org/elasticsearch/plugins/RemovePluginCommandTests.java
+++ b/qa/evil-tests/src/test/java/org/elasticsearch/plugins/RemovePluginCommandTests.java
@@ -26,9 +26,7 @@ import java.nio.file.Path;
import org.apache.lucene.util.LuceneTestCase;
import org.elasticsearch.cli.UserError;
-import org.elasticsearch.common.cli.CliToolTestCase;
-import org.elasticsearch.common.cli.MockTerminal;
-import org.elasticsearch.common.cli.Terminal;
+import org.elasticsearch.cli.MockTerminal;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.env.Environment;
import org.elasticsearch.test.ESTestCase;
diff --git a/test/framework/src/main/java/org/elasticsearch/cli/CommandTestCase.java b/test/framework/src/main/java/org/elasticsearch/cli/CommandTestCase.java
new file mode 100644
index 00000000000..3af25509adb
--- /dev/null
+++ b/test/framework/src/main/java/org/elasticsearch/cli/CommandTestCase.java
@@ -0,0 +1,48 @@
+/*
+ * 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.elasticsearch.common.cli.Terminal;
+import org.elasticsearch.test.ESTestCase;
+import org.junit.Before;
+
+/**
+ * A base test case for cli tools.
+ */
+public abstract class CommandTestCase extends ESTestCase {
+
+ protected final MockTerminal terminal = new MockTerminal();
+
+ @Before
+ public void resetTerminal() {
+ terminal.reset();
+ terminal.setVerbosity(Terminal.Verbosity.NORMAL);
+ }
+
+ protected abstract Command newCommand();
+
+ public String execute(String... args) throws Exception {
+ Command command = newCommand();
+ OptionSet options = command.parser.parse(args);
+ command.execute(terminal, options);
+ return terminal.getOutput();
+ }
+}
diff --git a/test/framework/src/main/java/org/elasticsearch/common/cli/MockTerminal.java b/test/framework/src/main/java/org/elasticsearch/cli/MockTerminal.java
similarity index 92%
rename from test/framework/src/main/java/org/elasticsearch/common/cli/MockTerminal.java
rename to test/framework/src/main/java/org/elasticsearch/cli/MockTerminal.java
index 3b2903b3fab..bb01369ac50 100644
--- a/test/framework/src/main/java/org/elasticsearch/common/cli/MockTerminal.java
+++ b/test/framework/src/main/java/org/elasticsearch/cli/MockTerminal.java
@@ -17,7 +17,7 @@
* under the License.
*/
-package org.elasticsearch.common.cli;
+package org.elasticsearch.cli;
import java.io.ByteArrayOutputStream;
import java.io.OutputStreamWriter;
@@ -27,6 +27,8 @@ import java.nio.charset.StandardCharsets;
import java.util.ArrayDeque;
import java.util.Deque;
+import org.elasticsearch.common.cli.Terminal;
+
/**
* A terminal for tests which captures all output, and
* can be plugged with fake input.
@@ -78,8 +80,10 @@ public class MockTerminal extends Terminal {
return buffer.toString("UTF-8");
}
- /** Wipes the output. */
- public void resetOutput() {
+ /** Wipes the input and output. */
+ public void reset() {
buffer.reset();
+ textInput.clear();
+ secretInput.clear();
}
}
diff --git a/test/framework/src/main/java/org/elasticsearch/common/cli/CliToolTestCase.java b/test/framework/src/main/java/org/elasticsearch/common/cli/CliToolTestCase.java
index 9debf4b8f33..330758223a5 100644
--- a/test/framework/src/main/java/org/elasticsearch/common/cli/CliToolTestCase.java
+++ b/test/framework/src/main/java/org/elasticsearch/common/cli/CliToolTestCase.java
@@ -21,6 +21,7 @@ package org.elasticsearch.common.cli;
import java.io.IOException;
+import org.elasticsearch.cli.MockTerminal;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.SuppressForbidden;
import org.elasticsearch.test.ESTestCase;