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

Change all beans.xml to make bean archives implicit. #236

Merged
merged 1 commit into from
Dec 4, 2020

Conversation

manovotn
Copy link
Contributor

@manovotn manovotn commented Dec 3, 2020

Fixes #235

@manovotn manovotn requested a review from FroMage December 3, 2020 15:12
FroMage
FroMage previously approved these changes Dec 3, 2020
@manovotn
Copy link
Contributor Author

manovotn commented Dec 3, 2020

Ah, tests seem to rely on that discovery mode, will fix that in a bit.

@manovotn
Copy link
Contributor Author

manovotn commented Dec 3, 2020

I have figured one of the test failures, but I can't understand why the FullStackTest crashes with changed discovery mode. It just gives me 404 but it has to have something to do with bean discovery.
ATM I have reverted the PR to just keep discovery mode all within tests (which is harmless anyway) and I will look into the above described problem tomorrow.

@manovotn manovotn requested a review from FroMage December 4, 2020 09:40
@manovotn
Copy link
Contributor Author

manovotn commented Dec 4, 2020

Ok, turns out this is the way it should be. Bean discovery mode all is required in tests because of ResteasyCdiExtension. Without it, Weld won't fire ProcessAnnotatedType for rest resources (because they don't have bean defining annotations) and resteasy won't know about them, hence 404. Subsequently, we won't fire ProcessInjectionTarget which they extension is trying to wrap in order to perform their injection, so even if you specify the resource manually, this part will fail because of no injection into the resource.
Long story short, they extension is somewhat weird and relies in CDI mode all to actually work its magic. Having this mode in just tests is OK, so we can just keep it that way.

@FroMage please re-stamp the PR whenever you have some spare minute.

@FroMage FroMage merged commit fc942a1 into smallrye:master Dec 4, 2020
@FroMage FroMage added this to the 1.0.21 milestone Dec 4, 2020
@FroMage FroMage modified the milestones: 1.0.21, 1.1.0 Jan 19, 2021
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.

SR CP artifacts are currently transferred into explicit bean archives while they could be implicit
2 participants