From 7f44ecdb40826235cd0cf61be130bcdfdf7af7fb Mon Sep 17 00:00:00 2001 From: ljacqu Date: Fri, 29 Apr 2016 22:39:46 +0200 Subject: [PATCH] Create event consistency test + test code cleanup --- .../xephi/authme/util/MigrationService.java | 3 +- src/test/java/fr/xephi/authme/TestHelper.java | 36 +++++++ .../authme/events/EventsConsistencyTest.java | 94 +++++++++++++++++++ .../SettingsClassConsistencyTest.java | 23 +---- .../authme/util/MigrationServiceTest.java | 5 + 5 files changed, 139 insertions(+), 22 deletions(-) create mode 100644 src/test/java/fr/xephi/authme/events/EventsConsistencyTest.java diff --git a/src/main/java/fr/xephi/authme/util/MigrationService.java b/src/main/java/fr/xephi/authme/util/MigrationService.java index 6d86adbe..7bf028f6 100644 --- a/src/main/java/fr/xephi/authme/util/MigrationService.java +++ b/src/main/java/fr/xephi/authme/util/MigrationService.java @@ -44,8 +44,7 @@ public final class MigrationService { if (hash.startsWith("$SHA$")) { ConsoleLogger.showError("Skipping conversion for " + auth.getNickname() + "; detected SHA hash"); } else { - HashedPassword hashedPassword = authmeSha256.computeHash( - auth.getPassword().getHash(), auth.getNickname()); + HashedPassword hashedPassword = authmeSha256.computeHash(hash, auth.getNickname()); auth.setPassword(hashedPassword); dataSource.updatePassword(auth); } diff --git a/src/test/java/fr/xephi/authme/TestHelper.java b/src/test/java/fr/xephi/authme/TestHelper.java index fe55de5f..463b3870 100644 --- a/src/test/java/fr/xephi/authme/TestHelper.java +++ b/src/test/java/fr/xephi/authme/TestHelper.java @@ -6,6 +6,9 @@ import org.mockito.ArgumentCaptor; import org.mockito.Mockito; import java.io.File; +import java.lang.reflect.Constructor; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Modifier; import java.net.URL; import java.nio.file.Path; import java.nio.file.Paths; @@ -84,9 +87,42 @@ public final class TestHelper { runnable.run(); } + /** + * Assign the necessary fields on ConsoleLogger with mocks. + * + * @return The logger mock used + */ public static Logger setupLogger() { Logger logger = Mockito.mock(Logger.class); ConsoleLogger.setLogger(logger); return logger; } + + /** + * Check that a class only has a hidden, zero-argument constructor, preventing the + * instantiation of such classes (utility classes). Invokes the hidden constructor + * as to register the code coverage. + * + * @param clazz The class to validate + */ + public static void validateHasOnlyPrivateEmptyConstructor(Class clazz) { + Constructor[] constructors = clazz.getDeclaredConstructors(); + if (constructors.length > 1) { + throw new IllegalStateException("Class " + clazz.getSimpleName() + " has more than one constructor"); + } else if (constructors[0].getParameterTypes().length != 0) { + throw new IllegalStateException("Constructor of " + clazz + " does not have empty parameter list"); + } else if (!Modifier.isPrivate(constructors[0].getModifiers())) { + throw new IllegalStateException("Constructor of " + clazz + " is not private"); + } + + // Ugly hack to get coverage on the private constructors + // http://stackoverflow.com/questions/14077842/how-to-test-a-private-constructor-in-java-application + try { + constructors[0].setAccessible(true); + constructors[0].newInstance(); + } catch (InvocationTargetException | InstantiationException | IllegalAccessException e) { + throw new UnsupportedOperationException(e); + } + } + } diff --git a/src/test/java/fr/xephi/authme/events/EventsConsistencyTest.java b/src/test/java/fr/xephi/authme/events/EventsConsistencyTest.java new file mode 100644 index 00000000..d7ba5f74 --- /dev/null +++ b/src/test/java/fr/xephi/authme/events/EventsConsistencyTest.java @@ -0,0 +1,94 @@ +package fr.xephi.authme.events; + +import org.apache.commons.lang.reflect.MethodUtils; +import org.bukkit.event.Event; +import org.junit.BeforeClass; +import org.junit.Test; + +import java.io.File; +import java.lang.reflect.Method; +import java.lang.reflect.Modifier; +import java.util.ArrayList; +import java.util.List; + +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.nullValue; +import static org.junit.Assert.assertThat; + +/** + * Checks the consistency of the AuthMe event classes. + */ +public class EventsConsistencyTest { + + private static final String SRC_FOLDER = "src/main/java/"; + private static final String EVENTS_FOLDER = SRC_FOLDER + "/fr/xephi/authme/events/"; + private static List> classes; + + @BeforeClass + public static void scanEventClasses() { + File eventsFolder = new File(EVENTS_FOLDER); + File[] filesInFolder = eventsFolder.listFiles(); + if (filesInFolder == null || filesInFolder.length == 0) { + throw new IllegalStateException("Could not read folder '" + EVENTS_FOLDER + "'. Is it correct?"); + } + + classes = new ArrayList<>(); + for (File file : filesInFolder) { + Class clazz = getEventClassFromFile(file); + if (clazz != null) { + classes.add(clazz); + } + } + if (classes.isEmpty()) { + throw new IllegalStateException("Did not find any AuthMe event classes. Is the folder correct?"); + } + } + + @Test + public void shouldExtendFromCustomEvent() { + for (Class clazz : classes) { + assertThat("Class " + clazz.getSimpleName() + " is subtype of CustomEvent", + CustomEvent.class.isAssignableFrom(clazz), equalTo(true)); + } + } + + /** + * Bukkit requires a static getHandlerList() method on all event classes, see {@link Event}. + * This test checks that such a method is present, and that it is absent if the class + * is not instantiable (abstract class). + */ + @Test + public void shouldHaveStaticEventHandlerMethod() { + for (Class clazz : classes) { + Method handlerListMethod = MethodUtils.getAccessibleMethod(clazz, "getHandlerList", new Class[]{}); + if (canBeInstantiated(clazz)) { + assertThat("Class " + clazz.getSimpleName() + " has static method getHandlerList()", + handlerListMethod != null && Modifier.isStatic(handlerListMethod.getModifiers()), equalTo(true)); + } else { + assertThat("Non-instantiable class " + clazz.getSimpleName() + " does not have static getHandlerList()", + handlerListMethod, nullValue()); + } + } + } + + private static boolean canBeInstantiated(Class clazz) { + return !clazz.isInterface() && !clazz.isEnum() && !Modifier.isAbstract(clazz.getModifiers()); + } + + private static Class getEventClassFromFile(File file) { + String fileName = file.getPath(); + String className = fileName + .substring(SRC_FOLDER.length(), fileName.length() - ".java".length()) + .replace(File.separator, "."); + try { + Class clazz = EventsConsistencyTest.class.getClassLoader().loadClass(className); + if (Event.class.isAssignableFrom(clazz)) { + return (Class) clazz; + } + return null; + } catch (ClassNotFoundException e) { + throw new IllegalStateException("Could not load class '" + className + "'", e); + } + } + +} diff --git a/src/test/java/fr/xephi/authme/settings/properties/SettingsClassConsistencyTest.java b/src/test/java/fr/xephi/authme/settings/properties/SettingsClassConsistencyTest.java index c6b00383..75c1a56a 100644 --- a/src/test/java/fr/xephi/authme/settings/properties/SettingsClassConsistencyTest.java +++ b/src/test/java/fr/xephi/authme/settings/properties/SettingsClassConsistencyTest.java @@ -1,22 +1,20 @@ package fr.xephi.authme.settings.properties; import fr.xephi.authme.ReflectionTestUtils; +import fr.xephi.authme.TestHelper; import fr.xephi.authme.settings.domain.Property; import fr.xephi.authme.settings.domain.SettingsClass; import org.junit.BeforeClass; import org.junit.Test; import java.io.File; -import java.lang.reflect.Constructor; import java.lang.reflect.Field; -import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Modifier; import java.util.ArrayList; import java.util.HashSet; import java.util.List; import java.util.Set; -import static org.hamcrest.Matchers.arrayWithSize; import static org.hamcrest.Matchers.equalTo; import static org.junit.Assert.assertThat; import static org.junit.Assert.fail; @@ -86,24 +84,9 @@ public class SettingsClassConsistencyTest { } @Test - public void shouldHaveHiddenDefaultConstructorOnly() { + public void shouldHaveHiddenEmptyConstructorOnly() { for (Class clazz : classes) { - Constructor[] constructors = clazz.getDeclaredConstructors(); - assertThat(clazz + " should only have one constructor", - constructors, arrayWithSize(1)); - assertThat("Constructor of " + clazz + " is private", - Modifier.isPrivate(constructors[0].getModifiers()), equalTo(true)); - - // Ugly hack to get coverage on the private constructors - // http://stackoverflow.com/questions/14077842/how-to-test-a-private-constructor-in-java-application - try { - Constructor constructor = clazz.getDeclaredConstructor(); - constructor.setAccessible(true); - constructor.newInstance(); - } catch (NoSuchMethodException | InstantiationException - | IllegalAccessException | InvocationTargetException e) { - e.printStackTrace(); - } + TestHelper.validateHasOnlyPrivateEmptyConstructor(clazz); } } diff --git a/src/test/java/fr/xephi/authme/util/MigrationServiceTest.java b/src/test/java/fr/xephi/authme/util/MigrationServiceTest.java index d27f642e..34fda2bb 100644 --- a/src/test/java/fr/xephi/authme/util/MigrationServiceTest.java +++ b/src/test/java/fr/xephi/authme/util/MigrationServiceTest.java @@ -110,6 +110,11 @@ public class MigrationServiceTest { verifyNoMoreInteractions(settings, dataSource, sha256); } + @Test + public void shouldHaveHiddenEmptyConstructorOnly() { + TestHelper.validateHasOnlyPrivateEmptyConstructor(MigrationService.class); + } + private static PlayerAuth authWithNickAndHash(String nick, String hash) { return PlayerAuth.builder() .name(nick)