Uploaded image for project: 'Spring Framework'
  1. Spring Framework
  2. SPR-9940

Spring test module shouldn't pull in hamcrest-all as hard dependency

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Complete
    • Affects Version/s: 3.2 M2
    • Fix Version/s: 3.2 RC1
    • Component/s: Test
    • Labels:
      None
    • Last commented by a User:
      false

      Description

      Trying to upgrade Spring Data JPA from 3.1.2.RELEASE to 3.2.0.BUILD-SNAPSHOT broke my dependency setup as the test module apparently has hamcrest-all as hard dependency in a really ancient version (1.1 (2007), current is 1.3).

      The reason this broke is that my project depends on hamcrest-library only, so that my local declaration of the 1.3 version does not get preferred. I fear we might break quite a few projects with that move as setting up JUnit and Hamcrest to work reasonably is quite a dance anyway and it's not really obvious why tests suddenly don't compile anymore.

      Can we make Hamcrest an optional dependency?
      If not, do we really need the broad hamcrest-all or is hamcrest-library enough as it contains all the Matcher implementations (see http://code.google.com/p/hamcrest/wiki/HamcrestDistributables for details).
      If not, could we at least refer to the latest version (1.3).

        Issue Links

          Activity

          Hide
          cbeams Chris Beams added a comment -

          Adding Rob Winch as a watcher.

          Show
          cbeams Chris Beams added a comment - Adding Rob Winch as a watcher.
          Hide
          rstoya05-aop Rossen Stoyanchev added a comment -

          This is coming from the Spring MVC Test dependencies. I'll take a look.

          Show
          rstoya05-aop Rossen Stoyanchev added a comment - This is coming from the Spring MVC Test dependencies. I'll take a look.
          Hide
          rstoya05-aop Rossen Stoyanchev added a comment -

          This should now be resolved in commit 242bf7c4e303ce6d51ba7c0a3943b0b815d685c8. See the commit comment for details.

          /cc Sam Brannen, Rob Winch, Chris Beams

          Show
          rstoya05-aop Rossen Stoyanchev added a comment - This should now be resolved in commit 242bf7c4e303ce6d51ba7c0a3943b0b815d685c8 . See the commit comment for details. /cc Sam Brannen , Rob Winch , Chris Beams
          Hide
          sbrannen Sam Brannen added a comment -

          Note that this issues is closely related to the changes made in conjunction with SPR-6966.

          Show
          sbrannen Sam Brannen added a comment - Note that this issues is closely related to the changes made in conjunction with SPR-6966 .
          Hide
          sbrannen Sam Brannen added a comment - - edited

          Reopening this issue since any dependency on hamcrest libraries must be optional.

          The generated POM for spring-test now contains the following required transitive dependency:

          <dependency>
            <groupId>org.hamcrest</groupId>
            <artifactId>hamcrest-library</artifactId>
            <version>1.3</version>
            <scope>compile</scope>
          </dependency>

          Furthermore, existing JUnit and TestNG users who depend on the functionality provided in hamcrest-core may be confused by the omission of hamcrest-core as an optional dependency in the POM. Note that this is a regression from the changes made in conjunction with SPR-6966. I would therefore recommend that both hamcrest-core and hamcrest-library be included in the POM as optional dependencies.

          Show
          sbrannen Sam Brannen added a comment - - edited Reopening this issue since any dependency on hamcrest libraries must be optional . The generated POM for spring-test now contains the following required transitive dependency: < dependency > < groupId >org.hamcrest</ groupId > < artifactId >hamcrest-library</ artifactId > < version >1.3</ version > < scope >compile</ scope > </ dependency > Furthermore, existing JUnit and TestNG users who depend on the functionality provided in hamcrest-core may be confused by the omission of hamcrest-core as an optional dependency in the POM. Note that this is a regression from the changes made in conjunction with SPR-6966 . I would therefore recommend that both hamcrest-core and hamcrest-library be included in the POM as optional dependencies.
          Hide
          rstoya05-aop Rossen Stoyanchev added a comment -

          I'll add hamcrest-core even though it is already a transitive dependency of hamcrest-library.

          Note however that this change is not exactly a regression of SPR-6966. While JUnit 4.4 thru 4.8 embeds a subset of hamcrest 1.1 making it impossible to choose a different version, this change lists hamcrest-library as a transitive dependency. If a project already lists hamcrest-library as a direct dependency, its version should be chosen over the one specified in spring-test. A project may also list hamcrest-all as a direct dependency in which case it will end up with both hamcrest-library/core and hamcrest-all. However, since Spring MVC Test is careful not to use newer hamcrest functionality without checking if it is available and since it lists the latest hamcrest version as a dependency, this also shouldn't cause any compilation or runtime issues. So, really the only side effect is the possibility of having duplicate hamcrest dependencies on the classpath but nothing will break, which is very different from the issue in SPR-6966.

          I'll make hamcrest an optional dependency for now but it would have the added inconvenience for Spring MVC Test who would need to list hamcrest as a direct dependency before they can use Spring MVC Test. The other alternative we should consider still is to release a separate spring-test-mvc modules.

          Show
          rstoya05-aop Rossen Stoyanchev added a comment - I'll add hamcrest-core even though it is already a transitive dependency of hamcrest-library . Note however that this change is not exactly a regression of SPR-6966 . While JUnit 4.4 thru 4.8 embeds a subset of hamcrest 1.1 making it impossible to choose a different version, this change lists hamcrest-library as a transitive dependency. If a project already lists hamcrest-library as a direct dependency, its version should be chosen over the one specified in spring-test. A project may also list hamcrest-all as a direct dependency in which case it will end up with both hamcrest-library/core and hamcrest-all. However, since Spring MVC Test is careful not to use newer hamcrest functionality without checking if it is available and since it lists the latest hamcrest version as a dependency, this also shouldn't cause any compilation or runtime issues. So, really the only side effect is the possibility of having duplicate hamcrest dependencies on the classpath but nothing will break, which is very different from the issue in SPR-6966 . I'll make hamcrest an optional dependency for now but it would have the added inconvenience for Spring MVC Test who would need to list hamcrest as a direct dependency before they can use Spring MVC Test. The other alternative we should consider still is to release a separate spring-test-mvc modules.
          Hide
          sbrannen Sam Brannen added a comment -

          Rossen Stoyanchev, thanks for making the hamcrest dependencies in spring-test-mvc optional!

          I agree that merging spring-test and spring-test-mvc into a single artifact is cumbersome with regard to required vs. optional dependencies.

          From the perspective of a consumer of spring-test, hamcrest libraries are often not required – for example, if you're just using something like JdbcTestUtils, ReflectionTestUtils, etc. Even when using the TestContext framework in conjunction with JUnit, you still may not need the hamcrest libraries.

          On the other hand, a consumer of spring-test-mvc functionality would need the hamcrest libraries to be present.

          As you mentioned, splitting out spring-test-mvc into a separate artifact would overcome these issues. So we should keep that in consideration.

          Show
          sbrannen Sam Brannen added a comment - Rossen Stoyanchev , thanks for making the hamcrest dependencies in spring-test-mvc optional! I agree that merging spring-test and spring-test-mvc into a single artifact is cumbersome with regard to required vs. optional dependencies. From the perspective of a consumer of spring-test , hamcrest libraries are often not required – for example, if you're just using something like JdbcTestUtils , ReflectionTestUtils , etc. Even when using the TestContext framework in conjunction with JUnit, you still may not need the hamcrest libraries. On the other hand, a consumer of spring-test-mvc functionality would need the hamcrest libraries to be present. As you mentioned, splitting out spring-test-mvc into a separate artifact would overcome these issues. So we should keep that in consideration.

            People

            • Assignee:
              rstoya05-aop Rossen Stoyanchev
              Reporter:
              olivergierke Oliver Gierke
              Last updater:
              Sam Brannen
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:
                Days since last comment:
                5 years, 4 weeks ago