From 66250b1f8ec044b813e5ea8b96fbdf0af250f55c Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Fri, 15 Aug 2014 02:21:08 +0200 Subject: [PATCH] Support merging custom TELs with default TELs Prior to this commit, if a custom TestExecutionListener was registered via @TestExecutionListeners the defaults would not be registered. Thus, if a user wanted to declare a custom listener and use the default listeners, the user was forced to manually declare all default listeners in addition to any custom listeners. This unfortunately required that the user know exactly which listeners were registered by default. Moreover, the set of default listeners can change from release to release, and with the support for automatic discovery of default listeners introduced in SPR-11466 it is no longer even possible to know what the set of default TestExecutionListeners is before runtime. This commit addresses this issue by introducing a mechanism for merging custom declared listeners with the defaults for the current environment. Specifically, @TestExecutionListeners supports a new MergeMode that is used to control whether or not explicitly declared listeners are merged with the default listeners when @TestExecutionListeners is declared on a class that does not inherit listeners from a superclass. Issue: SPR-8854 --- .../test/context/TestExecutionListeners.java | 65 ++++- .../AbstractTestContextBootstrapper.java | 33 ++- .../context/TestExecutionListenersTests.java | 222 ++++++++++++------ 3 files changed, 236 insertions(+), 84 deletions(-) diff --git a/spring-test/src/main/java/org/springframework/test/context/TestExecutionListeners.java b/spring-test/src/main/java/org/springframework/test/context/TestExecutionListeners.java index 74464aa8d80..c66ccc125c9 100644 --- a/spring-test/src/main/java/org/springframework/test/context/TestExecutionListeners.java +++ b/spring-test/src/main/java/org/springframework/test/context/TestExecutionListeners.java @@ -24,9 +24,9 @@ import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; /** - * {@code TestExecutionListeners} defines class-level metadata for - * configuring which {@link TestExecutionListener TestExecutionListeners} should - * be registered with a {@link TestContextManager}. + * {@code TestExecutionListeners} defines class-level metadata for configuring + * which {@link TestExecutionListener TestExecutionListeners} should be + * registered with a {@link TestContextManager}. * *

Typically, {@code @TestExecutionListeners} will be used in conjunction with * {@link ContextConfiguration @ContextConfiguration}. @@ -47,7 +47,40 @@ import java.lang.annotation.Target; public @interface TestExecutionListeners { /** - * Alias for {@link #listeners() listeners}. + * Enumeration of modes that dictate whether or not explicitly + * declared listeners are merged with the default listeners when + * {@code @TestExecutionListeners} is declared on a class that does + * not inherit listeners from a superclass. + * @since 4.1 + */ + static enum MergeMode { + + /** + * Indicates that locally declared listeners should replace the default + * listeners. + */ + REPLACE_DEFAULTS, + + /** + * Indicates that locally declared listeners should be merged with the + * default listeners. + *

The merging algorithm ensures that duplicates are removed from + * the list and that the resulting set of merged listeners is sorted + * according to the semantics of + * {@link org.springframework.core.annotation.AnnotationAwareOrderComparator + * AnnotationAwareOrderComparator}. If a listener implements + * {@link org.springframework.core.Ordered Ordered} or is annotated + * with {@link org.springframework.core.annotation.Order @Order} it can + * influence the position in which it is merged with the defaults; otherwise, + * locally declared listeners will simply be appended to the list of default + * listeners when merged. + */ + MERGE_WITH_DEFAULTS, + } + + + /** + * Alias for {@link #listeners}. * *

This attribute may not be used in conjunction with * {@link #listeners}, but it may be used instead of {@link #listeners}. @@ -56,7 +89,7 @@ public @interface TestExecutionListeners { /** * The {@link TestExecutionListener TestExecutionListeners} to register with - * a {@link TestContextManager}. + * the {@link TestContextManager}. * *

This attribute may not be used in conjunction with * {@link #value}, but it may be used instead of {@link #value}. @@ -65,14 +98,15 @@ public @interface TestExecutionListeners { * @see org.springframework.test.context.support.DependencyInjectionTestExecutionListener * @see org.springframework.test.context.support.DirtiesContextTestExecutionListener * @see org.springframework.test.context.transaction.TransactionalTestExecutionListener + * @see org.springframework.test.context.jdbc.SqlScriptsTestExecutionListener */ Class[] listeners() default {}; /** - * Whether or not {@link #value() TestExecutionListeners} from superclasses + * Whether or not {@link #listeners TestExecutionListeners} from superclasses * should be inherited. - *

- * The default value is {@code true}, which means that an annotated + * + *

The default value is {@code true}, which means that an annotated * class will inherit the listeners defined by an annotated * superclass. Specifically, the listeners for an annotated class will be * appended to the list of listeners defined by an annotated superclass. @@ -106,4 +140,19 @@ public @interface TestExecutionListeners { */ boolean inheritListeners() default true; + /** + * The merge mode to use when {@code @TestExecutionListeners} is + * declared on a class that does not inherit listeners + * from a superclass. + *

Can be set to {@link MergeMode#MERGE_WITH_DEFAULTS MERGE_WITH_DEFAULTS} + * to have locally declared listeners merged with the default + * listeners. + *

The mode is ignored if listeners are inherited from a superclass. + *

Defaults to {@link MergeMode#REPLACE_DEFAULTS REPLACE_DEFAULTS} + * for backwards compatibility. + * @see MergeMode + * @since 4.1 + */ + MergeMode mergeMode() default MergeMode.REPLACE_DEFAULTS; + } diff --git a/spring-test/src/main/java/org/springframework/test/context/support/AbstractTestContextBootstrapper.java b/spring-test/src/main/java/org/springframework/test/context/support/AbstractTestContextBootstrapper.java index 017cd62c37d..4f97b670045 100644 --- a/spring-test/src/main/java/org/springframework/test/context/support/AbstractTestContextBootstrapper.java +++ b/spring-test/src/main/java/org/springframework/test/context/support/AbstractTestContextBootstrapper.java @@ -19,6 +19,7 @@ package org.springframework.test.context.support; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; +import java.util.HashSet; import java.util.LinkedHashSet; import java.util.List; import java.util.Map; @@ -45,6 +46,7 @@ import org.springframework.test.context.SmartContextLoader; import org.springframework.test.context.TestContextBootstrapper; import org.springframework.test.context.TestExecutionListener; import org.springframework.test.context.TestExecutionListeners; +import org.springframework.test.context.TestExecutionListeners.MergeMode; import org.springframework.test.util.MetaAnnotationUtils; import org.springframework.test.util.MetaAnnotationUtils.AnnotationDescriptor; import org.springframework.util.Assert; @@ -137,14 +139,37 @@ public abstract class AbstractTestContextBootstrapper implements TestContextBoot listenerClasses = valueListenerClasses; } - if (listenerClasses != null) { - classesList.addAll(0, Arrays.> asList(listenerClasses)); + boolean inheritListeners = annAttrs.getBoolean("inheritListeners"); + AnnotationDescriptor superDescriptor = MetaAnnotationUtils.findAnnotationDescriptor( + descriptor.getRootDeclaringClass().getSuperclass(), annotationType); + + // If there are no listeners to inherit, we might need to merge the + // locally declared listeners with the defaults. + if ((!inheritListeners || superDescriptor == null) + && (annAttrs.getEnum("mergeMode") == MergeMode.MERGE_WITH_DEFAULTS)) { + if (logger.isDebugEnabled()) { + logger.debug(String.format( + "Merging default listeners with listeners configured via @TestExecutionListeners for class [%s].", + clazz.getName())); + } + usingDefaults = true; + classesList.addAll(getDefaultTestExecutionListenerClasses()); } - descriptor = (annAttrs.getBoolean("inheritListeners") ? MetaAnnotationUtils.findAnnotationDescriptor( - descriptor.getRootDeclaringClass().getSuperclass(), annotationType) : null); + + classesList.addAll(0, Arrays.> asList(listenerClasses)); + + descriptor = (inheritListeners ? superDescriptor : null); } } + // Remove possible duplicates if we loaded default listeners. + if (usingDefaults) { + Set> classesSet = new HashSet>(); + classesSet.addAll(classesList); + classesList.clear(); + classesList.addAll(classesSet); + } + List listeners = instantiateListeners(classesList); // Sort by Ordered/@Order if we loaded default listeners. diff --git a/spring-test/src/test/java/org/springframework/test/context/TestExecutionListenersTests.java b/spring-test/src/test/java/org/springframework/test/context/TestExecutionListenersTests.java index fdc749c6946..0c3d9e669c0 100644 --- a/spring-test/src/test/java/org/springframework/test/context/TestExecutionListenersTests.java +++ b/spring-test/src/test/java/org/springframework/test/context/TestExecutionListenersTests.java @@ -18,23 +18,30 @@ package org.springframework.test.context; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; +import java.util.List; +import java.util.stream.Collectors; import org.junit.Test; +import org.springframework.core.Ordered; +import org.springframework.test.context.jdbc.SqlScriptsTestExecutionListener; import org.springframework.test.context.support.AbstractTestExecutionListener; +import org.springframework.test.context.support.DependencyInjectionTestExecutionListener; +import org.springframework.test.context.support.DirtiesContextTestExecutionListener; +import org.springframework.test.context.transaction.TransactionalTestExecutionListener; +import org.springframework.test.context.web.ServletTestExecutionListener; +import static java.util.Arrays.*; import static org.junit.Assert.*; +import static org.springframework.test.context.TestExecutionListeners.MergeMode.*; /** - *

- * JUnit 4 based unit test for the {@link TestExecutionListeners - * @TestExecutionListeners} annotation, which verifies: - *

+ * Unit tests for the {@link TestExecutionListeners @TestExecutionListeners} + * annotation, which verify: * * * @author Sam Brannen @@ -42,140 +49,195 @@ import static org.junit.Assert.*; */ public class TestExecutionListenersTests { + private List> classes(TestContextManager testContextManager) { + return testContextManager.getTestExecutionListeners().stream().map(listener -> listener.getClass()).collect( + Collectors.toList()); + } + + private List names(List> classes) { + return classes.stream().map(clazz -> clazz.getSimpleName()).collect(Collectors.toList()); + } + + private void assertRegisteredListeners(Class testClass, List> expected) { + TestContextManager testContextManager = new TestContextManager(testClass); + assertEquals("TELs registered for " + testClass.getSimpleName(), names(expected), + names(classes(testContextManager))); + } + @Test - public void verifyNumDefaultListenersRegistered() throws Exception { - TestContextManager testContextManager = new TestContextManager(DefaultListenersExampleTestCase.class); - assertEquals("Num registered TELs for DefaultListenersExampleTestCase.", 5, - testContextManager.getTestExecutionListeners().size()); + public void defaultListeners() { + List> expected = asList(ServletTestExecutionListener.class, + DependencyInjectionTestExecutionListener.class, DirtiesContextTestExecutionListener.class, + TransactionalTestExecutionListener.class, SqlScriptsTestExecutionListener.class); + assertRegisteredListeners(DefaultListenersTestCase.class, expected); } + /** + * @since 4.1 + */ @Test - public void verifyNumNonInheritedDefaultListenersRegistered() throws Exception { - TestContextManager testContextManager = new TestContextManager( - NonInheritedDefaultListenersExampleTestCase.class); - assertEquals("Num registered TELs for NonInheritedDefaultListenersExampleTestCase.", 1, - testContextManager.getTestExecutionListeners().size()); + public void defaultListenersMergedWithCustomListenerPrepended() { + List> expected = asList(QuuxTestExecutionListener.class, ServletTestExecutionListener.class, + DependencyInjectionTestExecutionListener.class, DirtiesContextTestExecutionListener.class, + TransactionalTestExecutionListener.class, SqlScriptsTestExecutionListener.class); + assertRegisteredListeners(MergedDefaultListenersWithCustomListenerPrependedTestCase.class, expected); } + /** + * @since 4.1 + */ @Test - public void verifyNumInheritedDefaultListenersRegistered() throws Exception { - TestContextManager testContextManager = new TestContextManager(InheritedDefaultListenersExampleTestCase.class); - assertEquals("Num registered TELs for InheritedDefaultListenersExampleTestCase.", 1, - testContextManager.getTestExecutionListeners().size()); + public void defaultListenersMergedWithCustomListenerAppended() { + List> expected = asList(ServletTestExecutionListener.class, + DependencyInjectionTestExecutionListener.class, DirtiesContextTestExecutionListener.class, + TransactionalTestExecutionListener.class, SqlScriptsTestExecutionListener.class, + BazTestExecutionListener.class); + assertRegisteredListeners(MergedDefaultListenersWithCustomListenerAppendedTestCase.class, expected); + } - testContextManager = new TestContextManager(SubInheritedDefaultListenersExampleTestCase.class); - assertEquals("Num registered TELs for SubInheritedDefaultListenersExampleTestCase.", 1, - testContextManager.getTestExecutionListeners().size()); + /** + * @since 4.1 + */ + @Test + public void defaultListenersMergedWithCustomListenerInserted() { + List> expected = asList(ServletTestExecutionListener.class, + DependencyInjectionTestExecutionListener.class, BarTestExecutionListener.class, + DirtiesContextTestExecutionListener.class, TransactionalTestExecutionListener.class, + SqlScriptsTestExecutionListener.class); + assertRegisteredListeners(MergedDefaultListenersWithCustomListenerInsertedTestCase.class, expected); + } - testContextManager = new TestContextManager(SubSubInheritedDefaultListenersExampleTestCase.class); - assertEquals("Num registered TELs for SubSubInheritedDefaultListenersExampleTestCase.", 2, - testContextManager.getTestExecutionListeners().size()); + @Test + public void nonInheritedDefaultListeners() { + assertRegisteredListeners(NonInheritedDefaultListenersTestCase.class, asList(QuuxTestExecutionListener.class)); } @Test - public void verifyNumListenersRegistered() throws Exception { - TestContextManager testContextManager = new TestContextManager(ExampleTestCase.class); - assertEquals("Num registered TELs for ExampleTestCase.", 3, - testContextManager.getTestExecutionListeners().size()); + public void inheritedDefaultListeners() { + assertRegisteredListeners(InheritedDefaultListenersTestCase.class, asList(QuuxTestExecutionListener.class)); + assertRegisteredListeners(SubInheritedDefaultListenersTestCase.class, asList(QuuxTestExecutionListener.class)); + assertRegisteredListeners(SubSubInheritedDefaultListenersTestCase.class, + asList(QuuxTestExecutionListener.class, EnigmaTestExecutionListener.class)); } @Test - public void verifyNumNonInheritedListenersRegistered() throws Exception { - TestContextManager testContextManager = new TestContextManager(NonInheritedListenersExampleTestCase.class); - assertEquals("Num registered TELs for NonInheritedListenersExampleTestCase.", 1, + public void customListeners() { + TestContextManager testContextManager = new TestContextManager(ExplicitListenersTestCase.class); + assertEquals("Num registered TELs for ExplicitListenersTestCase.", 3, testContextManager.getTestExecutionListeners().size()); } @Test - public void verifyNumInheritedListenersRegistered() throws Exception { - TestContextManager testContextManager = new TestContextManager(InheritedListenersExampleTestCase.class); - assertEquals("Num registered TELs for InheritedListenersExampleTestCase.", 4, + public void nonInheritedListeners() { + TestContextManager testContextManager = new TestContextManager(NonInheritedListenersTestCase.class); + assertEquals("Num registered TELs for NonInheritedListenersTestCase.", 1, testContextManager.getTestExecutionListeners().size()); } @Test - public void verifyNumListenersRegisteredViaMetaAnnotation() throws Exception { - TestContextManager testContextManager = new TestContextManager(MetaExampleTestCase.class); - assertEquals("Num registered TELs for MetaExampleTestCase.", 3, + public void inheritedListeners() { + TestContextManager testContextManager = new TestContextManager(InheritedListenersTestCase.class); + assertEquals("Num registered TELs for InheritedListenersTestCase.", 4, testContextManager.getTestExecutionListeners().size()); } @Test - public void verifyNumNonInheritedListenersRegisteredViaMetaAnnotation() throws Exception { - TestContextManager testContextManager = new TestContextManager(MetaNonInheritedListenersExampleTestCase.class); - assertEquals("Num registered TELs for MetaNonInheritedListenersExampleTestCase.", 1, + public void customListenersRegisteredViaMetaAnnotation() { + TestContextManager testContextManager = new TestContextManager(MetaTestCase.class); + assertEquals("Num registered TELs for MetaTestCase.", 3, testContextManager.getTestExecutionListeners().size()); + } + + @Test + public void nonInheritedListenersRegisteredViaMetaAnnotation() { + TestContextManager testContextManager = new TestContextManager(MetaNonInheritedListenersTestCase.class); + assertEquals("Num registered TELs for MetaNonInheritedListenersTestCase.", 1, testContextManager.getTestExecutionListeners().size()); } @Test - public void verifyNumInheritedListenersRegisteredViaMetaAnnotation() throws Exception { - TestContextManager testContextManager = new TestContextManager(MetaInheritedListenersExampleTestCase.class); - assertEquals("Num registered TELs for MetaInheritedListenersExampleTestCase.", 4, + public void inheritedListenersRegisteredViaMetaAnnotation() { + TestContextManager testContextManager = new TestContextManager(MetaInheritedListenersTestCase.class); + assertEquals("Num registered TELs for MetaInheritedListenersTestCase.", 4, testContextManager.getTestExecutionListeners().size()); } @Test - public void verifyNumListenersRegisteredViaMetaAnnotationWithOverrides() throws Exception { - TestContextManager testContextManager = new TestContextManager(MetaWithOverridesExampleTestCase.class); - assertEquals("Num registered TELs for MetaWithOverridesExampleTestCase.", 3, + public void customListenersRegisteredViaMetaAnnotationWithOverrides() { + TestContextManager testContextManager = new TestContextManager(MetaWithOverridesTestCase.class); + assertEquals("Num registered TELs for MetaWithOverridesTestCase.", 3, testContextManager.getTestExecutionListeners().size()); } @Test - public void verifyNumListenersRegisteredViaMetaAnnotationWithInheritedListenersWithOverrides() throws Exception { + public void customsListenersRegisteredViaMetaAnnotationWithInheritedListenersWithOverrides() { TestContextManager testContextManager = new TestContextManager( - MetaInheritedListenersWithOverridesExampleTestCase.class); - assertEquals("Num registered TELs for MetaInheritedListenersWithOverridesExampleTestCase.", 5, + MetaInheritedListenersWithOverridesTestCase.class); + assertEquals("Num registered TELs for MetaInheritedListenersWithOverridesTestCase.", 5, testContextManager.getTestExecutionListeners().size()); } @Test - public void verifyNumListenersRegisteredViaMetaAnnotationWithNonInheritedListenersWithOverrides() throws Exception { + public void customListenersRegisteredViaMetaAnnotationWithNonInheritedListenersWithOverrides() { TestContextManager testContextManager = new TestContextManager( - MetaNonInheritedListenersWithOverridesExampleTestCase.class); - assertEquals("Num registered TELs for MetaNonInheritedListenersWithOverridesExampleTestCase.", 8, + MetaNonInheritedListenersWithOverridesTestCase.class); + assertEquals("Num registered TELs for MetaNonInheritedListenersWithOverridesTestCase.", 8, testContextManager.getTestExecutionListeners().size()); } @Test(expected = IllegalStateException.class) - public void verifyDuplicateListenersConfigThrowsException() throws Exception { - new TestContextManager(DuplicateListenersConfigExampleTestCase.class); + public void listenersAndValueAttributesDeclared() { + new TestContextManager(DuplicateListenersConfigTestCase.class); } - static class DefaultListenersExampleTestCase { + // ------------------------------------------------------------------- + + static class DefaultListenersTestCase { + } + + @TestExecutionListeners(listeners = { QuuxTestExecutionListener.class, + DependencyInjectionTestExecutionListener.class }, mergeMode = MERGE_WITH_DEFAULTS) + static class MergedDefaultListenersWithCustomListenerPrependedTestCase { + } + + @TestExecutionListeners(listeners = BazTestExecutionListener.class, mergeMode = MERGE_WITH_DEFAULTS) + static class MergedDefaultListenersWithCustomListenerAppendedTestCase { + } + + @TestExecutionListeners(listeners = BarTestExecutionListener.class, mergeMode = MERGE_WITH_DEFAULTS) + static class MergedDefaultListenersWithCustomListenerInsertedTestCase { } @TestExecutionListeners(QuuxTestExecutionListener.class) - static class InheritedDefaultListenersExampleTestCase extends DefaultListenersExampleTestCase { + static class InheritedDefaultListenersTestCase extends DefaultListenersTestCase { } - static class SubInheritedDefaultListenersExampleTestCase extends InheritedDefaultListenersExampleTestCase { + static class SubInheritedDefaultListenersTestCase extends InheritedDefaultListenersTestCase { } @TestExecutionListeners(EnigmaTestExecutionListener.class) - static class SubSubInheritedDefaultListenersExampleTestCase extends SubInheritedDefaultListenersExampleTestCase { + static class SubSubInheritedDefaultListenersTestCase extends SubInheritedDefaultListenersTestCase { } @TestExecutionListeners(listeners = { QuuxTestExecutionListener.class }, inheritListeners = false) - static class NonInheritedDefaultListenersExampleTestCase extends InheritedDefaultListenersExampleTestCase { + static class NonInheritedDefaultListenersTestCase extends InheritedDefaultListenersTestCase { } @TestExecutionListeners({ FooTestExecutionListener.class, BarTestExecutionListener.class, BazTestExecutionListener.class }) - static class ExampleTestCase { + static class ExplicitListenersTestCase { } @TestExecutionListeners(QuuxTestExecutionListener.class) - static class InheritedListenersExampleTestCase extends ExampleTestCase { + static class InheritedListenersTestCase extends ExplicitListenersTestCase { } @TestExecutionListeners(listeners = QuuxTestExecutionListener.class, inheritListeners = false) - static class NonInheritedListenersExampleTestCase extends InheritedListenersExampleTestCase { + static class NonInheritedListenersTestCase extends InheritedListenersTestCase { } @TestExecutionListeners(listeners = FooTestExecutionListener.class, value = BarTestExecutionListener.class) - static class DuplicateListenersConfigExampleTestCase { + static class DuplicateListenersConfigTestCase { } @TestExecutionListeners({// @@ -224,15 +286,15 @@ public class TestExecutionListenersTests { } @MetaListeners - static class MetaExampleTestCase { + static class MetaTestCase { } @MetaInheritedListeners - static class MetaInheritedListenersExampleTestCase extends MetaExampleTestCase { + static class MetaInheritedListenersTestCase extends MetaTestCase { } @MetaNonInheritedListeners - static class MetaNonInheritedListenersExampleTestCase extends MetaInheritedListenersExampleTestCase { + static class MetaNonInheritedListenersTestCase extends MetaInheritedListenersTestCase { } @MetaListenersWithOverrides(listeners = {// @@ -240,11 +302,11 @@ public class TestExecutionListenersTests { BarTestExecutionListener.class,// BazTestExecutionListener.class // }) - static class MetaWithOverridesExampleTestCase { + static class MetaWithOverridesTestCase { } @MetaInheritedListenersWithOverrides(listeners = { FooTestExecutionListener.class, BarTestExecutionListener.class }) - static class MetaInheritedListenersWithOverridesExampleTestCase extends MetaWithOverridesExampleTestCase { + static class MetaInheritedListenersWithOverridesTestCase extends MetaWithOverridesTestCase { } @MetaNonInheritedListenersWithOverrides(listeners = {// @@ -253,20 +315,36 @@ public class TestExecutionListenersTests { BazTestExecutionListener.class // },// inheritListeners = true) - static class MetaNonInheritedListenersWithOverridesExampleTestCase extends - MetaInheritedListenersWithOverridesExampleTestCase { + static class MetaNonInheritedListenersWithOverridesTestCase extends MetaInheritedListenersWithOverridesTestCase { } static class FooTestExecutionListener extends AbstractTestExecutionListener { } static class BarTestExecutionListener extends AbstractTestExecutionListener { + + @Override + public int getOrder() { + // 2500 is between DependencyInjectionTestExecutionListener (2000) and + // DirtiesContextTestExecutionListener (3000) + return 2500; + } } static class BazTestExecutionListener extends AbstractTestExecutionListener { + + @Override + public int getOrder() { + return Ordered.LOWEST_PRECEDENCE; + } } static class QuuxTestExecutionListener extends AbstractTestExecutionListener { + + @Override + public int getOrder() { + return Ordered.HIGHEST_PRECEDENCE; + } } static class EnigmaTestExecutionListener extends AbstractTestExecutionListener {