Beans injected in tests are removed as unused

[sarxos]

Hello @mkouba,

This is in regards to our conversation from Zulip.

Beans injected into a test are automatically removed.

I have a service:

@ApplicationScoped
public class TestService {
}

And a test:

@QuarkusTest
public class GreetingResourceTest {

    @Inject
    TestService ts;

    @Test
    void test_lookupService() {
        Assertions.assertThat(ts).isNotNull();
    }
}

Since TestService has bean defining annotation it is automatically discovered. And since it's @Inject-ed into test, it should not be autoremoved. But it's removed and test fails

The workarounds so far:

1. Configure quarkus.arc.remove-unused-beans=false in application.preferences
2. Add scope annotation on test, e.g. @Singleton:

@Singleton // this cause test pass
@QuarkusTest
public class GreetingResourceTest {

    @Inject
    TestService ts;

    @Test
    void test_lookupService() {
        Assertions.assertThat(ts).isNotNull();
    }
}

A failure log:

sarxos@sarxos-comp:~/workspace/getting-started$ mvn clean test
[INFO] Scanning for projects...
[INFO] 
[INFO] ----------------------< org.acme:getting-started >----------------------
[INFO] Building getting-started 1.0-SNAPSHOT
[INFO] --------------------------------[ jar ]---------------------------------
[INFO] 
[INFO] --- maven-clean-plugin:2.5:clean (default-clean) @ getting-started ---
[INFO] Deleting /home/sarxos/workspace-2019/getting-started/target
[INFO] 
[INFO] --- maven-resources-plugin:2.6:resources (default-resources) @ getting-started ---
[INFO] Using 'UTF-8' encoding to copy filtered resources.
[INFO] Copying 2 resources
[INFO] 
[INFO] --- maven-compiler-plugin:3.1:compile (default-compile) @ getting-started ---
[INFO] Changes detected - recompiling the module!
[INFO] Compiling 8 source files to /home/sarxos/workspace-2019/getting-started/target/classes
[INFO] 
[INFO] --- maven-resources-plugin:2.6:testResources (default-testResources) @ getting-started ---
[INFO] Using 'UTF-8' encoding to copy filtered resources.
[INFO] skip non existing resourceDirectory /home/sarxos/workspace-2019/getting-started/src/test/resources
[INFO] 
[INFO] --- maven-compiler-plugin:3.1:testCompile (default-testCompile) @ getting-started ---
[INFO] Changes detected - recompiling the module!
[INFO] Compiling 2 source files to /home/sarxos/workspace-2019/getting-started/target/test-classes
[INFO] 
[INFO] --- maven-surefire-plugin:2.22.0:test (default-test) @ getting-started ---
[INFO] 
[INFO] -------------------------------------------------------
[INFO]  T E S T S
[INFO] -------------------------------------------------------
[INFO] Running org.acme.quickstart.GreetingResourceTest
2019-03-28 20:37:07,486 INFO  [io.qua.dep.QuarkusAugmentor] (main) Beginning quarkus augmentation
2019-03-28 20:37:08,014 INFO  [io.qua.dep.QuarkusAugmentor] (main) Quarkus augmentation completed in 528ms
2019-03-28 20:37:08,331 INFO  [io.quarkus] (main) Quarkus 0.12.0 started in 0.300s. Listening on: http://[::]:8081
2019-03-28 20:37:08,331 INFO  [io.quarkus] (main) Installed features: [cdi, resteasy]
[ERROR] Tests run: 1, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 0.902 s <<< FAILURE! - in org.acme.quickstart.GreetingResourceTest
[ERROR] test_lookupService  Time elapsed: 0.01 s  <<< ERROR!
org.junit.jupiter.api.extension.TestInstantiationException: TestInstanceFactory [io.quarkus.test.junit.QuarkusTestExtension] failed to instantiate test class [org.acme.quickstart.GreetingResourceTest]: Failed to inject field com.github.sarxos.abberwoult.cdi.TestService org.acme.quickstart.GreetingResourceTest.ts
Caused by: java.lang.RuntimeException: Failed to inject field com.github.sarxos.abberwoult.cdi.TestService org.acme.quickstart.GreetingResourceTest.ts
Caused by: java.lang.NullPointerException: Managed Bean [class com.github.sarxos.abberwoult.cdi.TestService] is null

2019-03-28 20:37:08,359 INFO  [io.quarkus] (main) Quarkus stopped in 0.008s
[INFO] 
[INFO] Results:
[INFO] 
[ERROR] Errors: 
[ERROR]   GreetingResourceTest.test_lookupService » TestInstantiation TestInstanceFactor...
[INFO] 
[ERROR] Tests run: 1, Failures: 0, Errors: 1, Skipped: 0
[INFO] 
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  4.112 s
[INFO] Finished at: 2019-03-28T20:37:08+01:00
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-surefire-plugin:2.22.0:test (default-test) on project getting-started: There are test failures.
[ERROR] 
[ERROR] Please refer to /home/sarxos/workspace-2019/getting-started/target/surefire-reports for the individual test results.
[ERROR] Please refer to dump files (if any exist) [date]-jvmRun[N].dump, [date].dumpstream and [date]-jvmRun[N].dumpstream.
[ERROR] -> [Help 1]
[ERROR] 
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR] 
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoFailureException

[geoand]

@mkouba Would this PR I closed perhaps be of some use?

#1545

I was also thinking that maybe we should introduce a BuildItem that could be classes that are not but we be consulted (and checked if they contain @Inject annotations) when creating the predicates that prevent beans from being removed?

[mkouba]

@sarxos Ok, so we should clarify that this problem occurs iff an application bean (TestService) is used only in a test class (GreetingResourceTest). In other words, it's not used in the app code AND it's not used in any bean defined in src/test/java.

@geoand Yes, your proposal could one way how to address this issue. The other way would be to register @QuarkusTest as a bean defining annotation - test classes would be analyzed during test even though the test instances would not be instantiated by container.

[geoand]

@geoand Yes, your proposal could one way how to address this issue. The other way would be to register @QuarkusTest as a bean defining annotation - test classes would be analyzed during test even though the test instances would not be instantiated by container.

You are the expert, whatever you think is best :). If you don't have enough time to fix it today, I can most likely squeeze in whichever fix you prefer over the weekend.

[sarxos]

@mkouba Yes, I confirm. This issue appears if and only if my CDI bean is @Inject-ed only in a test class. If I additionally inject it in application class, e.g. a resource annotated with @Path, the problem does not exist.

[sarxos]

@mkouba I do not insist on fixing this, but this behaviour should be described in more details since I spent some time trying to narrow it down and I assume integrators to have similar problems when working with ArC's CDI. I can prepare PR against https://github.com/quarkusio/quarkusio.github.io/blob/develop/_guides/cdi-reference.adoc if you like. Quarkus is developed under Red Hat, am I correct? Do you require signing contributors agreement or similar like the guys from Oracle?

[mkouba]

@sarxos I agree that we should definitely improve the docs in this area. There is already an issue: #1669.

I also think that should fix this. @geoand I lean towards the "register @QuarkusTest as a bean defining annotation" solution but your idea of having a BuildItem to specify classes that should be analyzed for injection points is also interesting. In fact, we could analyze the test classes even without a special BuildItem but the problem is that CDI resolution rules are quite complex so it wouldn't be so easy to find out wheter an injection point resolves to a particular bean.

Let's think about it over the weekend and we can decide early next week.

@sarxos Re contributors agreement - I'm not aware of any agreement. It's just an ASL2 open source project.

[geoand]

Let's think about it over the weekend and we can decide early next week.

👍

[geoand]

@mkouba I am thinking for the time being we should just go with the straight-forward of having @QuarkusTest be a bean defining annotation.
Should the need for a more complex solution (like the one I proposed above) arise, I think we could reevaluate. WDYT?

[mkouba]

@geoand Yes, makes sense. Would you like to send a PR? BTW we should probably also check the JUnit4 runner.

[geoand]

@mkouba Sure thing, I will take care of it either tonight or tomorrow morning :)

[sarxos]

Thank you @geoand :)

[geoand]

You are welcome @sarxos !