Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fixes #2489 : Fixed issue related to exceptions thrown from the nested spies #2546

Merged
merged 2 commits into from Jan 22, 2022
Merged

Fixes #2489 : Fixed issue related to exceptions thrown from the nested spies #2546

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
728c2e6
Fixes #2489 : Fixed issue related to exceptions thrown from the neste…
Jan 21, 2022
7ec800d
Fixes #2489 : Fixed codestyle
Jan 22, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
61 changes: 26 additions & 35 deletions src/main/java/org/mockito/internal/creation/bytebuddy/MockMethodAdvice.java
View file
Open in desktop
Expand Up @@ -19,6 +19,9 @@
import java.lang.reflect.Method;
import java.util.List;
import java.util.Map;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Comparator;
import java.util.concurrent.Callable;
import java.util.function.Predicate;

Expand Down Expand Up @@ -114,27 +117,6 @@ private static void exit(
}
}

static Throwable hideRecursiveCall(Throwable throwable, int current, Class<?> targetType) {
try {
StackTraceElement[] stack = throwable.getStackTrace();
int skip = 0;
StackTraceElement next;
do {
next = stack[stack.length - current - ++skip];
} while (!next.getClassName().equals(targetType.getName()));
int top = stack.length - current - skip;
StackTraceElement[] cleared = new StackTraceElement[stack.length - skip];
System.arraycopy(stack, 0, cleared, 0, top);
System.arraycopy(stack, top + skip, cleared, top, current);
throwable.setStackTrace(cleared);
return throwable;
} catch (RuntimeException ignored) {
// This should not happen unless someone instrumented or manipulated exception stack
// traces.
return throwable;
}
}

@Override
public Callable<?> handle(Object instance, Method origin, Object[] arguments) throws Throwable {
MockMethodInterceptor interceptor = interceptors.get(instance);
Expand Down Expand Up @@ -333,25 +315,34 @@ private static Object tryInvoke(Method origin, Object instance, Object[] argumen
return accessor.invoke(origin, instance, arguments);
} catch (InvocationTargetException exception) {
Throwable cause = exception.getCause();
StackTraceElement[] tmpStack = new Throwable().getStackTrace();

int skip = tmpStack.length;
// if there is a suitable instance, do not skip the root-cause for the exception
if (instance != null) {
skip = 0;
String causingClassName = instance.getClass().getName();
StackTraceElement stackFrame;
do {
stackFrame = tmpStack[skip++];
} while (stackFrame.getClassName().startsWith(causingClassName));
}

new ConditionalStackTraceFilter()
.filter(hideRecursiveCall(cause, skip, origin.getDeclaringClass()));
.filter(removeRecursiveCalls(cause, origin.getDeclaringClass()));
throw cause;
}
}

static Throwable removeRecursiveCalls(final Throwable cause, final Class<?> declaringClass) {
final List<String> uniqueStackTraceItems = new ArrayList<>();
final List<Integer> indexesToBeRemoved = new ArrayList<>();
for (StackTraceElement element : cause.getStackTrace()) {
final String key = element.getClassName() + element.getLineNumber();
final int elementIndex = uniqueStackTraceItems.lastIndexOf(key);
uniqueStackTraceItems.add(key);

if (elementIndex > -1 && declaringClass.getName().equals(element.getClassName())) {
indexesToBeRemoved.add(elementIndex);
}
}
final List<StackTraceElement> adjustedList =
new ArrayList<>(Arrays.asList(cause.getStackTrace()));
indexesToBeRemoved.stream()
.sorted(Comparator.reverseOrder())
.mapToInt(Integer::intValue)
.forEach(adjustedList::remove);
cause.setStackTrace(adjustedList.toArray(new StackTraceElement[] {}));
return cause;
}

private static class ReturnValueWrapper implements Callable<Object> {

private final Object returned;
Expand Down
83 changes: 63 additions & 20 deletions ...st/java/org/mockito/internal/creation/bytebuddy/InlineDelegateByteBuddyMockMakerTest.java
View file
Open in desktop
Expand Up @@ -9,19 +9,14 @@
import static net.bytebuddy.matcher.ElementMatchers.named;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.fail;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.*;
import static org.junit.Assume.assumeTrue;

import java.io.IOException;
import java.util.HashMap;
import java.util.List;
import java.util.Observable;
import java.util.Observer;
import java.util.Optional;
import java.util.Set;
import java.util.*;
import java.util.concurrent.Callable;
import java.util.regex.Pattern;
import java.util.stream.Collectors;

import net.bytebuddy.ByteBuddy;
import net.bytebuddy.ClassFileVersion;
Expand Down Expand Up @@ -253,48 +248,90 @@ public void should_leave_causing_stack() throws Exception {
mockMaker.createSpy(
settings, new MockHandlerImpl<>(settings), new ExceptionThrowingClass());

StackTraceElement[] returnedStack = null;
try {
proxy.get().throwException();
} catch (IOException ex) {
returnedStack = ex.getStackTrace();
}
StackTraceElement[] returnedStack =
assertThrows(IOException.class, () -> proxy.get().throwException()).getStackTrace();

assertNotNull("Stack trace from mockito expected", returnedStack);

assertEquals(ExceptionThrowingClass.class.getName(), returnedStack[0].getClassName());
assertEquals("internalThrowException", returnedStack[0].getMethodName());
List<StackTraceElement> exceptionClassElements =
Arrays.stream(returnedStack)
.filter(
element ->
element.getClassName()
.equals(ExceptionThrowingClass.class.getName()))
.collect(Collectors.toList());
assertEquals(3, exceptionClassElements.size());
assertEquals("internalThrowException", exceptionClassElements.get(0).getMethodName());
assertEquals("internalThrowException", exceptionClassElements.get(1).getMethodName());
assertEquals("throwException", exceptionClassElements.get(2).getMethodName());
}

@Test
public void should_leave_causing_stack_with_two_spies() throws Exception {
// given
MockSettingsImpl<ExceptionThrowingClass> settingsEx = new MockSettingsImpl<>();
settingsEx.setTypeToMock(ExceptionThrowingClass.class);
settingsEx.defaultAnswer(Answers.CALLS_REAL_METHODS);
Optional<ExceptionThrowingClass> proxyEx =
mockMaker.createSpy(
settingsEx,
new MockHandlerImpl<>(settingsEx),
new ExceptionThrowingClass());

MockSettingsImpl<WrapperClass> settingsWr = new MockSettingsImpl<>();
settingsWr.setTypeToMock(WrapperClass.class);
settingsWr.defaultAnswer(Answers.CALLS_REAL_METHODS);
Optional<WrapperClass> proxyWr =
mockMaker.createSpy(
settingsWr, new MockHandlerImpl<>(settingsWr), new WrapperClass());

// when
IOException ex =
assertThrows(IOException.class, () -> proxyWr.get().callWrapped(proxyEx.get()));
List<StackTraceElement> wrapperClassElements =
Arrays.stream(ex.getStackTrace())
.filter(
element ->
element.getClassName().equals(WrapperClass.class.getName()))
.collect(Collectors.toList());

// then
assertEquals(1, wrapperClassElements.size());
assertEquals("callWrapped", wrapperClassElements.get(0).getMethodName());
}

@Test
public void should_remove_recursive_self_call_from_stack_trace() throws Exception {
StackTraceElement[] stack =
new StackTraceElement[] {
new StackTraceElement("foo", "", "", -1),
new StackTraceElement(SampleInterface.class.getName(), "", "", -1),
new StackTraceElement(SampleInterface.class.getName(), "", "", 15),
new StackTraceElement("qux", "", "", -1),
new StackTraceElement("bar", "", "", -1),
new StackTraceElement(SampleInterface.class.getName(), "", "", 15),
new StackTraceElement("baz", "", "", -1)
};

Throwable throwable = new Throwable();
throwable.setStackTrace(stack);
throwable = MockMethodAdvice.hideRecursiveCall(throwable, 2, SampleInterface.class);
throwable = MockMethodAdvice.removeRecursiveCalls(throwable, SampleInterface.class);

assertThat(throwable.getStackTrace())
.isEqualTo(
new StackTraceElement[] {
new StackTraceElement("foo", "", "", -1),
new StackTraceElement("qux", "", "", -1),
new StackTraceElement("bar", "", "", -1),
new StackTraceElement(SampleInterface.class.getName(), "", "", 15),
new StackTraceElement("baz", "", "", -1)
});
}

@Test
public void should_handle_missing_or_inconsistent_stack_trace() throws Exception {
public void should_handle_missing_or_inconsistent_stack_trace() {
Throwable throwable = new Throwable();
throwable.setStackTrace(new StackTraceElement[0]);
assertThat(MockMethodAdvice.hideRecursiveCall(throwable, 0, SampleInterface.class))
assertThat(MockMethodAdvice.removeRecursiveCalls(throwable, SampleInterface.class))
.isSameAs(throwable);
}

Expand Down Expand Up @@ -579,6 +616,12 @@ public T value() {
}
}

public static class WrapperClass {
public void callWrapped(ExceptionThrowingClass exceptionThrowingClass) throws IOException {
exceptionThrowingClass.throwException();
}
}

public static class GenericSubClass extends GenericClass<String> {}

public static class ExceptionThrowingClass {
Expand Down