Don't assert exact exception type when StackOverflowError is thrown #2815
Conversation
| assertThrowsStackOverflow(() -> gson.toJson(obj)); | ||
| } | ||
|
|
||
| /** Asserts that a {@link StackOverflowError} is thrown. */ |
There was a problem hiding this comment.
But it doesn't assert that, does it? 🙂 I think we can probably assume that StackOverflowError is at least in the cause chain, and probably that it is the root cause. So then this?
Throwable t = assertThrows(Throwable.class, runnable);
assertThat(Throwables.getRootCause(t)).isInstanceOf(StackOverflowError.class);There was a problem hiding this comment.
But it doesn't assert that, does it?
Yes you are right. My intention was to indicate that this methods is intended for cases where a StackOverflowError is expected, but then the implementation admits that it cannot reliably detect that.
I think we can probably assume that
StackOverflowErroris at least in the cause chain
Yes that could be an option. Let's just hope that there is no JDK code which sets it only as 'suppressed exception', or which entirely discards the cause.
There was a problem hiding this comment.
Right. If that ever happens, we can adjust accordingly.
| assertThrowsStackOverflow(() -> gson.toJson(obj)); | ||
| } | ||
|
|
||
| /** Asserts that a {@link StackOverflowError} is thrown. */ |
There was a problem hiding this comment.
Right. If that ever happens, we can adjust accordingly.
Purpose
Make test more robust
Description
In https://github.com/google/gson/actions/runs/13467912934/job/37637273646?pr=2814 the JDK 21 build randomly failed:
The test
CircularReferenceTest#testSelfReferenceArrayFieldSerializationexpects aStackOverflowError, and indeed that error occurred. However, it happened inside the JDK code and that wrapped it in anInternalError.So to make the test more reliable it should not assume that the
StackOverflowErroractually reaches the assertion but just that any exception reaches it.Checklist
This is automatically checked by
mvn verify, but can also be checked on its own usingmvn spotless:check.Style violations can be fixed using
mvn spotless:apply; this can be done in a separate commit to verify that it did not cause undesired changes.null@since $next-version$(
$next-version$is a special placeholder which is automatically replaced during release)TestCase)mvn clean verify javadoc:jarpasses without errors