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

TOMEE-4342 - ApplicationComposer in PER_JVM should be able to inject @Resource into test #1167

Merged
merged 3 commits into from
May 26, 2024

Conversation

rzo1
Copy link
Contributor

@rzo1 rzo1 commented May 25, 2024

No description provided.

Copy link
Contributor

@mawiesne mawiesne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thx @rzo1!

@mawiesne
Copy link
Contributor

Once merged, could this be 🍒 picked to 9.1.x maintenance branch for a potential 9.1.4 release? @rzo1

@rmannibucau
Copy link
Contributor

Shouldnt it use injector if cdi is disabled and nothing else (owbinjector)?
Can mean scanning is wrongly configured for tests no?

@rzo1
Copy link
Contributor Author

rzo1 commented May 26, 2024

Shouldnt it use injector if cdi is disabled and nothing else (owbinjector)? Can mean scanning is wrongly configured for tests no?

We don't have a lot of tests with @Applicationand docs are sparse in that area, so might not get it correctly.

Adding the actual test class to @Classes of the @Application will inject the @Resource into the test otherwise it seems, that @Resource is not scanned anywhere else (and the fallbacks only inject @Inject fields for a class, which is not part of the actual deployment).

Would that be the actual way to solve it with application composer? (If so, we just need to update the docs).

Or do you mean, that we are missing a fallback via Injector in composerInject(...) as the OWBInjector won't inject @Resource ?

@rmannibucau
Copy link
Contributor

Test classes are injected and resources already handled.
Limit for single instance for suites is that classes must be scanned for injections to be prepared as any bean so test classes must be scanned.
Can be adding it in classes or marking test classes to be a cdi bean (@Dependent)

@rzo1
Copy link
Contributor Author

rzo1 commented May 26, 2024

Thx @rmannibucau for clarification - I will update the test, verify it works and than remove my changes and update the docs, so it is clear how to use from user perspective.

@mawiesne
Copy link
Contributor

mawiesne commented May 26, 2024

I can confirm that adding the test class in @Classes helps, for example:

@Module
@Classes(cdi = true, value = {MyTest.class, App.class})
  public EjbJar modules() {
    return new EjbJar();
  }

Thx @rmannibucau for clarification. Thx @rzo1 for updating the docs - so community can better understand what is required in such a scenario.

@rzo1 rzo1 merged commit 6e21a76 into main May 26, 2024
1 check passed
@rzo1 rzo1 deleted the TOMEE-4342 branch May 26, 2024 18:27
@rzo1
Copy link
Contributor Author

rzo1 commented May 26, 2024

Thx for checking it, @mawiesne and @rmannibucau for pointing out the limitations of the single instance in suites.

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