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

Add basic injection capabilities to QuarkusTestResourceLifecycleManager #18698

Closed
famod opened this issue Jul 14, 2021 · 12 comments · Fixed by #18714
Closed

Add basic injection capabilities to QuarkusTestResourceLifecycleManager #18698

famod opened this issue Jul 14, 2021 · 12 comments · Fixed by #18714
Assignees
Labels
area/testing kind/enhancement New feature or request
Milestone

Comments

@famod
Copy link
Member

famod commented Jul 14, 2021

Description

As discussed on Zulip, it would come in handy if there was some convenient injection support in QuarkusTestResourceLifecycleManager.
Today you are on own since you only get the testInstance in inject() and you have to do all the hard reflection work yourself.

Implementation ideas

In a backward-compatible way, a new inject(BasicInjector injector) method could be introduced.
That BasicInbjector (or whatever its name is) wraps the test instance and could provide some basic convenience methods like:

  • injectIntoFields(Class<?> type, Object value) (or with <T>)
  • injectIntoFields(Class<? extends Annotation> type, Object value)
  • etc.
@famod famod added the kind/enhancement New feature or request label Jul 14, 2021
@quarkus-bot
Copy link

quarkus-bot bot commented Jul 14, 2021

/cc @manovotn, @mkouba

@quarkus-bot quarkus-bot bot added the area/arc Issue related to ARC (dependency injection) label Jul 14, 2021
@famod famod added area/testing and removed area/arc Issue related to ARC (dependency injection) labels Jul 14, 2021
@famod
Copy link
Member Author

famod commented Jul 14, 2021

/cc @geoand

@geoand
Copy link
Contributor

geoand commented Jul 15, 2021

Thanks for reporting!

This seems pretty useful to me. Hopefully I can pick it up soon

@geoand geoand self-assigned this Jul 15, 2021
@geoand
Copy link
Contributor

geoand commented Jul 15, 2021

@FroMage always has exquisite API improvement ideas, so let's tap into his knowledge to get another take on this one :)

@geoand
Copy link
Contributor

geoand commented Jul 15, 2021

My idea building on what @famod posted here is something like:

    default void inject(TestInjector testInjector) { // this is the new method

    }

    interface TestInjector { // the implementation will be supplied by the test extension and one new instance will be created per test - the instance will also hold the test object
        
        void injectIntoFields(Object fieldValue, Class<?> fieldType);

        void injectIntoFields(Object fieldValue, Class<?> fieldType, Class<? extends Annotation> annotation); // perhaps we don't need this method at all, and just make the previous one use varargs
    }

Then a supplied user implementations of QuarkusTestResourceLifecycleManager could do something like:

    @Override
    public void inject(TestInjector testInjector) {
        testInjector.injectIntoFields(this.someValue, SomeClass.class);
    }

WDYT?

@famod
Copy link
Member Author

famod commented Jul 15, 2021

I like the varargs idea. CDI BeanManager and Instance use similar approaches.

PS: Another strategy, which can be used today, is to define an interface with one or more setters etc. and then use the existing inject method that can then simply do an instanceof + cast + call the setter. The rest is up to the implementing test class.
But Quarkus is about developer joy and the injector would make things easier regardless.

@geoand
Copy link
Contributor

geoand commented Jul 15, 2021

#18714 is what I have in mind

@mkouba
Copy link
Contributor

mkouba commented Jul 15, 2021

I like the idea but should be really clear that it has nothing to do with CDI injection ;-).

@geoand
Copy link
Contributor

geoand commented Jul 15, 2021

@mkouba definitely. How do you propose we make it clearer?

@mkouba
Copy link
Contributor

mkouba commented Jul 15, 2021

@mkouba definitely. How do you propose we make it clearer?

I'm not sure. Maybe just emphasize that the manager is fully in control of the lifecycle of the injected instance and that this "injection" happens after the CDI injection is perfomed??

geoand added a commit to geoand/quarkus that referenced this issue Jul 15, 2021
@geoand
Copy link
Contributor

geoand commented Jul 15, 2021

PR updated to include that note in the Javadoc and the documentation

geoand added a commit to geoand/quarkus that referenced this issue Jul 16, 2021
geoand added a commit to geoand/quarkus that referenced this issue Jul 16, 2021
geoand added a commit to geoand/quarkus that referenced this issue Jul 16, 2021
geoand added a commit to geoand/quarkus that referenced this issue Jul 16, 2021
geoand added a commit that referenced this issue Jul 16, 2021
Provide easy way for QuarkusTestResourceLifecycleManager implementations to inject into test
@quarkus-bot quarkus-bot bot added this to the 2.2 - main milestone Jul 16, 2021
@gsmet gsmet modified the milestones: 2.2 - main, 2.1.0.Final Jul 19, 2021
gsmet pushed a commit to gsmet/quarkus that referenced this issue Jul 19, 2021
@FroMage
Copy link
Member

FroMage commented Jul 27, 2021

@FroMage always has exquisite API improvement ideas, so let's tap into his knowledge to get another take on this one :)

This is perfect!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/testing kind/enhancement New feature or request
Projects
None yet
5 participants