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

TCK Challenge: ObserverExceptionRethrownTest modify event type #487

Closed
jbescos opened this issue Oct 10, 2023 · 3 comments · Fixed by #488
Closed

TCK Challenge: ObserverExceptionRethrownTest modify event type #487

jbescos opened this issue Oct 10, 2023 · 3 comments · Fixed by #488
Labels
accepted Issue/challenge is accepted challenge TCK test challenge

Comments

@jbescos
Copy link
Member

jbescos commented Oct 10, 2023

Describe the bug
ObserverExceptionRethrownTest#testNonTransactionalObserverThrowsNonCheckedExceptionIsRethrown verifies that one exception happens when firing one String event.

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

To Reproduce
N/A

Expected behavior
TeaCupPomeranian will handle a different event type, to throw the exception only when we want to throw it.

Additional information
TCK challenge for EE 10 (CDI 4.0) PR: #488
PR for master: #486

@Ladicek
Copy link
Contributor

Ladicek commented Oct 11, 2023

It's your call, obviously, but I'd suggest Helidon maybe also use a dedicated event type, or use a qualifier. An event type of String, unqualified, is overly broad and there's a big risk of overlap (as this challenge shows).

@Ladicek
Copy link
Contributor

Ladicek commented Oct 11, 2023

PRs merged, closing.

@Ladicek Ladicek closed this as completed Oct 11, 2023
@manovotn
Copy link
Contributor

It's your call, obviously, but I'd suggest Helidon maybe also use a dedicated event type, or use a qualifier. An event type of String, unqualified, is overly broad and there's a big risk of overlap (as this challenge shows).

Fully agree but I didn't even suggest that as I assume this will be hard to remove anyway due to backward compatility.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted Issue/challenge is accepted challenge TCK test challenge
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants