Add methods to Reflection utils; test ChangePasswordCommand fully

src/main/java/fr/xephi/authme/command/executable/changepassword/ChangePasswordCommand.java

@@ -66,7 +66,7 @@ public class ChangePasswordCommand extends ExecutableCommand {
 
         // Set the password
         final AuthMe plugin = wrapper.getAuthMe();
-        plugin.getServer().getScheduler().runTaskAsynchronously(plugin,
+        wrapper.getServer().getScheduler().runTaskAsynchronously(plugin,
             new ChangePasswordTask(plugin, player, playerPass, playerPassVerify));
         return true;
     }

src/main/java/fr/xephi/authme/task/ChangePasswordTask.java

@@ -36,11 +36,6 @@ public class ChangePasswordTask implements Runnable {
         this.newPassword = newPassword;
     }
 
-    /**
-     * Method run.
-     *
-     * @see java.lang.Runnable#run()
-     */
     @Override
     public void run() {
         Messages m = plugin.getMessages();

src/test/java/fr/xephi/authme/ReflectionTestUtils.java

@@ -1,6 +1,9 @@
 package fr.xephi.authme;
 
 import java.lang.reflect.Field;
+import java.lang.reflect.Method;
+
+import static java.lang.String.format;
 
 /**
  * Offers reflection functionality to set up tests. Use only when absolutely necessary.
@@ -21,13 +24,53 @@ public final class ReflectionTestUtils {
      */
     public static <T> void setField(Class<T> clazz, T instance, String fieldName, Object value) {
         try {
-            Field field = clazz.getDeclaredField(fieldName);
+            Field field = getField(clazz, instance, fieldName);
-            field.setAccessible(true);
             field.set(instance, value);
-        } catch (NoSuchFieldException | IllegalAccessException e) {
+        } catch (IllegalAccessException e) {
             throw new RuntimeException(
-                String.format("Could not set value to field '%s' for instance '%s' of class '%s'",
+                format("Could not set value to field '%s' for instance '%s' of class '%s'",
                     fieldName, instance, clazz.getName()), e);
         }
     }
+
+    private static <T> Field getField(Class<T> clazz, T instance, String fieldName) {
+        try {
+            Field field = clazz.getDeclaredField(fieldName);
+            field.setAccessible(true);
+            return field;
+        } catch (NoSuchFieldException e) {
+            throw new RuntimeException(format("Could not get field '%s' for instance '%s' of class '%s'",
+                fieldName, instance, clazz.getName()), e);
+        }
+    }
+
+
+    public static <T> Object getFieldValue(Class<T> clazz, T instance, String fieldName) {
+        Field field = getField(clazz, instance, fieldName);
+        try {
+            return field.get(instance);
+        } catch (IllegalAccessException e) {
+            throw new RuntimeException("Could not get value of field '" + fieldName + "'");
+        }
+    }
+
+    /**
+     * Return the method on the given class with the supplied parameter types.
+     *
+     * @param clazz The class to retrieve a method from
+     * @param methodName The name of the method
+     * @param parameterTypes The parameter types the method to retrieve has
+     *
+     * @return The method of the class, set to be accessible
+     */
+    public static Method getMethod(Class<?> clazz, String methodName, Class<?>... parameterTypes) {
+        try {
+            Method method = clazz.getDeclaredMethod(methodName, parameterTypes);
+            method.setAccessible(true);
+            return method;
+        } catch (NoSuchMethodException e) {
+            throw new RuntimeException("Could not retrieve method '" + methodName + "' from class '"
+                + clazz.getName() + "'");
+        }
+    }
 }

src/test/java/fr/xephi/authme/command/executable/changepassword/ChangePasswordCommandTest.java

@@ -1,19 +1,29 @@
 package fr.xephi.authme.command.executable.changepassword;
 
+import fr.xephi.authme.AuthMe;
+import fr.xephi.authme.ReflectionTestUtils;
 import fr.xephi.authme.cache.auth.PlayerCache;
 import fr.xephi.authme.command.CommandParts;
 import fr.xephi.authme.settings.MessageKey;
 import fr.xephi.authme.settings.Messages;
 import fr.xephi.authme.settings.Settings;
+import fr.xephi.authme.task.ChangePasswordTask;
 import fr.xephi.authme.util.WrapperMock;
+import org.bukkit.Server;
 import org.bukkit.command.BlockCommandSender;
 import org.bukkit.command.CommandSender;
 import org.bukkit.entity.Player;
+import org.bukkit.scheduler.BukkitScheduler;
 import org.junit.Before;
 import org.junit.Test;
+import org.mockito.ArgumentCaptor;
 
-import java.util.Arrays;
+import java.util.Collections;
 
+import static java.util.Arrays.asList;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.equalTo;
+import static org.mockito.BDDMockito.given;
 import static org.mockito.Matchers.anyInt;
 import static org.mockito.Mockito.*;
 
@@ -22,20 +32,21 @@ import static org.mockito.Mockito.*;
  */
 public class ChangePasswordCommandTest {
 
+    private WrapperMock wrapperMock;
     private Messages messagesMock;
     private PlayerCache cacheMock;
 
     @Before
     public void setUpMocks() {
-        WrapperMock wrapper = WrapperMock.createInstance();
+        wrapperMock = WrapperMock.createInstance();
-        messagesMock = wrapper.getMessages();
+        messagesMock = wrapperMock.getMessages();
-        cacheMock = wrapper.getPlayerCache();
+        cacheMock = wrapperMock.getPlayerCache();
 
         // Only allow passwords with alphanumerical characters for the test
         Settings.getPassRegex = "[a-zA-Z0-9]+";
         Settings.getPasswordMinLen = 2;
         Settings.passwordMaxLength = 50;
-        // TODO ljacqu 20151126: Verify the calls to getServer() (see commented code)
+        Settings.unsafePasswords = Collections.EMPTY_LIST;
     }
 
     @Test
@@ -50,7 +61,7 @@ public class ChangePasswordCommandTest {
 
         // then
         verify(arguments, never()).get(anyInt());
-        //verify(pluginMock, never()).getServer();
+        assertThat(wrapperMock.wasMockCalled(Server.class), equalTo(false));
     }
 
     @Test
@@ -64,7 +75,7 @@ public class ChangePasswordCommandTest {
 
         // then
         verify(messagesMock).send(sender, MessageKey.NOT_LOGGED_IN);
-        //verify(pluginMock, never()).getServer();
+        assertThat(wrapperMock.wasMockCalled(Server.class), equalTo(false));
     }
 
     @Test
@@ -78,7 +89,7 @@ public class ChangePasswordCommandTest {
 
         // then
         verify(messagesMock).send(sender, MessageKey.PASSWORD_MATCH_ERROR);
-        //verify(pluginMock, never()).getServer();
+        assertThat(wrapperMock.wasMockCalled(Server.class), equalTo(false));
     }
 
 
@@ -93,7 +104,7 @@ public class ChangePasswordCommandTest {
 
         // then
         verify(messagesMock).send(sender, MessageKey.PASSWORD_IS_USERNAME_ERROR);
-        //verify(pluginMock, never()).getServer();
+        assertThat(wrapperMock.wasMockCalled(Server.class), equalTo(false));
     }
 
     @Test
@@ -108,7 +119,7 @@ public class ChangePasswordCommandTest {
 
         // then
         verify(messagesMock).send(sender, MessageKey.INVALID_PASSWORD_LENGTH);
-        //verify(pluginMock, never()).getServer();
+        assertThat(wrapperMock.wasMockCalled(Server.class), equalTo(false));
     }
 
     @Test
@@ -123,7 +134,7 @@ public class ChangePasswordCommandTest {
 
         // then
         verify(messagesMock).send(sender, MessageKey.INVALID_PASSWORD_LENGTH);
-        //verify(pluginMock, never()).getServer();
+        assertThat(wrapperMock.wasMockCalled(Server.class), equalTo(false));
     }
 
     @Test
@@ -131,14 +142,34 @@ public class ChangePasswordCommandTest {
         // given
         CommandSender sender = initPlayerWithName("player", true);
         ChangePasswordCommand command = new ChangePasswordCommand();
-        Settings.unsafePasswords = Arrays.asList("test", "abc123");
+        Settings.unsafePasswords = asList("test", "abc123");
 
         // when
         command.executeCommand(sender, new CommandParts(), new CommandParts("abc123"));
 
         // then
         verify(messagesMock).send(sender, MessageKey.PASSWORD_UNSAFE_ERROR);
-        //verify(pluginMock, never()).getServer();
+        assertThat(wrapperMock.wasMockCalled(Server.class), equalTo(false));
+    }
+
+    @Test
+    public void shouldForwardTheDataForValidPassword() {
+        // given
+        CommandSender sender = initPlayerWithName("parker", true);
+        ChangePasswordCommand command = new ChangePasswordCommand();
+        BukkitScheduler schedulerMock = mock(BukkitScheduler.class);
+        given(wrapperMock.getServer().getScheduler()).willReturn(schedulerMock);
+
+        // when
+        command.executeCommand(sender, new CommandParts(), new CommandParts(asList("abc123", "abc123")));
+
+        // then
+        verify(messagesMock, never()).send(eq(sender), any(MessageKey.class));
+        ArgumentCaptor<ChangePasswordTask> taskCaptor = ArgumentCaptor.forClass(ChangePasswordTask.class);
+        verify(schedulerMock).runTaskAsynchronously(any(AuthMe.class), taskCaptor.capture());
+        ChangePasswordTask task = taskCaptor.getValue();
+        assertThat((String) ReflectionTestUtils.getFieldValue(ChangePasswordTask.class, task, "newPassword"),
+            equalTo("abc123"));
     }
 
     private Player initPlayerWithName(String name, boolean loggedIn) {

src/test/java/fr/xephi/authme/util/UtilsTest.java

@@ -23,7 +23,6 @@ import static org.mockito.Mockito.*;
  */
 public class UtilsTest {
 
-    private static WrapperMock wrapperMock;
     private static AuthMe authMeMock;
     private PermissionsManager permissionsManagerMock;
 
@@ -34,7 +33,7 @@ public class UtilsTest {
      */
     @BeforeClass
     public static void setUpMocks() {
-        wrapperMock = WrapperMock.createInstance();
+        WrapperMock wrapperMock = WrapperMock.createInstance();
         authMeMock = wrapperMock.getAuthMe();
     }
 
@@ -109,16 +108,11 @@ public class UtilsTest {
     }
 
     @Test
-    // Note ljacqu 20151122: This is a heavy test setup with reflections... If it causes trouble, skip it with @Ignore
     public void shouldRetrieveListOfOnlinePlayersFromReflectedMethod() {
         // given
         ReflectionTestUtils.setField(Utils.class, null, "getOnlinePlayersIsCollection", false);
-        try {
+        ReflectionTestUtils.setField(Utils.class, null, "getOnlinePlayers",
-            ReflectionTestUtils.setField(Utils.class, null, "getOnlinePlayers",
+            ReflectionTestUtils.getMethod(UtilsTest.class, "onlinePlayersImpl"));
-                UtilsTest.class.getDeclaredMethod("onlinePlayersImpl"));
-        } catch (NoSuchMethodException e) {
-            throw new RuntimeException("Could not get method onlinePlayersImpl() in test class", e);
-        }
 
         // when
         Collection<? extends Player> players = Utils.getOnlinePlayers();

src/test/java/fr/xephi/authme/util/WrapperMock.java

@@ -20,8 +20,6 @@ import java.util.logging.Logger;
 public class WrapperMock extends Wrapper {
 
     private Map<Class<?>, Object> mocks = new HashMap<>();
-    private static WrapperMock singleton;
-    private File getDataFolderValue = new File("/");
 
     private WrapperMock() {
         super();
@@ -33,9 +31,9 @@ public class WrapperMock extends Wrapper {
      * @return The created singleton
      */
     public static WrapperMock createInstance() {
-        singleton = new WrapperMock();
+        WrapperMock instance = new WrapperMock();
-        Wrapper.setSingleton(singleton);
+        Wrapper.setSingleton(instance);
-        return singleton;
+        return instance;
     }
 
     @Override
@@ -70,20 +68,19 @@ public class WrapperMock extends Wrapper {
 
     @Override
     public File getDataFolder() {
-        if (singleton.getDataFolderValue != null) {
+        return new File("/");
-            return singleton.getDataFolderValue;
-        }
-        return getMock(File.class);
     }
 
     /**
-     * Set the data folder to be returned for test contexts. Defaults to File("/"); supply null to make WrapperMock
+     * Return whether a mock of the given class type was created, i.e. verify whether a certain method was executed on
-     * return a mock of the File class as with the other fields.
+     * the Wrapper to retrieve an entity.
      *
-     * @param file The data folder location to return
+     * @param mockClass The class of the mock to verify
+     *
+     * @return True if the mock has been created, false otherwise
      */
-    public void setDataFolder(File file) {
+    public boolean wasMockCalled(Class<?> mockClass) {
-        this.getDataFolderValue = file;
+        return mocks.get(mockClass) != null;
     }
 
     @SuppressWarnings("unchecked")