From e706fc2a8b5a3535f8fe7b676c4991286a836705 Mon Sep 17 00:00:00 2001 From: Rafael Winterhalter Date: Sun, 12 Jul 2020 00:03:23 +0200 Subject: [PATCH] Fixed \#1967: Correctly handle mocks with limited life-cycle in listeners. --- src/main/java/org/mockito/MockedStatic.java | 10 +++ .../mockito/internal/MockedStaticImpl.java | 10 +++ .../MockAnnotationProcessor.java | 30 +++++---- .../framework/DefaultMockitoSession.java | 62 ++++++++++--------- .../finder/AllInvocationsFinder.java | 6 ++ .../MockAnnotationProcessorTest.java | 49 +++++++++++++++ 6 files changed, 125 insertions(+), 42 deletions(-) create mode 100644 src/test/java/org/mockito/internal/configuration/MockAnnotationProcessorTest.java diff --git a/src/main/java/org/mockito/MockedStatic.java b/src/main/java/org/mockito/MockedStatic.java index 19ed5e944c..fc23c90776 100644 --- a/src/main/java/org/mockito/MockedStatic.java +++ b/src/main/java/org/mockito/MockedStatic.java @@ -63,6 +63,16 @@ default void verify(Verification verification) { */ void verifyNoInteractions(); + /** + * Checks if this mock is closed. + * + * @return {@code true} if this mock is closed. + */ + boolean isClosed(); + + /** + * Releases this static mock and throws a {@link org.mockito.exceptions.base.MockitoException} if closed already. + */ @Override void close(); diff --git a/src/main/java/org/mockito/internal/MockedStaticImpl.java b/src/main/java/org/mockito/internal/MockedStaticImpl.java index 908b810be0..25434769e2 100644 --- a/src/main/java/org/mockito/internal/MockedStaticImpl.java +++ b/src/main/java/org/mockito/internal/MockedStaticImpl.java @@ -157,6 +157,11 @@ public void verifyNoInteractions() { noInteractions().verify(data); } + @Override + public boolean isClosed() { + return closed; + } + @Override public void close() { assertNotClosed(); @@ -181,4 +186,9 @@ private void assertNotClosed() { "is already resolved and cannot longer be used")); } } + + @Override + public String toString() { + return "static mock for " + control.getType().getTypeName(); + } } diff --git a/src/main/java/org/mockito/internal/configuration/MockAnnotationProcessor.java b/src/main/java/org/mockito/internal/configuration/MockAnnotationProcessor.java index c7f562a830..48f0515a4b 100644 --- a/src/main/java/org/mockito/internal/configuration/MockAnnotationProcessor.java +++ b/src/main/java/org/mockito/internal/configuration/MockAnnotationProcessor.java @@ -58,21 +58,25 @@ public static Object processAnnotationForMock( } } - private static Class inferStaticMock(Type type, String name) { + static Class inferStaticMock(Type type, String name) { if (type instanceof ParameterizedType) { ParameterizedType parameterizedType = (ParameterizedType) type; - return (Class) parameterizedType.getRawType(); - } else { - throw new MockitoException( - join( - "Mockito cannot infer a static mock from a raw type for " + name, - "", - "Instead of @Mock MockedStatic you need to specify a parameterized type", - "For example, if you would like to mock static methods of Sample.class, specify", - "", - "@Mock MockedStatic", - "", - "as the type parameter")); + Type[] arguments = parameterizedType.getActualTypeArguments(); + if (arguments.length == 1) { + if (arguments[0] instanceof Class) { + return (Class) arguments[0]; + } + } } + throw new MockitoException( + join( + "Mockito cannot infer a static mock from a raw type for " + name, + "", + "Instead of @Mock MockedStatic you need to specify a parameterized type", + "For example, if you would like to mock static methods of Sample.class, specify", + "", + "@Mock MockedStatic", + "", + "as the type parameter. If the type is parameterized, it should be specified as raw type.")); } } diff --git a/src/main/java/org/mockito/internal/framework/DefaultMockitoSession.java b/src/main/java/org/mockito/internal/framework/DefaultMockitoSession.java index dfd5acce9c..549019d135 100644 --- a/src/main/java/org/mockito/internal/framework/DefaultMockitoSession.java +++ b/src/main/java/org/mockito/internal/framework/DefaultMockitoSession.java @@ -43,7 +43,11 @@ public DefaultMockitoSession( closeables.add(MockitoAnnotations.openMocks(testClassInstance)); } } catch (RuntimeException e) { - release(); + try { + release(); + } catch (Throwable t) { + e.addSuppressed(t); + } // clean up in case 'openMocks' fails listener.setListenerDirty(); @@ -58,38 +62,38 @@ public void setStrictness(Strictness strictness) { @Override public void finishMocking() { - release(); - finishMocking(null); } @Override public void finishMocking(final Throwable failure) { - release(); - - // Cleaning up the state, we no longer need the listener hooked up - // The listener implements MockCreationListener and at this point - // we no longer need to listen on mock creation events. We are wrapping up the session - Mockito.framework().removeListener(listener); - - // Emit test finished event so that validation such as strict stubbing can take place - listener.testFinished( - new TestFinishedEvent() { - @Override - public Throwable getFailure() { - return failure; - } - - @Override - public String getTestName() { - return name; - } - }); - - // Validate only when there is no test failure to avoid reporting multiple problems - if (failure == null) { - // Finally, validate user's misuse of Mockito framework. - Mockito.validateMockitoUsage(); + try { + // Cleaning up the state, we no longer need the listener hooked up + // The listener implements MockCreationListener and at this point + // we no longer need to listen on mock creation events. We are wrapping up the session + Mockito.framework().removeListener(listener); + + // Emit test finished event so that validation such as strict stubbing can take place + listener.testFinished( + new TestFinishedEvent() { + @Override + public Throwable getFailure() { + return failure; + } + + @Override + public String getTestName() { + return name; + } + }); + + // Validate only when there is no test failure to avoid reporting multiple problems + if (failure == null) { + // Finally, validate user's misuse of Mockito framework. + Mockito.validateMockitoUsage(); + } + } finally { + release(); } } @@ -98,7 +102,7 @@ private void release() { try { closeable.close(); } catch (Exception e) { - throw new MockitoException("Failed to release mocks", e); + throw new MockitoException("Failed to release " + closeable, e); } } } diff --git a/src/main/java/org/mockito/internal/invocation/finder/AllInvocationsFinder.java b/src/main/java/org/mockito/internal/invocation/finder/AllInvocationsFinder.java index ea9c6d63f4..9b43bbfb1c 100644 --- a/src/main/java/org/mockito/internal/invocation/finder/AllInvocationsFinder.java +++ b/src/main/java/org/mockito/internal/invocation/finder/AllInvocationsFinder.java @@ -42,6 +42,12 @@ public static List find(Iterable mocks) { public static Set findStubbings(Iterable mocks) { Set stubbings = new TreeSet(new StubbingComparator()); for (Object mock : mocks) { + // TODO due to the limited scope of static mocks they cannot be processed + // it would rather be required to trigger this stubbing control upon releasing + // the static mock. + if (mock instanceof Class) { + continue; + } Collection fromSingleMock = new DefaultMockingDetails(mock).getStubbings(); stubbings.addAll(fromSingleMock); diff --git a/src/test/java/org/mockito/internal/configuration/MockAnnotationProcessorTest.java b/src/test/java/org/mockito/internal/configuration/MockAnnotationProcessorTest.java new file mode 100644 index 0000000000..906d134d61 --- /dev/null +++ b/src/test/java/org/mockito/internal/configuration/MockAnnotationProcessorTest.java @@ -0,0 +1,49 @@ +/* + * Copyright (c) 2020 Mockito contributors + * This program is made available under the terms of the MIT License. + */ +package org.mockito.internal.configuration; + +import java.util.List; + +import org.junit.Test; +import org.mockito.MockedStatic; +import org.mockito.exceptions.base.MockitoException; + +import static org.assertj.core.api.Assertions.*; + +public class MockAnnotationProcessorTest { + + @SuppressWarnings("unused") + private MockedStatic nonGeneric; + + @SuppressWarnings("unused") + private MockedStatic> generic; + + @SuppressWarnings({"raw", "unused"}) + private MockedStatic raw; + + @Test + public void testNonGeneric() throws Exception { + Class type = + MockAnnotationProcessor.inferStaticMock( + MockAnnotationProcessorTest.class + .getDeclaredField("nonGeneric") + .getGenericType(), + "nonGeneric"); + assertThat(type).isEqualTo(Void.class); + } + + @Test(expected = MockitoException.class) + public void testGeneric() throws Exception { + MockAnnotationProcessor.inferStaticMock( + MockAnnotationProcessorTest.class.getDeclaredField("generic").getGenericType(), + "generic"); + } + + @Test(expected = MockitoException.class) + public void testRaw() throws Exception { + MockAnnotationProcessor.inferStaticMock( + MockAnnotationProcessorTest.class.getDeclaredField("raw").getGenericType(), "raw"); + } +}