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

Gradle 1.7.0.CR1 regression: Gradle project does not work with test-fixtures plugin anymore #11144

Closed
asodja opened this issue Aug 1, 2020 · 11 comments · Fixed by #11179
Closed
Assignees
Labels
area/gradle Gradle kind/bug Something isn't working
Milestone

Comments

@asodja
Copy link
Contributor

asodja commented Aug 1, 2020

Describe the bug
In #10327 I was directed to use java-test-fixtures plugin for reused test resources. I am happily using it now. Unfortunately with 1.7.0.CR1 this stopped working. Reproducer can be found here: https://github.com/asodja/quarkus-test-fixtures-reproducer.

When running tests inside IDE (with Intellij Runner) I get:

java.lang.RuntimeException: java.io.UncheckedIOException: Error while reading file as JAR: /Users/anzes/prototype-dev/quarkus-test-fixtures-reproducible/build/libs/quarkus-test-fixtures-reproducible-my-version.jar
	at io.quarkus.test.junit.QuarkusTestExtension.throwBootFailureException(QuarkusTestExtension.java:508)
	at io.quarkus.test.junit.QuarkusTestExtension.beforeEach(QuarkusTestExtension.java:358)
...
Caused by: java.io.FileNotFoundException: /Users/anzes/prototype-dev/quarkus-test-fixtures-reproducible/build/libs/quarkus-test-fixtures-reproducible-my-version.jar (No such file or directory)
	at java.util.zip.ZipFile.open(Native Method)
	at java.util.zip.ZipFile.<init>(ZipFile.java:225)
...

And when I run gradle test from terminal I get:

MyResourceTest > testTestFixtures() FAILED
    java.lang.LinkageError: loader constraint violation: loader (instance of io/quarkus/bootstrap/classloading/QuarkusClassLoader) previously initiated loading for a different type with name "org/my/group/MyService"
        at java.lang.ClassLoader.defineClass1(Native Method)
        at java.lang.ClassLoader.defineClass(ClassLoader.java:757)
        at io.quarkus.bootstrap.classloading.QuarkusClassLoader.loadClass(QuarkusClassLoader.java:420)
        at io.quarkus.bootstrap.classloading.QuarkusClassLoader.loadClass(QuarkusClassLoader.java:363)
        at org.my.group.MyResourceTest.testTestFixtures(MyResourceTest.java:18)

Of course if I do gradle build and then run inside IntelliJ I get the same error as with gradle test, since jar is present in build directory.

Expected behavior
Test works and passes, no exception is thrown.

Actual behavior
Exception is thrown, test fails.

To Reproduce
Steps to reproduce the behavior:

  1. git clone [email protected]:asodja/quarkus-test-fixtures-reproducer.git
  2. Open in IntellIj and set Gradle: Build and run using: IntelliJ IDEA and Run tests using: IntelliJ IDEA
  3. Run MyResourceTest, it fails
  4. open terminal and run gradle test, it also fails
  5. Set version of Quarkus to 1.6.1.Final
  6. run gradle test again, it passes

Environment (please complete the following information):

  • Output of uname -a or ver: MacBook Pro, Darwin Kernel Version 19.5.0
  • Output of java -version: openjdk version "1.8.0_242"
  • Quarkus version: 1.7.0.CR1
  • Build tool: Gradle 6.5.1

Off topic, thank you for implementing #8737. As I follow there is still some work to do, like model caching etc, to have speed of running maven project from Intellij. But it looks like the first big step is done. Using IntelliJ runner really makes a difference when dealing with big projects, since Gradle initialization can be slow.

@asodja asodja added the kind/bug Something isn't working label Aug 1, 2020
@quarkusbot quarkusbot added the area/gradle Gradle label Aug 1, 2020
@quarkusbot
Copy link

/cc @quarkusio/devtools, @glefloch

@glefloch
Copy link
Member

glefloch commented Aug 1, 2020

Thanks for reporting, the test-fixtures dependency does not seem to be available in the test configuration. We don't scan the test-fixture configuration yet. I will add it and use your reproducer to create an integration test.

@gsmet gsmet added this to the 1.7.0.Final milestone Aug 1, 2020
@glefloch glefloch self-assigned this Aug 2, 2020
@gsmet
Copy link
Member

gsmet commented Aug 3, 2020

@glefloch as it's a regression, it would be nice to get this fixed for the Final, which means to merge a fix before Thursday evening. Is it OK for you?

@glefloch
Copy link
Member

glefloch commented Aug 3, 2020

Yes I'm on it, I should be able to create the pull request tonight

@aloubyansky
Copy link
Member

@gsmet, @glefloch is on vacation for two more weeks.

@glefloch
Copy link
Member

glefloch commented Aug 4, 2020

@aloubyansky, I thought adding the testFixtures class directory into the model would be enough. It fixes the issue related to the file not found but not the class loader conflict issue. I try updating the reproducer to avoid the lambda implementation but nothing changed. Could you have a look? I don't think this is related to the introduction of the model.

@aloubyansky
Copy link
Member

I will.

@aloubyansky
Copy link
Member

Just fyi, here is a dirty way of fixing the classloading issue

diff --git a/test-framework/junit5/src/main/java/io/quarkus/test/junit/QuarkusTestExtension.java b/test-framework/junit5/src/main/java/io/quarkus/test/junit/QuarkusTestExtension.java
index 36591a668..bb1102646 100644
--- a/test-framework/junit5/src/main/java/io/quarkus/test/junit/QuarkusTestExtension.java
+++ b/test-framework/junit5/src/main/java/io/quarkus/test/junit/QuarkusTestExtension.java
@@ -8,6 +8,7 @@ import java.io.IOException;
 import java.lang.reflect.Constructor;
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
+import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.util.AbstractMap;
@@ -117,6 +118,7 @@ public class QuarkusTestExtension
 
             Class<?> requiredTestClass = context.getRequiredTestClass();
             testClassLocation = getTestClassesLocation(requiredTestClass);
             final Path appClassLocation = getAppClassLocationForTestLocation(testClassLocation.toString());
 
             PathsCollection.Builder rootBuilder = PathsCollection.builder();
@@ -130,6 +132,9 @@ public class QuarkusTestExtension
                     rootBuilder.add(testResourcesLocation);
                 }
             }
+            if (Files.exists(testClassLocation.getParent().resolve("testFixtures"))) {
+                rootBuilder.add(testClassLocation.getParent().resolve("testFixtures"));
+            }

@aloubyansky
Copy link
Member

The classes from testFixtures are going to be on the original test classpath and loadable from Quarkus tests by delegating to the system classloader. But in this case, a class from testFixtures is also referencing an application class, which we load in the Quarkus class loader. Which results in the LinkageError. So the diff above adds testFixtures to the Quarkus classloader.

@aloubyansky
Copy link
Member

There are other (primarily Gradle-related) issues about recognizing and/or adding more test paths there. This is not the way we want to evolve. We need to properly integrate build system-specific (maven/gradle) workspace paths in here instead adding hacks.

@aloubyansky
Copy link
Member

For 1.7.0.Final I guess we can live with this kind of quick fix, given that it's too late to introduce major changes.

stuartwdouglas added a commit to stuartwdouglas/quarkus that referenced this issue Aug 5, 2020
This is a partial fix for quarkusio#11144. At present
QuarkusTestExtension essentually guesses where
the test classes are, and these are used to build
the ClassLoader.

The Gradle resolver can add additional paths to the
AppArtifact (e.g. the testFixtures path), however they
will not actuall end up in the ClassLoader which causes
problems.
stuartwdouglas added a commit to stuartwdouglas/quarkus that referenced this issue Aug 5, 2020
This is a partial fix for quarkusio#11144. At present
QuarkusTestExtension essentually guesses where
the test classes are, and these are used to build
the ClassLoader.

The Gradle resolver can add additional paths to the
AppArtifact (e.g. the testFixtures path), however they
will not actuall end up in the ClassLoader which causes
problems.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/gradle Gradle kind/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants