Skip to content

Commit

Permalink
Reduce cli tests calling main directly (#85855)
Browse files Browse the repository at this point in the history 
The main method of Command is not normally called by cli tests. Instead
they call the execute helper which calls mainWithoutErrorHandling.
Sometimes, though, it is desirable to test the top level behavior. This
commit cleans up the base class tests to not call main directly. This
will help with future refactorings to change the signature of main, so
that less uses need to be changed.
  • Loading branch information
@rjernst
rjernst committed Apr 13, 2022
1 parent eb031ff commit 28a008a
Show file tree
Hide file tree
Showing 5 changed files with 113 additions and 177 deletions.
24 changes: 14 additions & 10 deletions distribution/tools/geoip-cli/src/test/java/org/elasticsearch/geoip/GeoIpCliTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,13 @@

package org.elasticsearch.geoip;

import joptsimple.OptionException;

import org.apache.commons.compress.archivers.tar.TarArchiveEntry;
import org.apache.commons.compress.archivers.tar.TarArchiveInputStream;
import org.apache.lucene.tests.util.LuceneTestCase;
import org.elasticsearch.cli.MockTerminal;
import org.elasticsearch.cli.Command;
import org.elasticsearch.cli.CommandTestCase;
import org.elasticsearch.xcontent.XContentParser;
import org.elasticsearch.xcontent.XContentParserConfiguration;
import org.elasticsearch.xcontent.XContentType;
Expand All @@ -34,7 +37,7 @@
import static org.hamcrest.Matchers.hasKey;

@LuceneTestCase.SuppressFileSystems(value = "ExtrasFS") // Don't randomly add 'extra' files to directory.
public class GeoIpCliTests extends LuceneTestCase {
public class GeoIpCliTests extends CommandTestCase {

private Path source;
private Path target;
Expand All @@ -46,16 +49,14 @@ public void setUp() throws Exception {
}

public void testNoSource() throws Exception {
MockTerminal terminal = new MockTerminal();
new GeoIpCli().main(new String[] {}, terminal);
assertThat(terminal.getErrorOutput(), containsString("Missing required option(s) [s/source]"));
var e = expectThrows(OptionException.class, () -> execute());
assertThat(e.getMessage(), containsString("Missing required option(s) [s/source]"));
}

public void testDifferentDirectories() throws Exception {
Map<String, byte[]> data = createTestFiles(source);

GeoIpCli cli = new GeoIpCli();
cli.main(new String[] { "-t", target.toAbsolutePath().toString(), "-s", source.toAbsolutePath().toString() }, new MockTerminal());
execute("-t", target.toAbsolutePath().toString(), "-s", source.toAbsolutePath().toString());

try (Stream<Path> list = Files.list(source)) {
List<String> files = list.map(p -> p.getFileName().toString()).collect(Collectors.toList());
Expand All @@ -73,9 +74,7 @@ public void testDifferentDirectories() throws Exception {

public void testSameDirectory() throws Exception {
Map<String, byte[]> data = createTestFiles(target);

GeoIpCli cli = new GeoIpCli();
cli.main(new String[] { "-s", target.toAbsolutePath().toString() }, new MockTerminal());
execute("-s", target.toAbsolutePath().toString());

try (Stream<Path> list = Files.list(target)) {
List<String> files = list.map(p -> p.getFileName().toString()).collect(Collectors.toList());
Expand Down Expand Up @@ -143,4 +142,9 @@ private Map<String, byte[]> createTestFiles(Path dir) throws IOException {

return data;
}

@Override
protected Command newCommand() {
return new GeoIpCli();
}
}
85 changes: 33 additions & 52 deletions ...tools/plugin-cli/src/test/java/org/elasticsearch/plugins/cli/ListPluginsCommandTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,13 @@

import org.apache.lucene.tests.util.LuceneTestCase;
import org.elasticsearch.Version;
import org.elasticsearch.cli.ExitCodes;
import org.elasticsearch.cli.MockTerminal;
import org.elasticsearch.cli.UserException;
import org.elasticsearch.cli.Command;
import org.elasticsearch.cli.CommandTestCase;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.env.Environment;
import org.elasticsearch.env.TestEnvironment;
import org.elasticsearch.plugins.PluginInfo;
import org.elasticsearch.plugins.PluginTestUtil;
import org.elasticsearch.test.ESTestCase;
import org.junit.Before;

import java.io.IOException;
Expand All @@ -30,47 +28,19 @@
import java.util.stream.Collectors;

@LuceneTestCase.SuppressFileSystems("*")
public class ListPluginsCommandTests extends ESTestCase {
public class ListPluginsCommandTests extends CommandTestCase {

private Path home;
private Environment env;

@Before
public void setUp() throws Exception {
super.setUp();
public void initEnv() throws Exception {
home = createTempDir();
Files.createDirectories(home.resolve("plugins"));
Settings settings = Settings.builder().put("path.home", home).build();
env = TestEnvironment.newEnvironment(settings);
}

static MockTerminal listPlugins(Path home) throws Exception {
return listPlugins(home, new String[0]);
}

static MockTerminal listPlugins(Path home, String[] args) throws Exception {
String[] argsAndHome = new String[args.length + 1];
System.arraycopy(args, 0, argsAndHome, 0, args.length);
argsAndHome[args.length] = "-Epath.home=" + home;
MockTerminal terminal = new MockTerminal();
int status = new ListPluginsCommand() {
@Override
protected Environment createEnv(Map<String, String> settings) throws UserException {
Settings.Builder builder = Settings.builder().put("path.home", home);
settings.forEach((k, v) -> builder.put(k, v));
final Settings realSettings = builder.build();
return new Environment(realSettings, home.resolve("config"));
}

@Override
protected boolean addShutdownHook() {
return false;
}
}.main(argsAndHome, terminal);
assertEquals(ExitCodes.OK, status);
return terminal;
}

private static String buildMultiline(String... args) {
return Arrays.stream(args).collect(Collectors.joining("\n", "", "\n"));
}
Expand Down Expand Up @@ -108,32 +78,31 @@ private static void buildFakePlugin(

public void testPluginsDirMissing() throws Exception {
Files.delete(env.pluginsFile());
IOException e = expectThrows(IOException.class, () -> listPlugins(home));
IOException e = expectThrows(IOException.class, () -> execute());
assertEquals("Plugins directory missing: " + env.pluginsFile(), e.getMessage());
}

public void testNoPlugins() throws Exception {
MockTerminal terminal = listPlugins(home);
execute();
assertTrue(terminal.getOutput(), terminal.getOutput().isEmpty());
}

public void testOnePlugin() throws Exception {
buildFakePlugin(env, "fake desc", "fake", "org.fake");
MockTerminal terminal = listPlugins(home);
execute();
assertEquals(buildMultiline("fake"), terminal.getOutput());
}

public void testTwoPlugins() throws Exception {
buildFakePlugin(env, "fake desc", "fake1", "org.fake");
buildFakePlugin(env, "fake desc 2", "fake2", "org.fake");
MockTerminal terminal = listPlugins(home);
execute();
assertEquals(buildMultiline("fake1", "fake2"), terminal.getOutput());
}

public void testPluginWithVerbose() throws Exception {
buildFakePlugin(env, "fake desc", "fake_plugin", "org.fake");
String[] params = { "-v" };
MockTerminal terminal = listPlugins(home, params);
execute("-v");
assertEquals(
buildMultiline(
"Plugins directory: " + env.pluginsFile(),
Expand All @@ -156,8 +125,7 @@ public void testPluginWithVerbose() throws Exception {

public void testPluginWithNativeController() throws Exception {
buildFakePlugin(env, "fake desc 1", "fake_plugin1", "org.fake", true);
String[] params = { "-v" };
MockTerminal terminal = listPlugins(home, params);
execute("-v");
assertEquals(
buildMultiline(
"Plugins directory: " + env.pluginsFile(),
Expand All @@ -181,8 +149,7 @@ public void testPluginWithNativeController() throws Exception {
public void testPluginWithVerboseMultiplePlugins() throws Exception {
buildFakePlugin(env, "fake desc 1", "fake_plugin1", "org.fake");
buildFakePlugin(env, "fake desc 2", "fake_plugin2", "org.fake2");
String[] params = { "-v" };
MockTerminal terminal = listPlugins(home, params);
execute("-v");
assertEquals(
buildMultiline(
"Plugins directory: " + env.pluginsFile(),
Expand Down Expand Up @@ -218,22 +185,21 @@ public void testPluginWithVerboseMultiplePlugins() throws Exception {
public void testPluginWithoutVerboseMultiplePlugins() throws Exception {
buildFakePlugin(env, "fake desc 1", "fake_plugin1", "org.fake");
buildFakePlugin(env, "fake desc 2", "fake_plugin2", "org.fake2");
MockTerminal terminal = listPlugins(home, new String[0]);
String output = terminal.getOutput();
assertEquals(buildMultiline("fake_plugin1", "fake_plugin2"), output);
execute();
assertEquals(buildMultiline("fake_plugin1", "fake_plugin2"), terminal.getOutput());
}

public void testPluginWithoutDescriptorFile() throws Exception {
final Path pluginDir = env.pluginsFile().resolve("fake1");
Files.createDirectories(pluginDir);
NoSuchFileException e = expectThrows(NoSuchFileException.class, () -> listPlugins(home));
NoSuchFileException e = expectThrows(NoSuchFileException.class, () -> execute());
assertEquals(pluginDir.resolve(PluginInfo.ES_PLUGIN_PROPERTIES).toString(), e.getFile());
}

public void testPluginWithWrongDescriptorFile() throws Exception {
final Path pluginDir = env.pluginsFile().resolve("fake1");
PluginTestUtil.writePluginProperties(pluginDir, "description", "fake desc");
IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> listPlugins(home));
IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> execute());
final Path descriptorPath = pluginDir.resolve(PluginInfo.ES_PLUGIN_PROPERTIES);
assertEquals("property [name] is missing in [" + descriptorPath.toString() + "]", e.getMessage());
}
Expand All @@ -256,19 +222,34 @@ public void testExistingIncompatiblePlugin() throws Exception {
);
buildFakePlugin(env, "fake desc 2", "fake_plugin2", "org.fake2");

MockTerminal terminal = listPlugins(home);
execute();
String message = "plugin [fake_plugin1] was built for Elasticsearch version 1.0.0 but version " + Version.CURRENT + " is required";
assertEquals("""
fake_plugin1
fake_plugin2
""", terminal.getOutput());
assertEquals("WARNING: " + message + "\n", terminal.getErrorOutput());

String[] params = { "-s" };
terminal = listPlugins(home, params);
terminal.reset();
execute("-s");
assertEquals("""
fake_plugin1
fake_plugin2
""", terminal.getOutput());
}

@Override
protected Command newCommand() {
return new ListPluginsCommand() {
@Override
protected Environment createEnv(Map<String, String> settings) {
return env;
}

@Override
protected boolean addShutdownHook() {
return false;
}
};
}
}

0 comments on commit 28a008a

Please sign in to comment.