From ba692aa3efb22c5e686d44f38cec013ba1d83045 Mon Sep 17 00:00:00 2001 From: Sam Brannen <104798+sbrannen@users.noreply.github.com> Date: Thu, 31 Oct 2024 15:34:41 +0100 Subject: [PATCH] =?UTF-8?q?Honor=20MockReset=20without=20@=E2=81=A0Mockito?= =?UTF-8?q?Bean=20or=20@=E2=81=A0MockitoSpyBean=20fields?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Prior to this commit, the static factory methods in MockReset (such as MockReset.before() and MockReset.after()) could only be applied to beans within the ApplicationContext if the test class declared at least one field annotated with either @⁠MockitoBean or @⁠MockitoSpyBean. However, the Javadoc states that it should be possible to apply MockReset directly to any mock in the ApplicationContext using the static methods in MockReset. To address that, this commit reworks the "enabled" logic in MockitoResetTestExecutionListener as follows. - We no longer check for the presence of annotations from the org.springframework.test.context.bean.override.mockito package to determine if MockReset is enabled. - Instead, we now rely on a new isEnabled() method to determine if MockReset is enabled. The logic in the isEnabled() method still relies on the mockitoPresent flag as an initial check; however, mockitoPresent only determines if Mockito is present in the classpath. It does not determine if Mockito can actually be used. For example, it does not detect if the necessary reachability metadata has been registered to use Mockito within a GraalVM native image. To address that last point, the isEnabled() method performs an additional check to determine if Mockito can be used in the current environment. Specifically, it invokes Mockito.mockingDetails().isMock() which in turn initializes core Mockito classes without actually attempting to create a mock. If that fails, that means that Mockito cannot actually be used in the current environment, which typically indicates that GraalVM reachability metadata has not been registered for the org.mockito.plugins.MockMaker in use (such as the ProxyMockMaker). In addition, isEnabled() lazily determines if Mockito can be initialized, since attempting to detect that during static initialization results in a GraalVM native image error stating that Mockito internals were "unintentionally initialized at build time". If Mockito cannot be initialized, MockitoResetTestExecutionListener logs a DEBUG level message providing access to the corresponding stack trace, and MockReset support is disabled. Closes gh-33829 --- .../MockitoResetTestExecutionListener.java | 111 ++++++++---------- ...stenerWithMockitoBeanIntegrationTests.java | 22 +++- ...outMockitoAnnotationsIntegrationTests.java | 2 - 3 files changed, 66 insertions(+), 69 deletions(-) diff --git a/spring-test/src/main/java/org/springframework/test/context/bean/override/mockito/MockitoResetTestExecutionListener.java b/spring-test/src/main/java/org/springframework/test/context/bean/override/mockito/MockitoResetTestExecutionListener.java index 06f049f922..57d4c17b55 100644 --- a/spring-test/src/main/java/org/springframework/test/context/bean/override/mockito/MockitoResetTestExecutionListener.java +++ b/spring-test/src/main/java/org/springframework/test/context/bean/override/mockito/MockitoResetTestExecutionListener.java @@ -16,13 +16,12 @@ package org.springframework.test.context.bean.override.mockito; -import java.lang.reflect.AnnotatedElement; -import java.lang.reflect.Field; import java.util.Arrays; import java.util.HashSet; import java.util.Set; -import java.util.function.Predicate; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; import org.mockito.Mockito; import org.springframework.beans.factory.BeanFactory; @@ -33,12 +32,8 @@ import org.springframework.beans.factory.config.ConfigurableListableBeanFactory; import org.springframework.context.ApplicationContext; import org.springframework.context.ConfigurableApplicationContext; import org.springframework.core.Ordered; -import org.springframework.core.annotation.MergedAnnotation; -import org.springframework.core.annotation.MergedAnnotations; -import org.springframework.core.annotation.MergedAnnotations.SearchStrategy; import org.springframework.lang.Nullable; import org.springframework.test.context.TestContext; -import org.springframework.test.context.TestContextAnnotationUtils; import org.springframework.test.context.support.AbstractTestExecutionListener; import org.springframework.util.ClassUtils; @@ -54,13 +49,28 @@ import org.springframework.util.ClassUtils; */ public class MockitoResetTestExecutionListener extends AbstractTestExecutionListener { + private static final Log logger = LogFactory.getLog(MockitoResetTestExecutionListener.class); + + /** + * Boolean flag which tracks whether Mockito is present in the classpath. + * @see #mockitoInitialized + * @see #isEnabled() + */ private static final boolean mockitoPresent = ClassUtils.isPresent("org.mockito.Mockito", MockitoResetTestExecutionListener.class.getClassLoader()); - private static final String SPRING_MOCKITO_PACKAGE = "org.springframework.test.context.bean.override.mockito"; - - private static final Predicate> isSpringMockitoAnnotation = mergedAnnotation -> - mergedAnnotation.getType().getPackageName().equals(SPRING_MOCKITO_PACKAGE); + /** + * Boolean flag which tracks whether Mockito has been successfully initialized + * in the current environment. + *

Even if {@link #mockitoPresent} evaluates to {@code true}, this flag + * may eventually evaluate to {@code false} — for example, in a GraalVM + * native image if the necessary reachability metadata has not been registered + * for the {@link org.mockito.plugins.MockMaker} in use. + * @see #mockitoPresent + * @see #isEnabled() + */ + @Nullable + private static volatile Boolean mockitoInitialized; /** @@ -73,26 +83,26 @@ public class MockitoResetTestExecutionListener extends AbstractTestExecutionList @Override public void beforeTestMethod(TestContext testContext) { - if (mockitoPresent && hasMockitoAnnotations(testContext)) { + if (isEnabled()) { resetMocks(testContext.getApplicationContext(), MockReset.BEFORE); } } @Override public void afterTestMethod(TestContext testContext) { - if (mockitoPresent && hasMockitoAnnotations(testContext)) { + if (isEnabled()) { resetMocks(testContext.getApplicationContext(), MockReset.AFTER); } } - private void resetMocks(ApplicationContext applicationContext, MockReset reset) { + private static void resetMocks(ApplicationContext applicationContext, MockReset reset) { if (applicationContext instanceof ConfigurableApplicationContext configurableContext) { resetMocks(configurableContext, reset); } } - private void resetMocks(ConfigurableApplicationContext applicationContext, MockReset reset) { + private static void resetMocks(ConfigurableApplicationContext applicationContext, MockReset reset) { ConfigurableListableBeanFactory beanFactory = applicationContext.getBeanFactory(); String[] beanNames = beanFactory.getBeanDefinitionNames(); Set instantiatedSingletons = new HashSet<>(Arrays.asList(beanFactory.getSingletonNames())); @@ -139,59 +149,36 @@ public class MockitoResetTestExecutionListener extends AbstractTestExecutionList } /** - * Determine if the test class for the supplied {@linkplain TestContext - * test context} uses any of the annotations in this package (such as - * {@link MockitoBean @MockitoBean}). + * Determine if this listener is enabled in the current environment. + * @see #mockitoPresent + * @see #mockitoInitialized */ - static boolean hasMockitoAnnotations(TestContext testContext) { - return hasMockitoAnnotations(testContext.getTestClass()); - } - - /** - * Determine if Mockito annotations are declared on the supplied class, on an - * interface it implements, on a superclass, or on an enclosing class or - * whether a field in any such class is annotated with a Mockito annotation. - */ - private static boolean hasMockitoAnnotations(Class clazz) { - // Declared on the class? - if (isAnnotated(clazz)) { - return true; + private static boolean isEnabled() { + if (!mockitoPresent) { + return false; } + Boolean enabled = mockitoInitialized; + if (enabled == null) { + try { + // Invoke isMock() on a non-null object to initialize core Mockito classes + // in order to reliably determine if this listener is "enabled" both on the + // JVM as well as within a GraalVM native image. + Mockito.mockingDetails("a string is not a mock").isMock(); - // Declared on a field? - for (Field field : clazz.getDeclaredFields()) { - if (isAnnotated(field)) { - return true; + // If we got this far, we assume Mockito is usable in the current environment. + enabled = true; } - } - - // Declared on an interface? - for (Class ifc : clazz.getInterfaces()) { - if (hasMockitoAnnotations(ifc)) { - return true; + catch (Throwable ex) { + enabled = false; + if (logger.isDebugEnabled()) { + logger.debug(""" + MockitoResetTestExecutionListener is disabled in the current environment. \ + See exception for details.""", ex); + } } + mockitoInitialized = enabled; } - - // Declared on a superclass? - Class superclass = clazz.getSuperclass(); - if (superclass != null & superclass != Object.class) { - if (hasMockitoAnnotations(superclass)) { - return true; - } - } - - // Declared on an enclosing class? - if (TestContextAnnotationUtils.searchEnclosingClass(clazz)) { - if (hasMockitoAnnotations(clazz.getEnclosingClass())) { - return true; - } - } - - return false; - } - - private static boolean isAnnotated(AnnotatedElement element) { - return MergedAnnotations.from(element, SearchStrategy.DIRECT).stream().anyMatch(isSpringMockitoAnnotation); + return enabled; } } diff --git a/spring-test/src/test/java/org/springframework/test/context/bean/override/mockito/MockitoResetTestExecutionListenerWithMockitoBeanIntegrationTests.java b/spring-test/src/test/java/org/springframework/test/context/bean/override/mockito/MockitoResetTestExecutionListenerWithMockitoBeanIntegrationTests.java index e1b450a7fd..1eedd43b55 100644 --- a/spring-test/src/test/java/org/springframework/test/context/bean/override/mockito/MockitoResetTestExecutionListenerWithMockitoBeanIntegrationTests.java +++ b/spring-test/src/test/java/org/springframework/test/context/bean/override/mockito/MockitoResetTestExecutionListenerWithMockitoBeanIntegrationTests.java @@ -18,6 +18,9 @@ package org.springframework.test.context.bean.override.mockito; import org.junit.jupiter.api.Test; +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.BDDMockito.given; + /** * Integration tests for {@link MockitoResetTestExecutionListener} with a * {@link MockitoBean @MockitoBean} field. @@ -29,15 +32,24 @@ import org.junit.jupiter.api.Test; class MockitoResetTestExecutionListenerWithMockitoBeanIntegrationTests extends MockitoResetTestExecutionListenerWithoutMockitoAnnotationsIntegrationTests { - // The following mock is not used but is currently required to trigger support for MockReset. + // We declare the following to ensure that MockReset is also supported with + // @MockitoBean or @MockitoSpyBean fields present in the test class. @MockitoBean - StringBuilder unusedVariable; + PuzzleService puzzleService; + // test001() and test002() are in the superclass. + @Test - @Override - void test002() { - super.test002(); + void test003() { + given(puzzleService.getAnswer()).willReturn("enigma"); + assertThat(puzzleService.getAnswer()).isEqualTo("enigma"); + } + + + interface PuzzleService { + + String getAnswer(); } } diff --git a/spring-test/src/test/java/org/springframework/test/context/bean/override/mockito/MockitoResetTestExecutionListenerWithoutMockitoAnnotationsIntegrationTests.java b/spring-test/src/test/java/org/springframework/test/context/bean/override/mockito/MockitoResetTestExecutionListenerWithoutMockitoAnnotationsIntegrationTests.java index 790a8140f8..75978c416e 100644 --- a/spring-test/src/test/java/org/springframework/test/context/bean/override/mockito/MockitoResetTestExecutionListenerWithoutMockitoAnnotationsIntegrationTests.java +++ b/spring-test/src/test/java/org/springframework/test/context/bean/override/mockito/MockitoResetTestExecutionListenerWithoutMockitoAnnotationsIntegrationTests.java @@ -16,7 +16,6 @@ package org.springframework.test.context.bean.override.mockito; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.MethodOrderer; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.TestMethodOrder; @@ -76,7 +75,6 @@ class MockitoResetTestExecutionListenerWithoutMockitoAnnotationsIntegrationTests assertThat(context.getBean(NonSingletonFactoryBean.class).getObjectInvocations).isEqualTo(2); } - @Disabled("MockReset is currently only honored if @MockitoBean or @MockitoSpyBean is used") @Test void test002() { // Should not have been reset.