From 51f5c8bcfed217342fc7c049f0ef9f051548f12b Mon Sep 17 00:00:00 2001 From: nickfelt Date: Tue, 27 Sep 2016 14:06:50 -0700 Subject: [PATCH] Improve tests for RegistryTool and GtechTool This improves the tests by: 1) Adding tests for alphabetical ordering of the command maps, to keep them organized, and fixing existing mis-orderings. Note that this is a no-op test for RegistryTool since it uses ImmutableSortedMap (to resort the commands after inserting GtechTool.COMMAND_MAP), but it'll be relevant in the upcoming CL when they get merged. I changed GtechTool.COMMAND_MAP to use regular ImmutableMap. 2) Checking that RegistryTool.COMMAND_MAP (the full map, after the existing GtechTool.COMMAND_MAP has been merged in) contains the exact same set of commands as all the concrete classes implementing Command that we can find. This is a stronger test than what we had before, which just checked that every Command class appeared in RegistryTool (i.e. that RegistryTool's commands are a superset of all Commands found). You'd think that it'd be impossible for RegistryTool to contain commands that aren't in the set of Commands we found, but it is if we're not searching for commands properly, which we weren't (we were only checking within the .tools package and not within any subpackages (e.g. tools.javascrap). This has now been fixed. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=134451859 --- java/google/registry/tools/GtechTool.java | 7 ++-- java/google/registry/tools/RegistryTool.java | 2 +- .../google/registry/tools/GtechToolTest.java | 6 ++++ .../registry/tools/RegistryToolTest.java | 34 +++++++++++-------- 4 files changed, 30 insertions(+), 19 deletions(-) diff --git a/java/google/registry/tools/GtechTool.java b/java/google/registry/tools/GtechTool.java index 58b183d54..c6b912450 100644 --- a/java/google/registry/tools/GtechTool.java +++ b/java/google/registry/tools/GtechTool.java @@ -15,7 +15,6 @@ package google.registry.tools; import com.google.common.collect.ImmutableMap; -import com.google.common.collect.ImmutableSortedMap; import google.registry.tools.Command.GtechCommand; /** Command line interface with a subset of commands that are safe for tech support to run. */ @@ -28,7 +27,7 @@ public final class GtechTool { * any invocations in scripts (e.g. PDT, ICANN reporting). */ static final ImmutableMap> COMMAND_MAP = - ImmutableSortedMap.>naturalOrder() + new ImmutableMap.Builder>() .put("auction_status", AuctionStatusCommand.class) .put("canonicalize_labels", CanonicalizeLabelsCommand.class) .put("convert_idn", ConvertIdnCommand.class) @@ -39,8 +38,8 @@ public final class GtechTool { .put("create_domain", CreateDomainCommand.class) .put("create_host", CreateHostCommand.class) .put("create_lrp_tokens", CreateLrpTokensCommand.class) - .put("create_registrar_groups", CreateRegistrarGroupsCommand.class) .put("create_registrar", CreateRegistrarCommand.class) + .put("create_registrar_groups", CreateRegistrarGroupsCommand.class) .put("create_sandbox_tld", CreateSandboxTldCommand.class) .put("delete_domain", DeleteDomainCommand.class) .put("domain_application_info", DomainApplicationInfoCommand.class) @@ -55,9 +54,9 @@ public final class GtechTool { .put("get_applied_labels", GetAppliedLabelsCommand.class) .put("get_contact", GetContactCommand.class) .put("get_domain", GetDomainCommand.class) - .put("get_lrp_token", GetLrpTokenCommand.class) .put("get_history_entries", GetHistoryEntriesCommand.class) .put("get_host", GetHostCommand.class) + .put("get_lrp_token", GetLrpTokenCommand.class) .put("get_registrar", GetRegistrarCommand.class) .put("get_schema", GetSchemaCommand.class) .put("get_schema_tree", GetSchemaTreeCommand.class) diff --git a/java/google/registry/tools/RegistryTool.java b/java/google/registry/tools/RegistryTool.java index b6d8734ea..15e1a79dc 100644 --- a/java/google/registry/tools/RegistryTool.java +++ b/java/google/registry/tools/RegistryTool.java @@ -46,9 +46,9 @@ public final class RegistryTool { .put("delete_reserved_list", DeleteReservedListCommand.class) .put("encrypt_escrow_deposit", EncryptEscrowDepositCommand.class) .put("execute_epp", ExecuteEppCommand.class) - .put("generate_zone_files", GenerateZoneFilesCommand.class) .put("generate_escrow_deposit", GenerateEscrowDepositCommand.class) .put("generate_lordn", GenerateLordnCommand.class) + .put("generate_zone_files", GenerateZoneFilesCommand.class) .put("get_claims_list", GetClaimsListCommand.class) .put("get_resource_by_key", GetResourceByKeyCommand.class) .put("ghostryde", GhostrydeCommand.class) diff --git a/javatests/google/registry/tools/GtechToolTest.java b/javatests/google/registry/tools/GtechToolTest.java index 5061866e3..1c926f8d9 100644 --- a/javatests/google/registry/tools/GtechToolTest.java +++ b/javatests/google/registry/tools/GtechToolTest.java @@ -17,6 +17,7 @@ package google.registry.tools; import static com.google.common.base.CaseFormat.LOWER_UNDERSCORE; import static com.google.common.base.CaseFormat.UPPER_CAMEL; import static com.google.common.reflect.Reflection.getPackageName; +import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertWithMessage; import com.google.common.base.Joiner; @@ -48,6 +49,11 @@ public class GtechToolTest { RegistryToolEnvironment.UNITTEST.setup(); } + @Test + public void test_commandMap_namesAreInAlphabeticalOrder() throws Exception { + assertThat(GtechTool.COMMAND_MAP.keySet()).isStrictlyOrdered(); + } + @Test public void testThatAllCommandsAreInCliOptions() throws Exception { Set> commandMapClasses = diff --git a/javatests/google/registry/tools/RegistryToolTest.java b/javatests/google/registry/tools/RegistryToolTest.java index 9a2450966..e93b612c7 100644 --- a/javatests/google/registry/tools/RegistryToolTest.java +++ b/javatests/google/registry/tools/RegistryToolTest.java @@ -17,9 +17,8 @@ package google.registry.tools; import static com.google.common.base.CaseFormat.LOWER_UNDERSCORE; import static com.google.common.base.CaseFormat.UPPER_CAMEL; import static com.google.common.reflect.Reflection.getPackageName; -import static com.google.common.truth.Truth.assertWithMessage; +import static com.google.common.truth.Truth.assertThat; -import com.google.common.base.Joiner; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Sets; import com.google.common.reflect.ClassPath; @@ -28,7 +27,6 @@ import com.google.common.truth.Expect; import java.io.IOException; import java.lang.reflect.Modifier; import java.util.Map; -import java.util.Set; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -48,19 +46,27 @@ public class RegistryToolTest { } @Test - public void testThatAllCommandsAreInCliOptions() throws Exception { - Set> commandMapClasses = - ImmutableSet.copyOf(RegistryTool.COMMAND_MAP.values()); - Set> commandsWithoutCliInvokers = - Sets.difference(getAllCommandClasses(), commandMapClasses); - String errorMsg = - "These Command classes are missing from RegistryTool.COMMAND_MAP: " - + Joiner.on(", ").join(commandsWithoutCliInvokers); - assertWithMessage(errorMsg).that(commandsWithoutCliInvokers).isEmpty(); + public void test_commandMap_namesAreInAlphabeticalOrder() throws Exception { + assertThat(RegistryTool.COMMAND_MAP.keySet()).isStrictlyOrdered(); } @Test - public void testThatCommandNamesAreDerivedFromClassNames() throws Exception { + public void test_commandMap_includesAllCommands() throws Exception { + ImmutableSet commandMapClasses = ImmutableSet.copyOf(RegistryTool.COMMAND_MAP.values()); + ImmutableSet classLoaderClasses = getAllCommandClasses(); + // Not using plain old containsExactlyElementsIn() since it produces a huge unreadable blob. + assertThat( + Sets.difference(commandMapClasses, classLoaderClasses)) + .named("command classes in RegistryTool.COMMAND_MAP but not found by class loader") + .isEmpty(); + assertThat( + Sets.difference(classLoaderClasses, commandMapClasses)) + .named("command classes found by class loader but not in RegistryTool.COMMAND_MAP") + .isEmpty(); + } + + @Test + public void test_commandMap_namesAreDerivedFromClassNames() throws Exception { for (Map.Entry> commandEntry : RegistryTool.COMMAND_MAP.entrySet()) { String className = commandEntry.getValue().getSimpleName(); @@ -83,7 +89,7 @@ public class RegistryToolTest { ImmutableSet.Builder> builder = new ImmutableSet.Builder<>(); for (ClassInfo classInfo : ClassPath .from(getClass().getClassLoader()) - .getTopLevelClasses(getPackageName(getClass()))) { + .getTopLevelClassesRecursive(getPackageName(getClass()))) { Class clazz = classInfo.load(); if (Command.class.isAssignableFrom(clazz) && !Modifier.isAbstract(clazz.getModifiers())