Spring-boot: Upgrade to Hamcrest 2.1

[ankurpathak]

Spring Boot Starter Test still uses old Hamcrest 1.3. Now Hamcrest 2.1 released after 3 years and have many new matchers, so it make sense to use it in Spring Boot Starter Test, Please check this url
for more details:
https://github.com/hamcrest/JavaHamcrest/issues/224

[wilkinsona]

so it make sense to use it in Spring Boot Starter Test

Thanks for the suggestion, but I'm not sure that it makes sense, at least not without considering other dependencies.

JUnit 4.12 depends on Hamcrest 1.3. We'd either need to be certain that it is compatible with 2.1 or consider the Hamcrest upgrade alongside a switch to JUnit 5 (version 5 dropped JUnit's Hamcrest dependency).

[wilkinsona]

Let's consider this in 2.2.x alongside https://github.com/spring-projects/spring-boot/issues/14736.

[wilkinsona]

This may still be blocked. While we have upgraded to JUnit 5 in spring-boot-starter-test, to allow users to gently migrate to it, we provide JUnit's Vintage Engine and the JUnit 4 API in the starter. Unfortunately, this means that Hamcrest 1.3 is still pulled in.

@sbrannen Do you know if JUnit 4, when being used "just" as an API on top of the Vintage Engine, requires Hamcrest 1.3, or can we exclude it and upgrade to 2.1?

[sbrannen]

At a first glance, it did not appear to be possible due to packaging changes in Hamcrest from 1.3 to 2.x; however, after some internal discussions with @marcphilipp and a bit of investigative work, it appears that it might be possible by following the Upgrading from Hamcrest 1.x guidelines.

For example, I have the JUnit 4 build working with the following dependencies:

    <dependencies>
        <dependency>
            <groupId>org.hamcrest</groupId>
            <artifactId>hamcrest-core</artifactId>
            <version>1.3</version>
        </dependency>
        <dependency>
            <groupId>org.hamcrest</groupId>
            <artifactId>hamcrest</artifactId>
            <version>2.1</version>
            <scope>test</scope>
        </dependency>
    </dependencies>

Though I still need to check if I can get something similar working with a project using the JUnit Vintage TestEngine on the JUnit Platform.

[sbrannen]

OK, I've gotten the following to work.

1. Create a vanilla Maven-based app using start.spring.io with Spring Boot 2.2 M3.
2. Change the dependencies to the following.

<dependencies>
    <dependency>
        <groupId>org.springframework.boot</groupId>
        <artifactId>spring-boot-starter</artifactId>
    </dependency>

    <dependency>
        <groupId>org.springframework.boot</groupId>
        <artifactId>spring-boot-starter-test</artifactId>
        <scope>test</scope>
        <exclusions>
            <exclusion>
                <groupId>org.hamcrest</groupId>
                <artifactId>hamcrest-core</artifactId>
            </exclusion>
            <exclusion>
                <groupId>org.hamcrest</groupId>
                <artifactId>hamcrest-library</artifactId>
            </exclusion>
        </exclusions>
    </dependency>
    <dependency>
        <groupId>org.hamcrest</groupId>
        <artifactId>hamcrest</artifactId>
        <version>2.1</version>
        <scope>test</scope>
    </dependency>
</dependencies>

1. Run the generated contextLoads() test method on the JUnit Vintage TestEngine (e.g., by running the test using the "JUnit 5" support in your IDE).

Modifying the generated test class as follows also works.

import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.MatcherAssert.assertThat;

import org.junit.Test;
import org.junit.runner.RunWith;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.test.context.junit4.SpringRunner;

@RunWith(SpringRunner.class)
@SpringBootTest
public class Hamcrest21ApplicationTests {

    @Test
    public void contextLoads() {
        assertThat(2 * 3, is(6));
    }

}

@wilkinsona, let me know if that's sufficient for you and whether or not it works for your purposes.

[wilkinsona]

Thanks, @sbrannen. That sounds hopeful on the JUnit side of things. Before I go looking, do you have any information to hand about the packaging changes in Hamcrest 2.1? Our goal in Boot 2.2 is to migrate people seamlessly onto JUnit 5 via the vintage engine. If an upgrade to Hamcrest 2.1 would require changes to a user's tests if they're using Hamcrest, we may want to hold off on moving to 2.1.

[sbrannen]

When I said "packaging", I meant the actual JARs produced, not changes to Java package names. Sorry if that was misleading.

AFAIK, Hamcrest 2.1 should be compatible with Hamcrest 1.3 except that the _internal_ @Factory annotation was removed (which hopefully no Spring Boot users depended on), and a few method signatures were changed with regard to generics.

The latter change likely only applies to users that wrote custom matchers, and resulting compiler errors can be fixed by replacing <T> with <? extends T> in generic type declarations for the affected methods (at least for the changes I encountered).

Regarding the published artifacts, you'll find a summary here, and if you want full details you'll need to read the discussions in https://github.com/hamcrest/JavaHamcrest/issues/224.

[sbrannen]

If an upgrade to Hamcrest 2.1 would require changes to a user's tests if they're using Hamcrest, we may want to hold off on moving to 2.1.

Just to reiterate on the above points...

1. Hamcrest 2.1 appears to be fully _binary compatible_ with Hamcrest 1.3.
2. The new hamcrest artifact contains everything previously in hamcrest-core and hamcrest-library (except the @Factory annotation).
3. AFAIK, users were not supposed to use the @Factory annotation. So although that is gone, I assume it won't affect typical users.
4. Users with custom Matcher implementations may need to make slight modifications to generic signatures in order to compile against Hamcrest 2.1, but code previously compiled against Hamcrest 1.3 should run against Hamcrest 2.1 without recompilation.

[wilkinsona]

Thanks very much, @sbrannen. Sounds like we can proceed with this. The impact on users is likely to be nil. If they have custom matchers the modifications that are needed are minimal enough to meet our requirement for minor Boot releases to be easy to adopt.

[spencergibb]

FYI the hamcrest API is not totally backward compatible. It broke a test in spring-cloud-kubernetes https://jenkins.spring.io/view/Spring%20Cloud/view/CI.MASTER/job/spring-cloud-kubernetes-master-ci/7157/console

https://github.com/spring-cloud/spring-cloud-kubernetes/commit/59a55fdec04cf604939565efe75e6125559ed359

[sbrannen]

Ouch!

16:23:42 [ERROR] /opt/jenkins/data/workspace/spring-cloud-kubernetes-master-ci/spring-cloud-kubernetes-integration-tests/discovery/tests/src/test/java/org/springframework/cloud/kubernetes/it/ServicesIT.java:[44,62] constructor StringContains in class org.hamcrest.core.StringContains cannot be applied to given types;
16:23:42 [ERROR]   required: boolean,java.lang.String
16:23:42 [ERROR]   found: java.lang.String
16:23:42 [ERROR]   reason: actual and formal argument lists differ in length

That's very unfortunate. 😞

@tumbarumba, I understood that Hamcrest 2.1 was intended to be API compatible with Hamcrest 1.3.

Are you aware of any other such breaking changes (other than changes to method signatures related to generics as I mentioned earlier)?

Also, are there are plans to publish a point release (e.g., Hamcrest 2.1.1) that reinstates original APIs such as the StringContains constructor mentioned in the above compilation error?

[sbrannen]

On a side note, @spencergibb, why don't you switch to the StringContains.containsString(...) factory method instead of the constructor for StringContains (potentially with a static import)?

.then().statusCode(200).body(containsString("service-a") {

[spencergibb]

@sbrannen yeah, I'm sure the test could change. I didn't write it but came from the community.

[wilkinsona]

Oh dear. Thanks for letting us know, @spencergibb. Sounds like we need to consider reverting this.

[wilkinsona]

Judging by this commit, it looks like StringEndsWith and StringStartsWith are affected too.

I wonder how many people will be calling the constructors (which are not backwards compatible) directly rather than using the static factory methods (which are backwards compatible). My hunch is that use of the latter will be much more common than the former but that is nothing more than a hunch.

[philwebb]

I've raised https://github.com/hamcrest/JavaHamcrest/issues/259 to ask them to restore the old constructors.

[philwebb]

I'm of the opinion that we should probably upgrade anyway. The fix is relatively simple and hopefully Hamcrest will provide a patch sometime in the future.

[wilkinsona]

Ok, let's stick with this for now. We can see how things pan out prior to 2.2's GA.

[tumbarumba]

I see that this now closed, so this may be moot, but I've just committed a fix to restore the original 1.3 compatible constructors: hamcrest/JavaHamcrest#260.

As suggested by @sbrannen above, it's unusual for the constructors to be used directly - it's more usual for the static builder methods to be used. But, given that the constructors are public, they should be kept backwards compatible.

[sbrannen]

Awesome! Thanks for providing the fix so quickly, @tumbarumba. 👏