Unwrap AssertionError from InvocationTargetException during Quarkus JUnit5 callbacks
#35646
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thanks for your pull request! The title of your pull request does not follow our editorial rules. Could you have a look?
This message is automatically generated by a bot. |
msavy
changed the title
AssertionError from InvocationTargetException during Quarkus JUnit5 callbacks.AssertionError from InvocationTargetException during Quarkus JUnit5 callbacks
msavy
force-pushed
the
unwrap-callback-assertion-error
branch
4 times, most recently
from
7166640 to
fb42cc1
Compare
…us JUnit5 callbacks. Previously, only `RuntimeException` derived throwables were unwrapped, but many test frameworks use throwables deriving from `AssertionError`, so this should be handled also.
msavy
force-pushed
the
unwrap-callback-assertion-error
branch
from
fb42cc1 to
7b9cb7d
Compare
geoand
added
the
triage/waiting-for-ci
|
@geoand CI doesn't seem to be triggering for some reason |
|
GH workflows says it is still waiting for approval from a maintainer, which you are, so not sure why it's not picking it up. |
|
Done! |
|
✔️ The latest workflow run for the pull request has completed successfully. It should be safe to merge provided you have a look at the other checks in the summary. |
quarkus-bot
bot
removed
the
triage/waiting-for-ci
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I have been writing some custom Quarkus JUnit 5 callbacks; these are specially provided by Quarkus to allow callbacks within the context of the Quarkus classloader to do things like CDI, etc.
I've generally managed to get most things working after experimentation, plus digging through code and documentation, but there was one nit:
Currently, only
RuntimeException-derived throwables are being unwrapped, but many test frameworks use throwables deriving fromAssertionError. Hence, if anAssertionErroris thrown in the callback, it currently ends up being wrapped by anInvocationTargetException, which means that tooling doesn't parse the stack trace properly for "expected" vs "actual" comparisons, etcetera.This small pull request ensures that
AssertionErroris unwrapped properly (but notErrormore generically, as this could mask categories of error that emanate from other sources such as OOM, where leaving it unwrapped may be helpful).I added a test that exercises the existing codepaths as well as the new one.
p.s: I initially tried to add the tests to existing
@QuarkusTests that cover callbacks, but despite sinking many hours into it, I couldn't find a sensible way to capture the exceptions thrown from the Quarkus callbacks inside of the JUnit test, so this semi-mocked approach seems the most practical way without significant code changes, which I doubt are worth it.