Test dependencies leak into Dev-Mode and QuarkusTest runtime classpath

[famod]

Describe the bug
In a multi-module project, a test module A ends up on the runtime classpath of Dev-Mode in another module B which uses module A in test scope.

E.g.:

root
   ├─ test
   └─ dist

• module test has test support code, e.g. SomeTestSupport.java
• module dist has:
  • a test scope dependency to module test
  • contains the Quarkus build goal
  • tries to load the SomeTestSupport class from a REST controller which is located in src/main/java (main, not test!)

The assumption here is that the controller must not see/be able to load the class.

But instead:

• the class is found and loaded if the controller is invoked in Dev-Mode
• the class is found and loaded if the controller is invoked via a QuarkusTest
• the class is not found if the controller is invoked when started via runner.jar (as expected!)

Expected behavior
Test scope dependencies must not be available on runtime classpath.

Actual behavior
Test scope dependencies leak into runtime classpath of Dev Mode and QuarkusTests.

To Reproduce
Steps to reproduce the behavior:

1. Clone https://github.com/famod/modmono-quarkus.git
2. Switch to branch quarkus-9352-test-leak
3. mvn clean install should fail in dist due to HelloResourceTest getting class found instead of class not found
4. Start Dev-Mode via mvn quarkus:dev -f dist (or similar)
5. curl http://localhost:8080/hello will yield class found instead of class not found
6. Start Jar via java -jar dist/target/modmono-quarkus-dist-1.0-SNAPSHOT-runner.jar
7. curl http://localhost:8080/hello will yield class not found (as expected!)

Configuration
n/a

Environment (please complete the following information):

• Output of uname -a or ver:
  MINGW64_NT-10.0-18363 W4DEUMSY9003463 3.0.7-338.x86_64 2019-11-21 23:07 UTC x86_64 Msys
• Output of java -version:

  openjdk version "11.0.6" 2020-01-14
  OpenJDK Runtime Environment AdoptOpenJDK (build 11.0.6+10)
  OpenJDK 64-Bit Server VM AdoptOpenJDK (build 11.0.6+10, mixed mode)
  

• GraalVM version (if different from Java): n/a
• Quarkus version or git rev: 1.4.2.Final, also happens on "current" master adbe1de
• Build tool (ie. output of mvnw --version or gradlew --version): Apache Maven 3.6.3

Additional context
We found this problem in our current multi-module project (which is quite a lot more complex) when we introduced org.reflections to scan for some specific classes.
reflections failed with:

2020-05-14 22:37:51,448 WARN  [org.ref.Reflections] (Quarkus Main Thread) could not get type for name io.quarkus.test.common.QuarkusTestResourceLifecycleManager from any class loader: org.reflections.ReflectionsException: could not get type for name io.quarkus.test.common.QuarkusTestResourceLifecycleManager
	at org.reflections.ReflectionUtils.forName(ReflectionUtils.java:312)
	at org.reflections.Reflections.expandSuperTypes(Reflections.java:382)
	at org.reflections.Reflections.<init>(Reflections.java:140)
	at com.someproject.middleware.shared.persistence.id.ClasspathScanningTypeContributor.contribute(ClasspathScanningTypeContributor.java:30)
	at org.hibernate.boot.model.process.spi.MetadataBuildingProcess.handleTypes(MetadataBuildingProcess.java:380)
	at org.hibernate.boot.model.process.spi.MetadataBuildingProcess.complete(MetadataBuildingProcess.java:123)
	at io.quarkus.hibernate.orm.runtime.boot.FastBootMetadataBuilder.build(FastBootMetadataBuilder.java:321)
	at io.quarkus.hibernate.orm.runtime.PersistenceUnitsHolder.createMetadata(PersistenceUnitsHolder.java:114)
	at io.quarkus.hibernate.orm.runtime.PersistenceUnitsHolder.constructMetadataAdvance(PersistenceUnitsHolder.java:86)
	at io.quarkus.hibernate.orm.runtime.PersistenceUnitsHolder.initializeJpa(PersistenceUnitsHolder.java:49)
	at io.quarkus.hibernate.orm.runtime.HibernateOrmRecorder$4.created(HibernateOrmRecorder.java:77)
	at io.quarkus.arc.runtime.ArcRecorder.initBeanContainer(ArcRecorder.java:106)
	at io.quarkus.deployment.steps.ArcProcessor$generateResources31.deploy_0(ArcProcessor$generateResources31.zig:513)
	at io.quarkus.deployment.steps.ArcProcessor$generateResources31.deploy(ArcProcessor$generateResources31.zig:36)
	at io.quarkus.runner.ApplicationImpl.<clinit>(ApplicationImpl.zig:487)
	at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
	at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
	at java.base/java.lang.reflect.Constructor.newInstance(Constructor.java:490)
	at java.base/java.lang.Class.newInstance(Class.java:584)
	at io.quarkus.runtime.Quarkus.run(Quarkus.java:60)
	at io.quarkus.runtime.Quarkus.run(Quarkus.java:38)
	at io.quarkus.runtime.Quarkus.run(Quarkus.java:106)
	at io.quarkus.runner.GeneratedMain.main(GeneratedMain.zig:29)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:566)
	at io.quarkus.runner.bootstrap.StartupActionImpl$1.run(StartupActionImpl.java:99)
	at java.base/java.lang.Thread.run(Thread.java:834)
Caused by: java.lang.ClassNotFoundException: io.quarkus.test.common.QuarkusTestResourceLifecycleManager
	at java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:581)
	at java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:178)
	at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:521)
	at io.quarkus.bootstrap.classloading.QuarkusClassLoader.loadClass(QuarkusClassLoader.java:357)
	at io.quarkus.bootstrap.classloading.QuarkusClassLoader.loadClass(QuarkusClassLoader.java:310)
	at org.reflections.ReflectionUtils.forName(ReflectionUtils.java:310)
	... 29 more

This is because we have a test module that provides implementations of QuarkusTestResourceLifecycleManager. Those manager classes are found by reflections (which should not happen) and it tries to load the superclasses (which are not on the runtime classpath, which is good!).
Interestingly, the colleague who found the problem (@tkalmar) reported that if he removed the beans.xml from the test module everything is fine. But in the reproducer this does not make a difference!

cc @harthorst

[famod]

The QuarkusTest part might be debatable but ideally, the "production code" that is being invoked by the test should be isolated from the test classpath.

Edit a few hours later: Such a QuarkusTest mode (or even a new test type) is more of a feature request than a bug. The current way is needed to enable mocking etc.

[tkalmar]

When looking at the target dir while dev-mode is running i notice two things:

1. target\quarkus\bootstrap\dev-app-model.dat contains no references to the test module, target\quarkus\bootstrap\test-app-model.dat contains references to the test module
2. target\modmono-quarkus-dist-dev.jar [only classpath of dev mode] META-INF\MANIFEST.MF contains no reference to test module, META-INF\dev-mode-context.dat does contain references
   Maybe thats related

[famod]

cc @stuartwdouglas @geoand

[geoand]

Sounds more like something for @aloubyansky

[famod]

Sounds more like something for @aloubyansky

Ok. Somehow "classloading" triggered your names in my head. 😉

[stuartwdouglas]

@aloubyansky this is because our workspace discovery just discovers every module in the workspace, it does not test to see if the module is actually a dependency of the current project.

[stuartwdouglas]

Running with -DnoDeps is a temporary workaround

[aloubyansky]

I haven't yet looked into this but it doesn't sound like workspace discovery is wrong here. Sounds like a scope-related issue.

[stuartwdouglas]

No, the project contains two modules, and the main module only has a test scoped dependency on the other module. If you remove the dependency is still leaks into this modules scope.

Basically we are assuming that everything in the active project goes in (via io.quarkus.deployment.dev.DevModeContext#additionalModules which translates into io.quarkus.bootstrap.app.QuarkusBootstrap#additionalApplicationArchives, so this is not resolved via the AppModel).

I think we need to be able to add an optional AppArtifactKey to the additional modules, and if this is present only have the modules be added if they are actually in the AppModel.

[aloubyansky]

I am looking into this. BTW, I don't think the test failure is a wrong. It's on the test classpath, so it should be loadable.

[stuartwdouglas]

I have a pr with a fix. I agree the test should fall, but it should not be there in Dev mode