Create event consistency test + test code cleanup

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(
+                    HashedPassword hashedPassword = authmeSha256.computeHash(hash, auth.getNickname());
-                        auth.getPassword().getHash(), auth.getNickname());
                     auth.setPassword(hashedPassword);
                     dataSource.updatePassword(auth);
                 }

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);
+        }
+    }
+
 }

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<Class<? extends Event>> 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<? extends Event> 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 <i>absent</i> 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<? extends Event> 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<? extends Event>) clazz;
+            }
+            return null;
+        } catch (ClassNotFoundException e) {
+            throw new IllegalStateException("Could not load class '" + className + "'", e);
+        }
+    }
+
+}

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();
+            TestHelper.validateHasOnlyPrivateEmptyConstructor(clazz);
-            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();
-            }
         }
     }
 

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)