Skip to content

Commit 8cdf0cc

Browse files
andrey-kozelandrey-kozel
authored
Fixes #2489 : Fixed issue related to exceptions thrown from the nested spies (#2546)
Behaviour of the filtering redundant method calls from stacktrace was changed. Now it removes only lines that have the same method name and line number Co-authored-by: Andrey Kozel <[email protected]>
1 parent faa6e92 commit 8cdf0cc

File tree

2 files changed

+89
-55
lines changed

2 files changed

+89
-55
lines changed

‎src/main/java/org/mockito/internal/creation/bytebuddy/MockMethodAdvice.java

+26-35
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,9 @@
1919
import java.lang.reflect.Method;
2020
import java.util.List;
2121
import java.util.Map;
22+
import java.util.ArrayList;
23+
import java.util.Arrays;
24+
import java.util.Comparator;
2225
import java.util.concurrent.Callable;
2326
import java.util.function.Predicate;
2427

@@ -114,27 +117,6 @@ private static void exit(
114117
}
115118
}
116119

117-
static Throwable hideRecursiveCall(Throwable throwable, int current, Class<?> targetType) {
118-
try {
119-
StackTraceElement[] stack = throwable.getStackTrace();
120-
int skip = 0;
121-
StackTraceElement next;
122-
do {
123-
next = stack[stack.length - current - ++skip];
124-
} while (!next.getClassName().equals(targetType.getName()));
125-
int top = stack.length - current - skip;
126-
StackTraceElement[] cleared = new StackTraceElement[stack.length - skip];
127-
System.arraycopy(stack, 0, cleared, 0, top);
128-
System.arraycopy(stack, top + skip, cleared, top, current);
129-
throwable.setStackTrace(cleared);
130-
return throwable;
131-
} catch (RuntimeException ignored) {
132-
// This should not happen unless someone instrumented or manipulated exception stack
133-
// traces.
134-
return throwable;
135-
}
136-
}
137-
138120
@Override
139121
public Callable<?> handle(Object instance, Method origin, Object[] arguments) throws Throwable {
140122
MockMethodInterceptor interceptor = interceptors.get(instance);
@@ -333,25 +315,34 @@ private static Object tryInvoke(Method origin, Object instance, Object[] argumen
333315
return accessor.invoke(origin, instance, arguments);
334316
} catch (InvocationTargetException exception) {
335317
Throwable cause = exception.getCause();
336-
StackTraceElement[] tmpStack = new Throwable().getStackTrace();
337-
338-
int skip = tmpStack.length;
339-
// if there is a suitable instance, do not skip the root-cause for the exception
340-
if (instance != null) {
341-
skip = 0;
342-
String causingClassName = instance.getClass().getName();
343-
StackTraceElement stackFrame;
344-
do {
345-
stackFrame = tmpStack[skip++];
346-
} while (stackFrame.getClassName().startsWith(causingClassName));
347-
}
348-
349318
new ConditionalStackTraceFilter()
350-
.filter(hideRecursiveCall(cause, skip, origin.getDeclaringClass()));
319+
.filter(removeRecursiveCalls(cause, origin.getDeclaringClass()));
351320
throw cause;
352321
}
353322
}
354323

324+
static Throwable removeRecursiveCalls(final Throwable cause, final Class<?> declaringClass) {
325+
final List<String> uniqueStackTraceItems = new ArrayList<>();
326+
final List<Integer> indexesToBeRemoved = new ArrayList<>();
327+
for (StackTraceElement element : cause.getStackTrace()) {
328+
final String key = element.getClassName() + element.getLineNumber();
329+
final int elementIndex = uniqueStackTraceItems.lastIndexOf(key);
330+
uniqueStackTraceItems.add(key);
331+
332+
if (elementIndex > -1 && declaringClass.getName().equals(element.getClassName())) {
333+
indexesToBeRemoved.add(elementIndex);
334+
}
335+
}
336+
final List<StackTraceElement> adjustedList =
337+
new ArrayList<>(Arrays.asList(cause.getStackTrace()));
338+
indexesToBeRemoved.stream()
339+
.sorted(Comparator.reverseOrder())
340+
.mapToInt(Integer::intValue)
341+
.forEach(adjustedList::remove);
342+
cause.setStackTrace(adjustedList.toArray(new StackTraceElement[] {}));
343+
return cause;
344+
}
345+
355346
private static class ReturnValueWrapper implements Callable<Object> {
356347

357348
private final Object returned;

‎src/test/java/org/mockito/internal/creation/bytebuddy/InlineDelegateByteBuddyMockMakerTest.java

+63-20
Original file line numberDiff line numberDiff line change
@@ -9,19 +9,14 @@
99
import static net.bytebuddy.matcher.ElementMatchers.named;
1010
import static org.assertj.core.api.Assertions.assertThat;
1111
import static org.assertj.core.api.Assertions.fail;
12-
import static org.junit.Assert.assertEquals;
13-
import static org.junit.Assert.assertNotNull;
12+
import static org.junit.Assert.*;
1413
import static org.junit.Assume.assumeTrue;
1514

1615
import java.io.IOException;
17-
import java.util.HashMap;
18-
import java.util.List;
19-
import java.util.Observable;
20-
import java.util.Observer;
21-
import java.util.Optional;
22-
import java.util.Set;
16+
import java.util.*;
2317
import java.util.concurrent.Callable;
2418
import java.util.regex.Pattern;
19+
import java.util.stream.Collectors;
2520

2621
import net.bytebuddy.ByteBuddy;
2722
import net.bytebuddy.ClassFileVersion;
@@ -253,48 +248,90 @@ public void should_leave_causing_stack() throws Exception {
253248
mockMaker.createSpy(
254249
settings, new MockHandlerImpl<>(settings), new ExceptionThrowingClass());
255250

256-
StackTraceElement[] returnedStack = null;
257-
try {
258-
proxy.get().throwException();
259-
} catch (IOException ex) {
260-
returnedStack = ex.getStackTrace();
261-
}
251+
StackTraceElement[] returnedStack =
252+
assertThrows(IOException.class, () -> proxy.get().throwException()).getStackTrace();
262253

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

265-
assertEquals(ExceptionThrowingClass.class.getName(), returnedStack[0].getClassName());
266-
assertEquals("internalThrowException", returnedStack[0].getMethodName());
256+
List<StackTraceElement> exceptionClassElements =
257+
Arrays.stream(returnedStack)
258+
.filter(
259+
element ->
260+
element.getClassName()
261+
.equals(ExceptionThrowingClass.class.getName()))
262+
.collect(Collectors.toList());
263+
assertEquals(3, exceptionClassElements.size());
264+
assertEquals("internalThrowException", exceptionClassElements.get(0).getMethodName());
265+
assertEquals("internalThrowException", exceptionClassElements.get(1).getMethodName());
266+
assertEquals("throwException", exceptionClassElements.get(2).getMethodName());
267+
}
268+
269+
@Test
270+
public void should_leave_causing_stack_with_two_spies() throws Exception {
271+
// given
272+
MockSettingsImpl<ExceptionThrowingClass> settingsEx = new MockSettingsImpl<>();
273+
settingsEx.setTypeToMock(ExceptionThrowingClass.class);
274+
settingsEx.defaultAnswer(Answers.CALLS_REAL_METHODS);
275+
Optional<ExceptionThrowingClass> proxyEx =
276+
mockMaker.createSpy(
277+
settingsEx,
278+
new MockHandlerImpl<>(settingsEx),
279+
new ExceptionThrowingClass());
280+
281+
MockSettingsImpl<WrapperClass> settingsWr = new MockSettingsImpl<>();
282+
settingsWr.setTypeToMock(WrapperClass.class);
283+
settingsWr.defaultAnswer(Answers.CALLS_REAL_METHODS);
284+
Optional<WrapperClass> proxyWr =
285+
mockMaker.createSpy(
286+
settingsWr, new MockHandlerImpl<>(settingsWr), new WrapperClass());
287+
288+
// when
289+
IOException ex =
290+
assertThrows(IOException.class, () -> proxyWr.get().callWrapped(proxyEx.get()));
291+
List<StackTraceElement> wrapperClassElements =
292+
Arrays.stream(ex.getStackTrace())
293+
.filter(
294+
element ->
295+
element.getClassName().equals(WrapperClass.class.getName()))
296+
.collect(Collectors.toList());
297+
298+
// then
299+
assertEquals(1, wrapperClassElements.size());
300+
assertEquals("callWrapped", wrapperClassElements.get(0).getMethodName());
267301
}
268302

269303
@Test
270304
public void should_remove_recursive_self_call_from_stack_trace() throws Exception {
271305
StackTraceElement[] stack =
272306
new StackTraceElement[] {
273307
new StackTraceElement("foo", "", "", -1),
274-
new StackTraceElement(SampleInterface.class.getName(), "", "", -1),
308+
new StackTraceElement(SampleInterface.class.getName(), "", "", 15),
275309
new StackTraceElement("qux", "", "", -1),
276310
new StackTraceElement("bar", "", "", -1),
311+
new StackTraceElement(SampleInterface.class.getName(), "", "", 15),
277312
new StackTraceElement("baz", "", "", -1)
278313
};
279314

280315
Throwable throwable = new Throwable();
281316
throwable.setStackTrace(stack);
282-
throwable = MockMethodAdvice.hideRecursiveCall(throwable, 2, SampleInterface.class);
317+
throwable = MockMethodAdvice.removeRecursiveCalls(throwable, SampleInterface.class);
283318

284319
assertThat(throwable.getStackTrace())
285320
.isEqualTo(
286321
new StackTraceElement[] {
287322
new StackTraceElement("foo", "", "", -1),
323+
new StackTraceElement("qux", "", "", -1),
288324
new StackTraceElement("bar", "", "", -1),
325+
new StackTraceElement(SampleInterface.class.getName(), "", "", 15),
289326
new StackTraceElement("baz", "", "", -1)
290327
});
291328
}
292329

293330
@Test
294-
public void should_handle_missing_or_inconsistent_stack_trace() throws Exception {
331+
public void should_handle_missing_or_inconsistent_stack_trace() {
295332
Throwable throwable = new Throwable();
296333
throwable.setStackTrace(new StackTraceElement[0]);
297-
assertThat(MockMethodAdvice.hideRecursiveCall(throwable, 0, SampleInterface.class))
334+
assertThat(MockMethodAdvice.removeRecursiveCalls(throwable, SampleInterface.class))
298335
.isSameAs(throwable);
299336
}
300337

@@ -579,6 +616,12 @@ public T value() {
579616
}
580617
}
581618

619+
public static class WrapperClass {
620+
public void callWrapped(ExceptionThrowingClass exceptionThrowingClass) throws IOException {
621+
exceptionThrowingClass.throwException();
622+
}
623+
}
624+
582625
public static class GenericSubClass extends GenericClass<String> {}
583626

584627
public static class ExceptionThrowingClass {

0 commit comments

Comments
 (0)