From e7ba579960a5ca24dc21b95df07fe1ca849c3fa1 Mon Sep 17 00:00:00 2001 From: ljacqu Date: Sun, 19 Jun 2016 22:54:12 +0200 Subject: [PATCH] #778 Delayed runner: add support for annotations, add validation - Add support for dependencies identified by annotations - Add some more usage validation - Change a few test classes to use the DelayedInjectionRunner --- .../authme/AuthMeInitializationTest.java | 2 - .../authme/command/CommandHandlerTest.java | 15 +-- .../converter/CrazyLoginConverterTest.java | 14 +-- .../authme/runner/DelayedInjectionRunner.java | 53 ++------- .../DelayedInjectionRunnerValidator.java | 36 ++++++ .../authme/runner/InjectionResolver.java | 104 ++++++++++++++++++ .../xephi/authme/runner/PendingInjection.java | 45 ++------ .../authme/runner/RunDelayedInjects.java | 15 ++- .../authme/settings/SpawnLoaderTest.java | 27 ++++- 9 files changed, 203 insertions(+), 108 deletions(-) create mode 100644 src/test/java/fr/xephi/authme/runner/DelayedInjectionRunnerValidator.java create mode 100644 src/test/java/fr/xephi/authme/runner/InjectionResolver.java diff --git a/src/test/java/fr/xephi/authme/AuthMeInitializationTest.java b/src/test/java/fr/xephi/authme/AuthMeInitializationTest.java index 1fcf7ff4a..25e95f83f 100644 --- a/src/test/java/fr/xephi/authme/AuthMeInitializationTest.java +++ b/src/test/java/fr/xephi/authme/AuthMeInitializationTest.java @@ -7,7 +7,6 @@ import fr.xephi.authme.datasource.DataSource; import fr.xephi.authme.initialization.AuthMeServiceInitializer; import fr.xephi.authme.initialization.DataFolder; import fr.xephi.authme.listener.AuthMeBlockListener; -import fr.xephi.authme.output.Messages; import fr.xephi.authme.permission.PermissionsManager; import fr.xephi.authme.process.Management; import fr.xephi.authme.process.login.ProcessSyncPlayerLogin; @@ -106,7 +105,6 @@ public class AuthMeInitializationTest { initializer.register(AuthMe.class, authMe); initializer.register(NewSetting.class, settings); initializer.register(DataSource.class, mock(DataSource.class)); - initializer.register(Messages.class, mock(Messages.class)); // when authMe.instantiateServices(initializer); diff --git a/src/test/java/fr/xephi/authme/command/CommandHandlerTest.java b/src/test/java/fr/xephi/authme/command/CommandHandlerTest.java index 137d9f1f8..85ae92a76 100644 --- a/src/test/java/fr/xephi/authme/command/CommandHandlerTest.java +++ b/src/test/java/fr/xephi/authme/command/CommandHandlerTest.java @@ -7,14 +7,15 @@ import fr.xephi.authme.command.TestCommandsUtil.TestUnregisterCommand; import fr.xephi.authme.command.help.HelpProvider; import fr.xephi.authme.initialization.AuthMeServiceInitializer; import fr.xephi.authme.permission.PermissionsManager; +import fr.xephi.authme.runner.BeforeInjecting; +import fr.xephi.authme.runner.DelayedInjectionRunner; +import fr.xephi.authme.runner.InjectDelayed; import org.bukkit.command.CommandSender; -import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.ArgumentCaptor; import org.mockito.Mock; import org.mockito.invocation.InvocationOnMock; -import org.mockito.runners.MockitoJUnitRunner; import org.mockito.stubbing.Answer; import java.util.Collections; @@ -48,9 +49,10 @@ import static org.mockito.Mockito.verify; // Justification: It's more readable to use asList() everywhere in the test when we often generated two lists where one // often consists of only one element, e.g. myMethod(asList("authme"), asList("my", "args"), ...) @SuppressWarnings("ArraysAsListWithZeroOrOneArgument") -@RunWith(MockitoJUnitRunner.class) +@RunWith(DelayedInjectionRunner.class) public class CommandHandlerTest { + @InjectDelayed private CommandHandler handler; @Mock @@ -64,13 +66,12 @@ public class CommandHandlerTest { private Map, ExecutableCommand> mockedCommands = new HashMap<>(); - @Before + @BeforeInjecting @SuppressWarnings("unchecked") public void initializeCommandMapper() { - given(commandMapper.getCommandClasses()).willReturn(Sets.newHashSet(ExecutableCommand.class, - TestLoginCommand.class, TestRegisterCommand.class, TestUnregisterCommand.class)); + given(commandMapper.getCommandClasses()).willReturn(Sets.newHashSet( + ExecutableCommand.class, TestLoginCommand.class, TestRegisterCommand.class, TestUnregisterCommand.class)); setInjectorToMockExecutableCommandClasses(); - handler = new CommandHandler(initializer, commandMapper, permissionsManager, helpProvider); } /** diff --git a/src/test/java/fr/xephi/authme/converter/CrazyLoginConverterTest.java b/src/test/java/fr/xephi/authme/converter/CrazyLoginConverterTest.java index e64fb97cb..0183c613e 100644 --- a/src/test/java/fr/xephi/authme/converter/CrazyLoginConverterTest.java +++ b/src/test/java/fr/xephi/authme/converter/CrazyLoginConverterTest.java @@ -3,16 +3,17 @@ package fr.xephi.authme.converter; import fr.xephi.authme.TestHelper; import fr.xephi.authme.cache.auth.PlayerAuth; import fr.xephi.authme.datasource.DataSource; +import fr.xephi.authme.initialization.DataFolder; +import fr.xephi.authme.runner.DelayedInjectionRunner; +import fr.xephi.authme.runner.InjectDelayed; import fr.xephi.authme.settings.NewSetting; import fr.xephi.authme.settings.properties.ConverterSettings; import org.bukkit.command.CommandSender; -import org.junit.Before; import org.junit.BeforeClass; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.ArgumentCaptor; import org.mockito.Mock; -import org.mockito.runners.MockitoJUnitRunner; import java.io.File; import java.util.List; @@ -31,9 +32,10 @@ import static org.mockito.Mockito.verifyZeroInteractions; /** * Test for {@link CrazyLoginConverter}. */ -@RunWith(MockitoJUnitRunner.class) +@RunWith(DelayedInjectionRunner.class) public class CrazyLoginConverterTest { + @InjectDelayed private CrazyLoginConverter crazyLoginConverter; @Mock @@ -42,6 +44,7 @@ public class CrazyLoginConverterTest { @Mock private NewSetting settings; + @DataFolder private File dataFolder = TestHelper.getJarFile("/converter/"); @BeforeClass @@ -49,11 +52,6 @@ public class CrazyLoginConverterTest { TestHelper.setupLogger(); } - @Before - public void instantiateConverter() { - crazyLoginConverter = new CrazyLoginConverter(dataFolder, dataSource, settings); - } - @Test public void shouldImportUsers() { // given diff --git a/src/test/java/fr/xephi/authme/runner/DelayedInjectionRunner.java b/src/test/java/fr/xephi/authme/runner/DelayedInjectionRunner.java index 27f328fb8..6d072e5f6 100644 --- a/src/test/java/fr/xephi/authme/runner/DelayedInjectionRunner.java +++ b/src/test/java/fr/xephi/authme/runner/DelayedInjectionRunner.java @@ -1,6 +1,5 @@ package fr.xephi.authme.runner; -import fr.xephi.authme.ReflectionTestUtils; import fr.xephi.authme.initialization.Injection; import fr.xephi.authme.initialization.InjectionHelper; import org.junit.runner.notification.RunNotifier; @@ -11,13 +10,9 @@ import org.junit.runners.model.InitializationError; import org.junit.runners.model.Statement; import org.mockito.Mock; import org.mockito.MockitoAnnotations; -import org.mockito.internal.runners.util.FrameworkUsageValidator; -import java.lang.reflect.Field; import java.util.ArrayList; -import java.util.HashMap; import java.util.List; -import java.util.Map; /** * Custom JUnit runner which adds support for {@link InjectDelayed} and {@link BeforeInjecting}. @@ -63,7 +58,7 @@ public class DelayedInjectionRunner extends BlockJUnit4ClassRunner { @Override public void run(final RunNotifier notifier) { // add listener that validates framework usage at the end of each test - notifier.addListener(new FrameworkUsageValidator(notifier)); + notifier.addListener(new DelayedInjectionRunnerValidator(notifier, getTestClass())); super.run(notifier); } @@ -87,53 +82,23 @@ public class DelayedInjectionRunner extends BlockJUnit4ClassRunner { List pendingInjections = new ArrayList<>(delayedFields.size()); for (FrameworkField field : delayedFields) { - pendingInjections.add(createPendingInjection(target, field.getField())); + pendingInjections.add(new PendingInjection(field.getField(), getInjection(field))); } - return new RunDelayedInjects(statement, pendingInjections, target); + InjectionResolver injectionResolver = new InjectionResolver(getTestClass(), target); + return new RunDelayedInjects(statement, pendingInjections, target, injectionResolver); } /** - * Creates a {@link PendingInjection} for the given field's type, using the target's values. + * Gets the injection method for the given field's type and ensures an injection method has been found. * - * @param target the target to get dependencies from - * @param field the field to prepare an injection for - * @return the resulting object + * @param field the field to get the injection for + * @return the injection */ - private PendingInjection createPendingInjection(Object target, Field field) { + private static Injection getInjection(FrameworkField field) { final Injection injection = InjectionHelper.getInjection(field.getType()); if (injection == null) { throw new IllegalStateException("No injection method available for field '" + field.getName() + "'"); } - final Object[] dependencies = fulfillDependencies(target, injection.getDependencies()); - return new PendingInjection(field, injection, dependencies); - } - - /** - * Returns a list of all objects for the given list of dependencies, retrieved from the given - * target's {@link @Mock} fields. - * - * @param target the target to get the required dependencies from - * @param dependencies the required dependency types - * @return the resolved dependencies - */ - private Object[] fulfillDependencies(Object target, Class[] dependencies) { - List availableMocks = getTestClass().getAnnotatedFields(Mock.class); - Map, Object> mocksByType = new HashMap<>(); - for (FrameworkField frameworkField : availableMocks) { - Field field = frameworkField.getField(); - Object fieldValue = ReflectionTestUtils.getFieldValue(field, target); - mocksByType.put(field.getType(), fieldValue); - } - - Object[] resolvedValues = new Object[dependencies.length]; - for (int i = 0; i < dependencies.length; ++i) { - Object o = mocksByType.get(dependencies[i]); - if (o == null) { - throw new IllegalStateException("No mock found for '" + dependencies[i] + "'. " - + "All dependencies of @InjectDelayed must be provided as @Mock fields"); - } - resolvedValues[i] = o; - } - return resolvedValues; + return injection; } } diff --git a/src/test/java/fr/xephi/authme/runner/DelayedInjectionRunnerValidator.java b/src/test/java/fr/xephi/authme/runner/DelayedInjectionRunnerValidator.java new file mode 100644 index 000000000..654dc90f9 --- /dev/null +++ b/src/test/java/fr/xephi/authme/runner/DelayedInjectionRunnerValidator.java @@ -0,0 +1,36 @@ +package fr.xephi.authme.runner; + +import org.junit.runner.Description; +import org.junit.runner.notification.Failure; +import org.junit.runner.notification.RunListener; +import org.junit.runner.notification.RunNotifier; +import org.junit.runners.model.TestClass; +import org.mockito.InjectMocks; +import org.mockito.Mockito; + +/** + * Validates that {@link DelayedInjectionRunner} is used as intended. + */ +class DelayedInjectionRunnerValidator extends RunListener { + + private final RunNotifier notifier; + private final TestClass testClass; + + public DelayedInjectionRunnerValidator(RunNotifier notifier, TestClass testClass) { + this.notifier = notifier; + this.testClass = testClass; + } + + @Override + public void testFinished(Description description) throws Exception { + try { + Mockito.validateMockitoUsage(); + if (!testClass.getAnnotatedFields(InjectMocks.class).isEmpty()) { + throw new IllegalStateException("Do not use @InjectMocks with the DelayedInjectionRunner:" + + " use @InjectDelayed or change runner"); + } + } catch (Throwable t) { + notifier.fireTestFailure(new Failure(description, t)); + } + } +} diff --git a/src/test/java/fr/xephi/authme/runner/InjectionResolver.java b/src/test/java/fr/xephi/authme/runner/InjectionResolver.java new file mode 100644 index 000000000..37477ee09 --- /dev/null +++ b/src/test/java/fr/xephi/authme/runner/InjectionResolver.java @@ -0,0 +1,104 @@ +package fr.xephi.authme.runner; + +import fr.xephi.authme.ReflectionTestUtils; +import fr.xephi.authme.initialization.Injection; +import fr.xephi.authme.initialization.InjectionHelper; +import org.junit.runners.model.FrameworkField; +import org.junit.runners.model.TestClass; +import org.mockito.Mock; + +import java.lang.annotation.Annotation; +import java.lang.reflect.Field; +import java.lang.reflect.Method; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +/** + * Resolves the dependencies of an injection based on the provided {@link TestClass} and {@link #target target}. + */ +class InjectionResolver { + + private final TestClass testClass; + private final Object target; + private final Map, Object> mocksByType; + + public InjectionResolver(TestClass testClass, Object target) { + this.testClass = testClass; + this.target = target; + this.mocksByType = gatherAvailableMocks(); + } + + public Object instantiate(Injection injection) { + Object[] dependencies = resolveDependencies(injection); + Object object = injection.instantiateWith(dependencies); + executePostConstructMethod(object); + return object; + } + + /** + * Returns a list of all objects for the given list of dependencies, retrieved from the given + * target's {@link Mock} fields. + * + * @param injection the injection whose dependencies to gather + * @return the resolved dependencies + */ + private Object[] resolveDependencies(Injection injection) { + final Class[] dependencies = injection.getDependencies(); + final Class[] annotations = injection.getDependencyAnnotations(); + Object[] resolvedValues = new Object[dependencies.length]; + for (int i = 0; i < dependencies.length; ++i) { + Object dependency = (annotations[i] == null) + ? resolveDependency(dependencies[i]) + : resolveAnnotation(annotations[i]); + resolvedValues[i] = dependency; + } + return resolvedValues; + } + + private Object resolveDependency(Class clazz) { + Object o = mocksByType.get(clazz); + if (o == null) { + throw new IllegalStateException("No mock found for '" + clazz + "'. " + + "All dependencies of @InjectDelayed must be provided as @Mock fields"); + } + return o; + } + + private Object resolveAnnotation(Class clazz) { + Class annotation = (Class) clazz; + List matches = testClass.getAnnotatedFields(annotation); + if (matches.isEmpty()) { + throw new IllegalStateException("No field found with @" + annotation.getSimpleName() + " in test class," + + "but a dependency in an @InjectDelayed field is using it"); + } else if (matches.size() > 1) { + throw new IllegalStateException("You cannot have multiple fields with @" + annotation.getSimpleName()); + } + return ReflectionTestUtils.getFieldValue(matches.get(0).getField(), target); + } + + /** + * Executes the class' PostConstruct method if available. Validates that all rules for + * {@link javax.annotation.PostConstruct} are met. + * + * @param object the object whose PostConstruct method should be run, if available + * @see InjectionHelper#getAndValidatePostConstructMethod + */ + private static void executePostConstructMethod(Object object) { + Method postConstructMethod = InjectionHelper.getAndValidatePostConstructMethod(object.getClass()); + if (postConstructMethod != null) { + ReflectionTestUtils.invokeMethod(postConstructMethod, object); + } + } + + private Map, Object> gatherAvailableMocks() { + List availableMocks = testClass.getAnnotatedFields(Mock.class); + Map, Object> mocksByType = new HashMap<>(); + for (FrameworkField frameworkField : availableMocks) { + Field field = frameworkField.getField(); + Object fieldValue = ReflectionTestUtils.getFieldValue(field, target); + mocksByType.put(field.getType(), fieldValue); + } + return mocksByType; + } +} diff --git a/src/test/java/fr/xephi/authme/runner/PendingInjection.java b/src/test/java/fr/xephi/authme/runner/PendingInjection.java index f326d3ccf..25d33f473 100644 --- a/src/test/java/fr/xephi/authme/runner/PendingInjection.java +++ b/src/test/java/fr/xephi/authme/runner/PendingInjection.java @@ -1,36 +1,29 @@ package fr.xephi.authme.runner; -import fr.xephi.authme.ReflectionTestUtils; import fr.xephi.authme.initialization.Injection; -import fr.xephi.authme.initialization.InjectionHelper; import java.lang.reflect.Field; -import java.lang.reflect.Method; /** - * Contains all necessary information to initialize a {@link InjectDelayed} field. + * Contains an injection and the field it's for. */ class PendingInjection { - private Field field; - private Object[] dependencies; - private Injection injection; + private final Field field; + private final Injection injection; - public PendingInjection(Field field, Injection injection, Object[] dependencies) { + public PendingInjection(Field field, Injection injection) { this.field = field; this.injection = injection; - this.dependencies = dependencies; } /** - * Constructs an object with the stored injection information. + * Returns the injection to perform. * - * @return the constructed object + * @return the injection */ - public Object instantiate() { - Object object = injection.instantiateWith(dependencies); - executePostConstructMethod(object); - return object; + public Injection getInjection() { + return injection; } /** @@ -42,26 +35,4 @@ class PendingInjection { return field; } - /** - * Clears all fields (avoids keeping a reference to all dependencies). - */ - public void clearFields() { - field = null; - dependencies = null; - injection = null; - } - - /** - * Executes the class' PostConstruct method if available. Validates that all rules for - * {@link javax.annotation.PostConstruct} are met. - * - * @param object the object whose PostConstruct method should be run, if available - * @see InjectionHelper#getAndValidatePostConstructMethod - */ - private static void executePostConstructMethod(Object object) { - Method postConstructMethod = InjectionHelper.getAndValidatePostConstructMethod(object.getClass()); - if (postConstructMethod != null) { - ReflectionTestUtils.invokeMethod(postConstructMethod, object); - } - } } diff --git a/src/test/java/fr/xephi/authme/runner/RunDelayedInjects.java b/src/test/java/fr/xephi/authme/runner/RunDelayedInjects.java index ec0026dfb..b59d71588 100644 --- a/src/test/java/fr/xephi/authme/runner/RunDelayedInjects.java +++ b/src/test/java/fr/xephi/authme/runner/RunDelayedInjects.java @@ -13,21 +13,28 @@ class RunDelayedInjects extends Statement { private final Statement next; private final Object target; - private final List pendingInjections; + private List pendingInjections; + private InjectionResolver injectionResolver; - public RunDelayedInjects(Statement next, List pendingInjections, Object target) { + public RunDelayedInjects(Statement next, List pendingInjections, Object target, + InjectionResolver injectionResolver) { this.next = next; this.pendingInjections = pendingInjections; this.target = target; + this.injectionResolver = injectionResolver; } @Override public void evaluate() throws Throwable { for (PendingInjection pendingInjection : pendingInjections) { - Object object = pendingInjection.instantiate(); + if (ReflectionTestUtils.getFieldValue(pendingInjection.getField(), target) != null) { + throw new IllegalStateException("Field with @InjectDelayed must be null on startup"); + } + Object object = injectionResolver.instantiate(pendingInjection.getInjection()); ReflectionTestUtils.setField(pendingInjection.getField(), target, object); - pendingInjection.clearFields(); } + this.pendingInjections = null; + this.injectionResolver = null; next.evaluate(); } } diff --git a/src/test/java/fr/xephi/authme/settings/SpawnLoaderTest.java b/src/test/java/fr/xephi/authme/settings/SpawnLoaderTest.java index 06088e194..c489e785f 100644 --- a/src/test/java/fr/xephi/authme/settings/SpawnLoaderTest.java +++ b/src/test/java/fr/xephi/authme/settings/SpawnLoaderTest.java @@ -4,14 +4,19 @@ import com.google.common.io.Files; import fr.xephi.authme.TestHelper; import fr.xephi.authme.datasource.DataSource; import fr.xephi.authme.hooks.PluginHooks; +import fr.xephi.authme.initialization.DataFolder; +import fr.xephi.authme.runner.BeforeInjecting; +import fr.xephi.authme.runner.DelayedInjectionRunner; +import fr.xephi.authme.runner.InjectDelayed; import fr.xephi.authme.settings.properties.RestrictionSettings; import org.bukkit.Location; import org.bukkit.World; import org.bukkit.configuration.file.YamlConfiguration; -import org.junit.Before; import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; +import org.junit.runner.RunWith; +import org.mockito.Mock; import java.io.File; import java.io.IOException; @@ -24,15 +29,28 @@ import static org.mockito.Mockito.mock; /** * Test for {@link SpawnLoader}. */ +@RunWith(DelayedInjectionRunner.class) public class SpawnLoaderTest { + @InjectDelayed + private SpawnLoader spawnLoader; + + @Mock + private NewSetting settings; + + @Mock + private DataSource dataSource; + + @Mock + private PluginHooks pluginHooks; + @Rule public TemporaryFolder temporaryFolder = new TemporaryFolder(); + @DataFolder private File testFolder; - private NewSetting settings; - @Before + @BeforeInjecting public void setup() throws IOException { // Copy test config into a new temporary folder testFolder = temporaryFolder.newFolder(); @@ -41,7 +59,6 @@ public class SpawnLoaderTest { Files.copy(source, destination); // Create a settings mock with default values - settings = mock(NewSetting.class); given(settings.getProperty(RestrictionSettings.SPAWN_PRIORITY)) .willReturn("authme, essentials, multiverse, default"); } @@ -49,8 +66,6 @@ public class SpawnLoaderTest { @Test public void shouldSetSpawn() { // given - SpawnLoader spawnLoader = - new SpawnLoader(testFolder, settings, mock(PluginHooks.class), mock(DataSource.class)); World world = mock(World.class); given(world.getName()).willReturn("new_world"); Location newSpawn = new Location(world, 123, 45.0, -67.89);