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

Since 1.3.0 excluding dependencies during tests with classpathDependencyExcludes does not work #8242

Closed
troosan opened this issue Mar 28, 2020 · 12 comments · Fixed by #25794
Closed
Assignees
Labels
kind/bug Something isn't working
Milestone

Comments

@troosan
Copy link
Contributor

troosan commented Mar 28, 2020

**classpathDependencyExcludes doesn't work anymore **
My project has in its dependencies an SPI implementation (compile dependency). When running the tests though, I include a different implementation (test dependency). So the solution I came up with is

<dependencies>
    <dependency>
        <groupId>org.acme</groupId>
        <artifactId>spi-impl-ws</artifactId>
        <version>1.0</version>
    </dependency>
    <dependency>
        <groupId>org.acme</groupId>
        <artifactId>spi-impl-file</artifactId>
        <version>1.0</version>
        <scope>test</scope>
    </dependency>
</dependencies>
...
<build>
    <plugins>
        <plugin>
            <artifactId>maven-surefire-plugin</artifactId>
            <version>${surefire-plugin.version}</version>
            <configuration>
                <systemProperties>
                    <java.util.logging.manager>org.jboss.logmanager.LogManager</java.util.logging.manager>
                </systemProperties>
                <classpathDependencyExcludes>
                    <classpathDependencyExclude>org.acme:spi-impl-ws</classpathDependencyExclude>
                </classpathDependencyExcludes>
            </configuration>
        </plugin>
    </plugins>
</build>

This worked in version 1.2.0

Expected behavior
Dependency specified in classpathDependencyExcludes is excluded

Actual behavior
Seems the exclude is ignored, because when running the tests, they fail because both jars are included so the service lookup fails because it finds 2 implementations.

Environment (please complete the following information):

  • Output of java -version: 8
  • Quarkus version or git rev: 1.3.0
  • Build tool (ie. output of mvnw --version or gradlew --version): 3.6.3
@troosan troosan added the kind/bug Something isn't working label Mar 28, 2020
@stuartwdouglas stuartwdouglas self-assigned this Mar 30, 2020
@troosan
Copy link
Contributor Author

troosan commented Jun 6, 2020

@stuartwdouglas Do you need more info or is the bug report clear.
I tried with the newly released 1.5, it's still failing.
I created a little test project to reproduce the issue: https://github.com/troosan/quarkus-spi-test-dependency

@stuartwdouglas
Copy link
Member

@aloubyansky do you have any ideas what we could do about this one? We could manually pull this out of the plugin config in the pom but that does not seem great.

@aloubyansky
Copy link
Member

Yeah, that's a tough one. I guess we don't have much of a choice. Given that we can't simply add the deployment deps right on top of the system classloader, I guess we could create a new isolating classloader that will be pulling classes and resources from the system one but allow us to perform additional processing when necessary and make that our runtime classloader. One of the issues with this approach would be adjusting the AppModel deps based on what's actually available on the the classpath. That basically means filtering out some artifacts that aren't found on the classpath. In addition, there could also be extra paths not present in the AppModel added with additionalClasspathElements.

Or we should actually be parsing the plugin configs. While exclusions, in this case, are based on GA, additional paths would still be simply FS paths.

@troosan
Copy link
Contributor Author

troosan commented Sep 29, 2020

Hello again, any plans to fix this issue? Or should I just not run my unit test? ;-)

@stuartwdouglas
Copy link
Member

@aloubyansky Maybe we need our own config to allow exclusions and inclusions (although as this happens before CL creation we can't really use our existing config stuff).

I actually had a similar requirement for this with #11962 which I did not complete as I ran into this issue.

@aloubyansky
Copy link
Member

Unless we parse the plugin config, we need to be analyzing the original test classpath and detect which artifacts it consists of.
Can we assume the original test classloader to always be an instance of URLClassLoader? Otherwise, we'd need to fallback to processing META-INF/maven resources which isn't reliable given that not every Maven artifact includes that entry.

@huseyin-kilic
Copy link

Hello, is there any update on this issue? Thank you.

@aloubyansky
Copy link
Member

@stuartwdouglas another alternative would be to intercept an "before test" event, read the plugin config, resolve the effective app model taking the plugin config into account, serialize it and read it in the quarkus test extension.
I've been considering app model serialization before the test launch anyway because the effective model resolution from the quarkus test extension is significantly slower (it looks like it even increases our CI time by one hour or more #15342).

@voronovmaksim
Copy link

Hello, any news? i came across same issue. My Test loads spi implementation of compile dependency

@aloubyansky aloubyansky self-assigned this May 5, 2022
@aloubyansky
Copy link
Member

I implemented support for it in https://github.com/aloubyansky/quarkus/tree/test-classpath-excludes
Basically, it reads the classpathDependencyExcludes and adds them to the WorkspaceModule. Then in the app model resolver the exclusions are applied.

It's not complete though. Currently, we don't differentiate between unit tests and integration tests when bootstrapping the app, we simply check whether it's any test mode or not. To complete this PR we'd need to be able to check whether we are bootstrapping for surefire or failsafe and apply the corresponding plugin's classpath configs.

Another issue, we might not be able to support custom executions (i.e. not default-test but those configured by users). But only those shown as examples in https://maven.apache.org/surefire/maven-surefire-plugin/examples/configuring-classpath.html and https://maven.apache.org/surefire/maven-failsafe-plugin/examples/configuring-classpath.html

WDYT @stuartwdouglas? Is it worth it taking it further?

PS: I suspect my impl might be doing more than what the plugins do. I am interpreting the configured excludes as exclusions for the resolver while the plugins might be be simply removing the configured artifacts from the resolved set, which is a lot simpler. I'll need to check that.

@aloubyansky
Copy link
Member

Also fyi @famod

@aloubyansky
Copy link
Member

Right, it's more primitive, in the test plugins the artifacts configured to be excluded are removed from the resolved flat set instead of as nodes of dependency trees.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something isn't working
Projects
None yet
5 participants