From d86cb6b981fcc5d254e173dcacdeb4cc71c803e6 Mon Sep 17 00:00:00 2001 From: Andrey Kozel Date: Sun, 23 Jan 2022 21:04:04 +0300 Subject: [PATCH 1/2] Fixes #2201 : Fixed checking of declared exceptions. In case when parent contains throws keyword on its method and child overrides this method removing throws, it should be possible to mock throwing exception from child --- .../stubbing/answers/InvocationInfo.java | 22 ++++++++++++++++++- .../invocation/InvocationBuilder.java | 9 +++++++- .../stubbing/answers/InvocationInfoTest.java | 15 +++++++++++++ src/test/java/org/mockitousage/IMethods.java | 2 ++ .../java/org/mockitousage/MethodsImpl.java | 5 +++++ 5 files changed, 51 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/mockito/internal/stubbing/answers/InvocationInfo.java b/src/main/java/org/mockito/internal/stubbing/answers/InvocationInfo.java index 8bfffca2fe..59a57e2be2 100644 --- a/src/main/java/org/mockito/internal/stubbing/answers/InvocationInfo.java +++ b/src/main/java/org/mockito/internal/stubbing/answers/InvocationInfo.java @@ -6,6 +6,7 @@ import java.lang.reflect.Method; import java.lang.reflect.Modifier; +import java.util.Arrays; import org.mockito.internal.invocation.AbstractAwareMethod; import org.mockito.internal.util.MockUtil; @@ -25,6 +26,26 @@ public InvocationInfo(InvocationOnMock theInvocation) { } public boolean isValidException(Throwable throwable) { + if (isValidException(method, throwable)) { + return true; + } + + return Arrays.stream(method.getDeclaringClass().getInterfaces()) + .anyMatch(parent -> isValidExceptionForParent(parent, throwable)); + } + + private boolean isValidExceptionForParent(Class parent, Throwable throwable) { + try { + Method parentMethod = + parent.getMethod(this.method.getName(), this.method.getParameterTypes()); + return isValidException(parentMethod, throwable); + } catch (NoSuchMethodException e) { + // ignore interfaces that doesn't have such a method + return false; + } + } + + private boolean isValidException(Method method, Throwable throwable) { Class[] exceptions = method.getExceptionTypes(); Class throwableClass = throwable.getClass(); for (Class exception : exceptions) { @@ -32,7 +53,6 @@ public boolean isValidException(Throwable throwable) { return true; } } - return false; } diff --git a/src/test/java/org/mockito/internal/invocation/InvocationBuilder.java b/src/test/java/org/mockito/internal/invocation/InvocationBuilder.java index 281d41b3c3..9900db9d98 100644 --- a/src/test/java/org/mockito/internal/invocation/InvocationBuilder.java +++ b/src/test/java/org/mockito/internal/invocation/InvocationBuilder.java @@ -30,6 +30,7 @@ public class InvocationBuilder { private int sequenceNumber = 0; private Object[] args = new Object[] {}; private Object mock = Mockito.mock(IMethods.class); + private Class mockClass = IMethods.class; private Method method; private boolean verified; private List> argTypes; @@ -57,7 +58,7 @@ public Invocation toInvocation() { try { method = - IMethods.class.getMethod( + mockClass.getMethod( methodName, argTypes.toArray(new Class[argTypes.size()])); } catch (Exception e) { throw new RuntimeException( @@ -115,6 +116,12 @@ public InvocationBuilder mock(Object mock) { return this; } + public InvocationBuilder mockClass(Class mockClass) { + this.mockClass = mockClass; + this.mock = mock(mockClass); + return this; + } + public InvocationBuilder method(Method method) { this.method = method; return this; diff --git a/src/test/java/org/mockito/internal/stubbing/answers/InvocationInfoTest.java b/src/test/java/org/mockito/internal/stubbing/answers/InvocationInfoTest.java index 3c9d96983d..c365710cdd 100644 --- a/src/test/java/org/mockito/internal/stubbing/answers/InvocationInfoTest.java +++ b/src/test/java/org/mockito/internal/stubbing/answers/InvocationInfoTest.java @@ -15,6 +15,7 @@ import org.mockito.internal.invocation.InvocationBuilder; import org.mockito.invocation.Invocation; import org.mockitousage.IMethods; +import org.mockitousage.MethodsImpl; public class InvocationInfoTest { @@ -29,6 +30,20 @@ public void should_know_valid_throwables() throws Exception { assertThat(info.isValidException(new CharacterCodingException())).isTrue(); } + @Test + public void should_mark_interface_overridden_exceptions_as_valid() { + // when + Invocation invocation = + new InvocationBuilder() + .method("throwsRemovedInSubclass") + .mockClass(MethodsImpl.class) + .toInvocation(); + InvocationInfo info = new InvocationInfo(invocation); + + // then + assertThat(info.isValidException(new CharacterCodingException())).isTrue(); + } + @Test public void should_know_valid_return_types() throws Exception { assertThat( diff --git a/src/test/java/org/mockitousage/IMethods.java b/src/test/java/org/mockitousage/IMethods.java index 12897422da..5d92dc1b13 100644 --- a/src/test/java/org/mockitousage/IMethods.java +++ b/src/test/java/org/mockitousage/IMethods.java @@ -159,6 +159,8 @@ String simpleMethod( String canThrowException() throws CharacterCodingException; + String throwsRemovedInSubclass() throws CharacterCodingException; + String oneArray(String[] array); void varargsString(int i, String... string); diff --git a/src/test/java/org/mockitousage/MethodsImpl.java b/src/test/java/org/mockitousage/MethodsImpl.java index bb69656712..a853e9f04c 100644 --- a/src/test/java/org/mockitousage/MethodsImpl.java +++ b/src/test/java/org/mockitousage/MethodsImpl.java @@ -303,6 +303,11 @@ public String canThrowException() throws CharacterCodingException { return null; } + @Override + public String throwsRemovedInSubclass() { + return null; + } + public String oneArray(String[] array) { return null; } From 792da83e123cee018a01bbf7b27322e83aa1ff63 Mon Sep 17 00:00:00 2001 From: Andrey Kozel Date: Wed, 26 Jan 2022 22:53:47 +0300 Subject: [PATCH 2/2] Fixes #2201 : Code review fixes --- .../stubbing/answers/InvocationInfo.java | 38 ++++++-- .../answers/InvocationInfoExceptionTest.java | 95 +++++++++++++++++++ .../stubbing/answers/InvocationInfoTest.java | 28 ------ src/test/java/org/mockitousage/IMethods.java | 2 - .../java/org/mockitousage/MethodsImpl.java | 5 - 5 files changed, 124 insertions(+), 44 deletions(-) create mode 100644 src/test/java/org/mockito/internal/stubbing/answers/InvocationInfoExceptionTest.java diff --git a/src/main/java/org/mockito/internal/stubbing/answers/InvocationInfo.java b/src/main/java/org/mockito/internal/stubbing/answers/InvocationInfo.java index 59a57e2be2..c159906af0 100644 --- a/src/main/java/org/mockito/internal/stubbing/answers/InvocationInfo.java +++ b/src/main/java/org/mockito/internal/stubbing/answers/InvocationInfo.java @@ -6,7 +6,9 @@ import java.lang.reflect.Method; import java.lang.reflect.Modifier; +import java.util.ArrayList; import java.util.Arrays; +import java.util.List; import org.mockito.internal.invocation.AbstractAwareMethod; import org.mockito.internal.util.MockUtil; @@ -25,18 +27,36 @@ public InvocationInfo(InvocationOnMock theInvocation) { this.invocation = theInvocation; } - public boolean isValidException(Throwable throwable) { + public boolean isValidException(final Throwable throwable) { if (isValidException(method, throwable)) { return true; } - return Arrays.stream(method.getDeclaringClass().getInterfaces()) - .anyMatch(parent -> isValidExceptionForParent(parent, throwable)); + return isValidExceptionForParents(method.getDeclaringClass(), throwable); } - private boolean isValidExceptionForParent(Class parent, Throwable throwable) { + private boolean isValidExceptionForParents(final Class parent, final Throwable throwable) { + final List> ancestors = new ArrayList<>(Arrays.asList(parent.getInterfaces())); + + if (parent.getSuperclass() != null) { + ancestors.add(parent.getSuperclass()); + } + + final boolean validException = + ancestors.stream() + .anyMatch(ancestor -> isValidExceptionForClass(ancestor, throwable)); + + if (validException) { + return true; + } + + return ancestors.stream() + .anyMatch(ancestor -> isValidExceptionForParents(ancestor, throwable)); + } + + private boolean isValidExceptionForClass(final Class parent, final Throwable throwable) { try { - Method parentMethod = + final Method parentMethod = parent.getMethod(this.method.getName(), this.method.getParameterTypes()); return isValidException(parentMethod, throwable); } catch (NoSuchMethodException e) { @@ -45,10 +65,10 @@ private boolean isValidExceptionForParent(Class parent, Throwable throwable) } } - private boolean isValidException(Method method, Throwable throwable) { - Class[] exceptions = method.getExceptionTypes(); - Class throwableClass = throwable.getClass(); - for (Class exception : exceptions) { + private boolean isValidException(final Method method, final Throwable throwable) { + final Class[] exceptions = method.getExceptionTypes(); + final Class throwableClass = throwable.getClass(); + for (final Class exception : exceptions) { if (exception.isAssignableFrom(throwableClass)) { return true; } diff --git a/src/test/java/org/mockito/internal/stubbing/answers/InvocationInfoExceptionTest.java b/src/test/java/org/mockito/internal/stubbing/answers/InvocationInfoExceptionTest.java new file mode 100644 index 0000000000..21802c1f54 --- /dev/null +++ b/src/test/java/org/mockito/internal/stubbing/answers/InvocationInfoExceptionTest.java @@ -0,0 +1,95 @@ +/* + * Copyright (c) 2022 Mockito contributors + * This program is made available under the terms of the MIT License. + */ +package org.mockito.internal.stubbing.answers; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.mockito.internal.invocation.InvocationBuilder; +import org.mockito.invocation.Invocation; + +import java.nio.charset.CharacterCodingException; +import java.util.Arrays; +import java.util.Collection; + +import static org.assertj.core.api.Assertions.assertThat; + +@RunWith(Parameterized.class) +public class InvocationInfoExceptionTest { + + private final String methodName; + + public InvocationInfoExceptionTest(final String methodName) { + this.methodName = methodName; + } + + @Parameterized.Parameters + public static Collection data() { + return Arrays.asList( + new Object[][] { + {"throwException"}, + {"parentThrowsException"}, + {"grandParentThrowsException"}, + {"interfaceThrowsException"}, + {"grandInterfaceThrowsException"}, + {"interfaceOfParentThrowsException"} + }); + } + + @Test + public void should_know_valid_throwables() throws Exception { + // when + final Invocation invocation = + new InvocationBuilder() + .method(methodName) + .mockClass(CurrentClass.class) + .toInvocation(); + final InvocationInfo info = new InvocationInfo(invocation); + + // then + assertThat(info.isValidException(new Exception())).isFalse(); + assertThat(info.isValidException(new CharacterCodingException())).isTrue(); + } + + private abstract static class GrandParent { + public abstract void grandParentThrowsException() throws CharacterCodingException; + } + + private interface InterfaceOfParent { + abstract void interfaceOfParentThrowsException() throws CharacterCodingException; + } + + private abstract static class Parent extends GrandParent implements InterfaceOfParent { + public abstract void parentThrowsException() throws CharacterCodingException; + } + + private interface GrandInterface { + void grandInterfaceThrowsException() throws CharacterCodingException; + } + + private interface Interface extends GrandInterface { + void interfaceThrowsException() throws CharacterCodingException; + } + + private static class CurrentClass extends Parent implements Interface { + + public void throwException() throws CharacterCodingException {} + + @Override + public void grandParentThrowsException() {} + + @Override + public void parentThrowsException() {} + + @Override + public void grandInterfaceThrowsException() {} + + @Override + public void interfaceThrowsException() {} + + @Override + public void interfaceOfParentThrowsException() {} + } +} diff --git a/src/test/java/org/mockito/internal/stubbing/answers/InvocationInfoTest.java b/src/test/java/org/mockito/internal/stubbing/answers/InvocationInfoTest.java index c365710cdd..fea0ec36eb 100644 --- a/src/test/java/org/mockito/internal/stubbing/answers/InvocationInfoTest.java +++ b/src/test/java/org/mockito/internal/stubbing/answers/InvocationInfoTest.java @@ -9,41 +9,13 @@ import static org.mockitoutil.TestBase.getLastInvocation; import java.lang.reflect.Method; -import java.nio.charset.CharacterCodingException; import org.junit.Test; import org.mockito.internal.invocation.InvocationBuilder; -import org.mockito.invocation.Invocation; import org.mockitousage.IMethods; -import org.mockitousage.MethodsImpl; public class InvocationInfoTest { - @Test - public void should_know_valid_throwables() throws Exception { - // when - Invocation invocation = new InvocationBuilder().method("canThrowException").toInvocation(); - InvocationInfo info = new InvocationInfo(invocation); - - // then - assertThat(info.isValidException(new Exception())).isFalse(); - assertThat(info.isValidException(new CharacterCodingException())).isTrue(); - } - - @Test - public void should_mark_interface_overridden_exceptions_as_valid() { - // when - Invocation invocation = - new InvocationBuilder() - .method("throwsRemovedInSubclass") - .mockClass(MethodsImpl.class) - .toInvocation(); - InvocationInfo info = new InvocationInfo(invocation); - - // then - assertThat(info.isValidException(new CharacterCodingException())).isTrue(); - } - @Test public void should_know_valid_return_types() throws Exception { assertThat( diff --git a/src/test/java/org/mockitousage/IMethods.java b/src/test/java/org/mockitousage/IMethods.java index 5d92dc1b13..12897422da 100644 --- a/src/test/java/org/mockitousage/IMethods.java +++ b/src/test/java/org/mockitousage/IMethods.java @@ -159,8 +159,6 @@ String simpleMethod( String canThrowException() throws CharacterCodingException; - String throwsRemovedInSubclass() throws CharacterCodingException; - String oneArray(String[] array); void varargsString(int i, String... string); diff --git a/src/test/java/org/mockitousage/MethodsImpl.java b/src/test/java/org/mockitousage/MethodsImpl.java index a853e9f04c..bb69656712 100644 --- a/src/test/java/org/mockitousage/MethodsImpl.java +++ b/src/test/java/org/mockitousage/MethodsImpl.java @@ -303,11 +303,6 @@ public String canThrowException() throws CharacterCodingException { return null; } - @Override - public String throwsRemovedInSubclass() { - return null; - } - public String oneArray(String[] array) { return null; }