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.

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

Specify event that triggers exception #486

Merged
merged 2 commits into from
Oct 11, 2023

Conversation

jbescos
Copy link
Member

@jbescos jbescos commented Oct 10, 2023

ObserverExceptionRethrownTest#testNonTransactionalObserverThrowsNonCheckedExceptionIsRethrown verifies that one exception happens when firing one event.

In the case of Helidon, this exception is thrown on deployment ObserverExceptionRethrownTest#createTestArchive because Helidon container fires 2 events on deployment. One to notify when beans are going to be deployed, and other when beans are deployed.

Proposed solution: TeaCupPomeranian will evaluate the event content, to throw the exception only when we want to throw it.

Signed-off-by: Jorge Bescos Gascon <jorge.bescos.gascon@oracle.com>
@manovotn
Copy link
Contributor

Proposed solution: TeaCupPomeranian will evaluate the event content, to throw the exception only when we want to throw it.

I'd rather use a custom event payload (some Foobar instead of String) which I suppose will solve your issue as well?

Also, do you need this fixed in any existing TCKs? Such as TCKs for CDI 4?
If so, we would need to have a challenge for that and another PR for that branch (master is for CDI 4.1., the upcoming version).

Signed-off-by: Jorge Bescos Gascon <jorge.bescos.gascon@oracle.com>
@jbescos
Copy link
Member Author

jbescos commented Oct 10, 2023

I have modified the PR to a custom event payload. That solves it too, thanks for the point.

I need it for CDI 4.0. I am going to create a challenge for that.

Thank you.

@Ladicek Ladicek merged commit 069e33a into jakartaee:master Oct 11, 2023
4 checks passed
@Ladicek Ladicek added this to the CDI 4.1 milestone Oct 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants