From c02bf7db766741b1221868a89773df2e091a1717 Mon Sep 17 00:00:00 2001 From: ljacqu Date: Sat, 28 Nov 2015 19:10:01 +0100 Subject: [PATCH] Testing - initialize data folder in WrapperMock; create ReflectionUtils - Change AuthMeMockUtils (reflection-based test setup) to ReflectionUtils: service providing reflection functionalities for particular tests where it is appropriate; - Initialize the data folder (required as soon as the Settings class is loaded) immediately in WrapperMock. Some tests did not set it up that required it and it goes unnoticed if the test is not run individually. This will hopefully fix the tests from failing in the Jenkins build. --- .../java/fr/xephi/authme/AntiBotTest.java | 5 -- .../java/fr/xephi/authme/AuthMeMockUtil.java | 65 ------------------- .../fr/xephi/authme/ReflectionTestUtils.java | 33 ++++++++++ .../ChangePasswordCommandTest.java | 9 +-- .../executable/login/LoginCommandTest.java | 8 +-- .../executable/logout/LogoutCommandTest.java | 3 - .../register/RegisterCommandTest.java | 3 - .../xephi/authme/settings/MessagesTest.java | 2 +- .../java/fr/xephi/authme/util/UtilsTest.java | 26 ++------ .../fr/xephi/authme/util/WrapperMock.java | 20 +++--- 10 files changed, 49 insertions(+), 125 deletions(-) delete mode 100644 src/test/java/fr/xephi/authme/AuthMeMockUtil.java create mode 100644 src/test/java/fr/xephi/authme/ReflectionTestUtils.java diff --git a/src/test/java/fr/xephi/authme/AntiBotTest.java b/src/test/java/fr/xephi/authme/AntiBotTest.java index 3398e7047..25e250fda 100644 --- a/src/test/java/fr/xephi/authme/AntiBotTest.java +++ b/src/test/java/fr/xephi/authme/AntiBotTest.java @@ -1,17 +1,13 @@ package fr.xephi.authme; import fr.xephi.authme.settings.Settings; -import fr.xephi.authme.util.Wrapper; import fr.xephi.authme.util.WrapperMock; import org.junit.Before; import org.junit.Test; -import java.io.File; - import static org.mockito.Matchers.any; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; /** * Test for {@link AntiBot}. @@ -23,7 +19,6 @@ public class AntiBotTest { @Before public void setUpMocks() { wrapper = WrapperMock.createInstance(); - wrapper.setDataFolder(new File("/")); } @Test diff --git a/src/test/java/fr/xephi/authme/AuthMeMockUtil.java b/src/test/java/fr/xephi/authme/AuthMeMockUtil.java deleted file mode 100644 index 38a68de8d..000000000 --- a/src/test/java/fr/xephi/authme/AuthMeMockUtil.java +++ /dev/null @@ -1,65 +0,0 @@ -package fr.xephi.authme; - -import fr.xephi.authme.cache.auth.PlayerCache; -import fr.xephi.authme.settings.Messages; -import fr.xephi.authme.util.Wrapper; -import fr.xephi.authme.util.WrapperMock; -import org.mockito.Mockito; - -import java.lang.reflect.Field; - -/** - * Creates a mock implementation of AuthMe for testing purposes. - */ -@Deprecated -public final class AuthMeMockUtil { - - private AuthMeMockUtil() { - // Util class - } - - /** - * Set the AuthMe plugin instance to a mock object. Use {@link AuthMe#getInstance()} to retrieve the mock. - * - * @return The generated mock for the AuthMe instance - */ - public static AuthMe mockAuthMeInstance() { - AuthMe mock = Mockito.mock(AuthMe.class); - mockSingletonForClass(AuthMe.class, "plugin", mock); - return mock; - } - - /** - * Create a mock Messages object for the instance returned from {@link Messages#getInstance()}. - */ - public static void mockMessagesInstance() { - Messages mock = Mockito.mock(Messages.class); - mockSingletonForClass(Messages.class, "singleton", mock); - } - - /** - * Creates a mock singleton for the player cache, retrievable from {@link PlayerCache#getInstance()}. - */ - public static void mockPlayerCacheInstance() { - PlayerCache mock = Mockito.mock(PlayerCache.class); - mockSingletonForClass(PlayerCache.class, "singleton", mock); - } - - - /** - * Set a field of a class to the given mock. - * - * @param clazz The class to modify - * @param fieldName The field name - * @param mock The mock to set for the given field - */ - public static void mockSingletonForClass(Class clazz, String fieldName, Object mock) { - try { - Field instance = clazz.getDeclaredField(fieldName); - instance.setAccessible(true); - instance.set(null, mock); - } catch (NoSuchFieldException | IllegalAccessException e) { - throw new RuntimeException("Could not set mock instance for class " + clazz.getName(), e); - } - } -} diff --git a/src/test/java/fr/xephi/authme/ReflectionTestUtils.java b/src/test/java/fr/xephi/authme/ReflectionTestUtils.java new file mode 100644 index 000000000..6007f1a82 --- /dev/null +++ b/src/test/java/fr/xephi/authme/ReflectionTestUtils.java @@ -0,0 +1,33 @@ +package fr.xephi.authme; + +import java.lang.reflect.Field; + +/** + * Offers reflection functionality to set up tests. Use only when absolutely necessary. + */ +public final class ReflectionTestUtils { + + private ReflectionTestUtils() { + // Util class + } + + /** + * Set the field of a given object to a new value with reflection. + * + * @param clazz The class of the object + * @param instance The instance to modify (pass null for static fields) + * @param fieldName The field name + * @param value The value to set the field to + */ + public static void setField(Class clazz, T instance, String fieldName, Object value) { + try { + Field field = clazz.getDeclaredField(fieldName); + field.setAccessible(true); + field.set(instance, value); + } catch (NoSuchFieldException | IllegalAccessException e) { + throw new RuntimeException( + String.format("Could not set value to field '%s' for instance '%s' of class '%s'", + fieldName, instance, clazz.getName()), e); + } + } +} diff --git a/src/test/java/fr/xephi/authme/command/executable/changepassword/ChangePasswordCommandTest.java b/src/test/java/fr/xephi/authme/command/executable/changepassword/ChangePasswordCommandTest.java index 9f095eab8..1cc820dd6 100644 --- a/src/test/java/fr/xephi/authme/command/executable/changepassword/ChangePasswordCommandTest.java +++ b/src/test/java/fr/xephi/authme/command/executable/changepassword/ChangePasswordCommandTest.java @@ -9,19 +9,13 @@ import fr.xephi.authme.util.WrapperMock; import org.bukkit.command.BlockCommandSender; import org.bukkit.command.CommandSender; import org.bukkit.entity.Player; - import org.junit.Before; import org.junit.Test; - -import java.io.File; import java.util.Arrays; import static org.mockito.Matchers.anyInt; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.never; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; +import static org.mockito.Mockito.*; /** * Test for {@link ChangePasswordCommand}. @@ -34,7 +28,6 @@ public class ChangePasswordCommandTest { @Before public void setUpMocks() { WrapperMock wrapper = WrapperMock.createInstance(); - wrapper.setDataFolder(new File("/")); messagesMock = wrapper.getMessages(); cacheMock = wrapper.getPlayerCache(); diff --git a/src/test/java/fr/xephi/authme/command/executable/login/LoginCommandTest.java b/src/test/java/fr/xephi/authme/command/executable/login/LoginCommandTest.java index f9f90619f..6f06a5fff 100644 --- a/src/test/java/fr/xephi/authme/command/executable/login/LoginCommandTest.java +++ b/src/test/java/fr/xephi/authme/command/executable/login/LoginCommandTest.java @@ -11,12 +11,7 @@ import org.junit.Before; import org.junit.Test; import org.mockito.Mockito; -import java.io.File; - -import static org.mockito.Matchers.any; -import static org.mockito.Matchers.anyBoolean; -import static org.mockito.Matchers.anyString; -import static org.mockito.Matchers.eq; +import static org.mockito.Matchers.*; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; @@ -30,7 +25,6 @@ public class LoginCommandTest { @Before public void initializeAuthMeMock() { WrapperMock wrapper = WrapperMock.createInstance(); - wrapper.setDataFolder(new File("/")); Settings.captchaLength = 10; managementMock = mock(Management.class); diff --git a/src/test/java/fr/xephi/authme/command/executable/logout/LogoutCommandTest.java b/src/test/java/fr/xephi/authme/command/executable/logout/LogoutCommandTest.java index a08784814..c8774978c 100644 --- a/src/test/java/fr/xephi/authme/command/executable/logout/LogoutCommandTest.java +++ b/src/test/java/fr/xephi/authme/command/executable/logout/LogoutCommandTest.java @@ -12,8 +12,6 @@ import org.junit.Before; import org.junit.Test; import org.mockito.Mockito; -import java.io.File; - import static org.mockito.Matchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; @@ -28,7 +26,6 @@ public class LogoutCommandTest { @Before public void initializeAuthMeMock() { WrapperMock wrapper = WrapperMock.createInstance(); - wrapper.setDataFolder(new File("/")); AuthMe pluginMock = wrapper.getAuthMe(); Settings.captchaLength = 10; diff --git a/src/test/java/fr/xephi/authme/command/executable/register/RegisterCommandTest.java b/src/test/java/fr/xephi/authme/command/executable/register/RegisterCommandTest.java index ef82aafef..bc6229c10 100644 --- a/src/test/java/fr/xephi/authme/command/executable/register/RegisterCommandTest.java +++ b/src/test/java/fr/xephi/authme/command/executable/register/RegisterCommandTest.java @@ -14,8 +14,6 @@ import org.junit.Test; import org.mockito.ArgumentCaptor; import org.mockito.Mockito; -import java.io.File; - import static org.hamcrest.Matchers.equalTo; import static org.junit.Assert.assertThat; import static org.mockito.Matchers.any; @@ -35,7 +33,6 @@ public class RegisterCommandTest { @Before public void initializeAuthMeMock() { WrapperMock wrapper = WrapperMock.createInstance(); - wrapper.setDataFolder(new File("/")); messagesMock = wrapper.getMessages(); Settings.captchaLength = 10; diff --git a/src/test/java/fr/xephi/authme/settings/MessagesTest.java b/src/test/java/fr/xephi/authme/settings/MessagesTest.java index f1467d1fc..b721987ec 100644 --- a/src/test/java/fr/xephi/authme/settings/MessagesTest.java +++ b/src/test/java/fr/xephi/authme/settings/MessagesTest.java @@ -30,7 +30,7 @@ public class MessagesTest { */ @Before public void setUpMessages() { - WrapperMock.getInstance(); + WrapperMock.createInstance(); Settings.messagesLanguage = "en"; URL url = getClass().getClassLoader().getResource(YML_TEST_FILE); diff --git a/src/test/java/fr/xephi/authme/util/UtilsTest.java b/src/test/java/fr/xephi/authme/util/UtilsTest.java index 57b3fc75a..3c79f63cf 100644 --- a/src/test/java/fr/xephi/authme/util/UtilsTest.java +++ b/src/test/java/fr/xephi/authme/util/UtilsTest.java @@ -1,22 +1,16 @@ package fr.xephi.authme.util; import fr.xephi.authme.AuthMe; -import fr.xephi.authme.AuthMeMockUtil; -import fr.xephi.authme.ConsoleLogger; +import fr.xephi.authme.ReflectionTestUtils; import fr.xephi.authme.permission.PermissionsManager; import fr.xephi.authme.settings.Settings; import org.bukkit.GameMode; import org.bukkit.entity.Player; import org.junit.Before; import org.junit.BeforeClass; -import org.junit.Ignore; import org.junit.Test; -import org.mockito.Mockito; -import java.io.File; -import java.lang.reflect.Field; import java.util.Collection; -import java.util.logging.Logger; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasSize; @@ -41,7 +35,6 @@ public class UtilsTest { @BeforeClass public static void setUpMocks() { wrapperMock = WrapperMock.createInstance(); - wrapperMock.setDataFolder(new File("/")); authMeMock = wrapperMock.getAuthMe(); } @@ -119,11 +112,12 @@ public class UtilsTest { // Note ljacqu 20151122: This is a heavy test setup with reflections... If it causes trouble, skip it with @Ignore public void shouldRetrieveListOfOnlinePlayersFromReflectedMethod() { // given - setField("getOnlinePlayersIsCollection", false); + ReflectionTestUtils.setField(Utils.class, null, "getOnlinePlayersIsCollection", false); try { - setField("getOnlinePlayers", UtilsTest.class.getDeclaredMethod("onlinePlayersImpl")); + ReflectionTestUtils.setField(Utils.class, null, "getOnlinePlayers", + UtilsTest.class.getDeclaredMethod("onlinePlayersImpl")); } catch (NoSuchMethodException e) { - throw new RuntimeException("Cannot initialize test with custom test method", e); + throw new RuntimeException("Could not get method onlinePlayersImpl() in test class", e); } // when @@ -133,16 +127,6 @@ public class UtilsTest { assertThat(players, hasSize(2)); } - private static void setField(String name, Object value) { - try { - Field field = Utils.class.getDeclaredField(name); - field.setAccessible(true); - field.set(null, value); - } catch (NoSuchFieldException | IllegalAccessException e) { - throw new RuntimeException("Could not set field '" + name + "'", e); - } - } - // Note: This method is used through reflections @SuppressWarnings("unused") public static Player[] onlinePlayersImpl() { diff --git a/src/test/java/fr/xephi/authme/util/WrapperMock.java b/src/test/java/fr/xephi/authme/util/WrapperMock.java index ed751cf03..efc5fa10f 100644 --- a/src/test/java/fr/xephi/authme/util/WrapperMock.java +++ b/src/test/java/fr/xephi/authme/util/WrapperMock.java @@ -21,14 +21,14 @@ public class WrapperMock extends Wrapper { private Map, Object> mocks = new HashMap<>(); private static WrapperMock singleton; - private File getDataFolderValue; + private File getDataFolderValue = new File("/"); private WrapperMock() { super(); } /** - * Create a new instance of the WrapperMock and inject it as singleton into Wrapper. + * Create a new instance of the WrapperMock and inject it as singleton into the Wrapper class. * * @return The created singleton */ @@ -38,16 +38,6 @@ public class WrapperMock extends Wrapper { return singleton; } - /** - * Return the WrapperMock singleton or null if it hasn't been initialized. To avoid confusion, it may be best to - * only call {@link WrapperMock#createInstance()} and to keep a reference to the returned singleton. - * - * @return The singleton or null - */ - public static WrapperMock getInstance() { - return singleton; - } - @Override public Logger getLogger() { return getMock(Logger.class); @@ -86,6 +76,12 @@ public class WrapperMock extends Wrapper { return getMock(File.class); } + /** + * Set the data folder to be returned for test contexts. Defaults to File("/"); supply null to make WrapperMock + * return a mock of the File class as with the other fields. + * + * @param file The data folder location to return + */ public void setDataFolder(File file) { this.getDataFolderValue = file; }