quarkusio / quarkus

Quarkus: Supersonic Subatomic Java.
https://quarkus.io
Apache License 2.0
13.64k stars 2.65k forks source link

Dev mode gets incorrect classpath for (some) test-scoped extension dependencies #30317

Open holly-cummins opened 1 year ago

holly-cummins commented 1 year ago

Describe the bug

See https://github.com/quarkiverse/quarkus-pact/issues/28, which has a reproducer. An alternate reproducer is https://github.com/holly-cummins/quarkus-pact/tree/test-test-and-dev-modes (just re-enable the @Disabled tests).

https://github.com/quarkusio/quarkus/pull/30238 fixes the issue for test mode, but the dev mode fix is more complex.

Expected behavior

Test behaviour in dev mode should be the same as normal mode.

Actual behavior

Dependencies need to be scoped with provided, runtime, or compile to be correctly resolved. This isn't true for all dependencies, but I haven't isolated exactly what the defining characteristic is (beyond providing a reproducer :) ) - it's perhaps that the dependency is an extension, or that the problematic dependencies involve kotlin, or that they're using the JUnit libraries so are in a different classloader.

How to Reproduce?

https://github.com/quarkiverse/quarkus-pact/issues/28, has a reproducer. An alternate reproducer is https://github.com/holly-cummins/quarkus-pact/tree/test-test-and-dev-modes (just re-enable the @Disabled tests, and then run mvn clean install from the top level).

Output of uname -a or ver

No response

Output of java -version

No response

GraalVM version (if different from Java)

No response

Quarkus version or git rev

No response

Build tool (ie. output of mvnw --version or gradlew --version)

No response

Additional information

No response

quarkus-bot[bot] commented 1 year ago

/cc @evanchooly(kotlin), @geoand(kotlin)

geoand commented 1 year ago

cc @aloubyansky

holly-cummins commented 1 year ago

Status update on this: https://github.com/quarkusio/quarkus/pull/30383 fixed the issue, but @aloubyansky would like to add tests, so we are keeping this issue open.

beatngu13 commented 1 year ago

I think the following problem is related (if not, please let me know and I will open a separate issue):

The repository for Red Hat's "The Containers and Cloud-Native Roadshow Dev Track - Module 4" uses a H2 DB for the dev profile (and Postgres for prod):

https://github.com/RedHat-Middleware-Workshops/cloud-native-workshop-v2m4-labs/blob/d959279597fa3623429877dfce172007fd45646f/inventory-service/src/main/resources/application.properties#L2

It includes the corresponding H2 extension/dependency in compile scope:

https://github.com/RedHat-Middleware-Workshops/cloud-native-workshop-v2m4-labs/blob/d959279597fa3623429877dfce172007fd45646f/inventory-service/pom.xml#L48-L51

As a result, com.h2database.h2-2.1.214.jar is included in target/quarkus-app/lib/main (i.e. the production artifact).

Since the H2 DB is only needed for testing purposes, the dependency should use test scope. However, when doing so, dev mode fails on start with the following exception:

Caused by: java.lang.ClassNotFoundException: org.h2.Driver
        at java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:641)
        at java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:188)
        at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:520)
        at io.quarkus.bootstrap.classloading.QuarkusClassLoader.loadClass(QuarkusClassLoader.java:505)
        at io.quarkus.bootstrap.classloading.QuarkusClassLoader.loadClass(QuarkusClassLoader.java:455)
        at io.quarkus.bootstrap.classloading.QuarkusClassLoader.loadClass(QuarkusClassLoader.java:505)
        at io.quarkus.bootstrap.classloading.QuarkusClassLoader.loadClass(QuarkusClassLoader.java:455)
        at java.base/java.lang.Class.forName0(Native Method)
        at java.base/java.lang.Class.forName(Class.java:467)
        at io.quarkus.agroal.deployment.AgroalProcessor.validateBuildTimeConfig(AgroalProcessor.java:150)
        ... 12 more

I wonder what is the recommended way to include dependencies only needed for dev and test profiles, without them being leaked into the production artifact?

holly-cummins commented 1 year ago

Hmm, the recommended way is to use the maven test scope. There were some issues with that in some cases, but @aloubyansky fixed them. It looks like the roadshow is using Quarkus version 2.13.7.Final-redhat-00003 for most services and 2.2.5.Final-redhat-00007 for one of them. Alexey's fix went into 2.16.6, so it wouldn't be in those versions.

One slight-workaround you can do is set scope provided. That ensures it's on the classpath, but there's at least a marker in the pom to show it's a 'different' dependency. I forget if they get packaged into the production artifacts with that scope. In principle they shouldn't, but we'd need to check.

beatngu13 commented 1 year ago

Hi @holly-cummins,

thx for your feedback. 🙏

Unfortunately, the problem seems to persist with the newest (3.2.2.Final) Quarkus version. I have created a reproducer based on the default app (quarkus create) with a minimal persistence setup. Just like the roadshow, it uses H2 for dev as well as test (i.e. quarkus-jdbc-h2 using scope test), and Postgres for prod (i.e. quarkus-jdbc-postgresql using scope compile):

https://github.com/beatngu13/code-with-quarkus/commit/a241758ace98da0af5ef0b62e9ce8bf8fda6e83f

The build launches the dev mode via ./mvnw quarkus:dev and yields the same java.lang.ClassNotFoundException: org.h2.Driver:

https://github.com/beatngu13/code-with-quarkus/actions/runs/5667169437/job/15355356313#step:5:234

holly-cummins commented 1 year ago

Oh, how sad! Thanks for checking on the latest version, and for the reproducer.

holly-cummins commented 1 year ago

Looking at your reproducer, @beatngu13, it doesn't look (to me) like it should work. If H2 is used by the main application code, the scope can't be test. That definitely applies if you're running with quarkus:dev, and it's also true if you're running with quarkus:test but expecting the 'non-test' part of the quarkus application to be doing persistence. If the Quarkus app is using H2, H2 has to be on the classpath. For test scope to be the right scope, it would have to be the tests that were doing the persistence themselves, without involving the main application.

I think it's perhaps a similar use case to what we were discussing in https://quarkusio.zulipchat.com/#narrow/stream/187030-users/topic/JPA.20and.20Panache.20with.20multiple.20datasources/near/378423815.

What's the reason to use H2 in dev mode, rather than just using a postgres dev service? The dev service would be less effort, and give you more realism. Is it to avoid using containers?

beatngu13 commented 1 year ago

@holly-cummins there is no particular reason, we – who are new to the Quarkiverse – are just trying to understand the roadshow codebase.

As you already said, the Quarkus documentation also states:

The recommended approach is to use the real database you intend to use in production, especially when Dev Services provide a zero-config database for testing, and running tests against a container is relatively quick and produces expected results on an actual environment. However, it is also possible to use JVM-powered databases for scenarios when the ability to run simple integration tests is required.

I suspect the roadshow example, using H2 for dev, is a bit unfortunate? Since io.quarkus:quarkus-jdbc-h2 uses compile scope, the dependency would be part of the production artifact, although it is only used for development/testing purposes. Which is probably not what the authors intended.

So, my conclusion would be: if you are using H2 and the like (for whatever reason), only do it "to run simple integration tests"?

holly-cummins commented 1 year ago

I agree, @beatngu13, it seems like the roadshow app you're using isn't showing Quarkus best practices. Can you point me to where you got it from? I'll raise an issue there (we should try and move the discussion off this issue, since it's something else). I've had a look at your reproducer but I can't work out the original URL from that.

beatngu13 commented 1 year ago

Unfortunately, I don't remember the exact course title. But maybe I can summon @karstengresch, who helped us during that (and other) course(s).

Karsten, do you recall to which of the courses we did the repository for Red Hat's "The Containers and Cloud-Native Roadshow Dev Track - Module 4" belongs?

holly-cummins commented 1 year ago

I've asked around, and the reason the workshop uses H2 is because containers can't be run in the Cloud IDE being used. There actually is an in-memory H2 dev service, so the workshop could be switched to use that ... but it doesn't resolve the scope/packaging issue. #34021 (sort of) covers the ability to specify an extension should only be included in some launch modes, so it's probably worth moving this discussion over to that work item.