-
Notifications
You must be signed in to change notification settings - Fork 196
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
Fix #78 ARQ-1944 Avoid packaging hamcrest in arquillian-junit.jar #113
base: main
Are you sure you want to change the base?
Conversation
…it.jar This way Arquillian tests can use hamcrest matchers, without getting a LinkageError. This change will not be strictly backwards compatible because users will be required to add the hamcrest dependency explicitly.
Many thanks for your contribution! I'm just wondering if such a simple removal is really what we want here. This might cause quite a bit of issues for tests which are already realying on built-in hamcrest (which I believe is shipped with junit as transitive dependency). I could think of doing it a bit differently - introducing a configuration entry WDYT? |
@bartoszmajsak I decided to go the same route that Kent Beck did with JUnit. JUnit used to package hamcrest in their jar, and dropped that in 4.10(?) because of the depencency problems this caused in maven projects. My thinking was that if JUnit can get away with such a breaking change, arquillian can too. The extra complexity is more harmful than the fact that people would have to change their dependencies when upgrading arquillian. |
WDYT @aslakknutsen? Shall we do the same? |
Is there any news on this PR @aslakknutsen @bartoszmajsak? I'm now running a fork, but I prefer to do things properly. |
Wat is the state of this pull request? |
In this shape, it's not what we would confidently ship as next version. Conditional excluding as suggested in #113 (comment) sounds more feasible I believe. |
this is now dated, but as of the latest junit4 junit:junit:jar:4.13.2, hamcrest is still an included dependency and this is where arquillian is picking up the hamcrest dependency from. If I look at the dependency tree in the testenrichers/ejb submodule that does make use of hamcrest, this is the output: [INFO] org.jboss.arquillian.testenricher:arquillian-testenricher-ejb:jar:1.7.3.Final-SNAPSHOT
[INFO] +- org.jboss.arquillian.test:arquillian-test-spi:jar:1.7.3.Final-SNAPSHOT:compile
[INFO] | +- org.jboss.arquillian.core:arquillian-core-spi:jar:1.7.3.Final-SNAPSHOT:compile
[INFO] | | \- org.jboss.arquillian.core:arquillian-core-api:jar:1.7.3.Final-SNAPSHOT:compile
[INFO] | \- org.jboss.arquillian.test:arquillian-test-api:jar:1.7.3.Final-SNAPSHOT:compile
[INFO] +- org.jboss.arquillian.container:arquillian-container-test-spi:jar:1.7.3.Final-SNAPSHOT:compile
[INFO] | +- org.jboss.arquillian.container:arquillian-container-spi:jar:1.7.3.Final-SNAPSHOT:compile
[INFO] | | +- org.jboss.arquillian.config:arquillian-config-api:jar:1.7.3.Final-SNAPSHOT:compile
[INFO] | | +- org.jboss.arquillian.config:arquillian-config-impl-base:jar:1.7.3.Final-SNAPSHOT:compile
[INFO] | | | +- org.jboss.arquillian.config:arquillian-config-spi:jar:1.7.3.Final-SNAPSHOT:compile
[INFO] | | | \- org.jboss.shrinkwrap.descriptors:shrinkwrap-descriptors-spi:jar:2.0.0:compile
[INFO] | | \- org.jboss.shrinkwrap.descriptors:shrinkwrap-descriptors-api-base:jar:2.0.0:compile
[INFO] | \- org.jboss.arquillian.container:arquillian-container-test-api:jar:1.7.3.Final-SNAPSHOT:compile
[INFO] +- org.jboss.shrinkwrap:shrinkwrap-impl-base:jar:1.2.6:test
[INFO] | +- org.jboss.shrinkwrap:shrinkwrap-api:jar:1.2.6:compile
[INFO] | \- org.jboss.shrinkwrap:shrinkwrap-spi:jar:1.2.6:test
[INFO] +- junit:junit:jar:4.13.2:test
[INFO] | \- org.hamcrest:hamcrest-core:jar:1.3:test
[INFO] \- org.jboss.spec.javax.ejb:jboss-ejb-api_3.1_spec:jar:1.0.2.Final:provided A simple exclusion property could still be added for use in JUnitDeploymentAppender |
Short description of what this resolves:
This patch removes the hamcrest classes from arquillian-junit.jar. This way users can chose their own hamcrest version and avoid linkage errors when running with Tomcat. See #78
Changes proposed in this pull request:
Fixes #98, ARQ-1944
This change will not be strictly backwards compatible because users will be required to add the hamcrest dependency explicitly.