-
Notifications
You must be signed in to change notification settings - Fork 40.8k
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
Upgrade to Hamcrest 2.1 #15555
Comments
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). |
Let's consider this in 2.2.x alongside #14736. |
This may still be blocked. While we have upgraded to JUnit 5 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? |
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 |
OK, I've gotten the following to work.
<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>
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. |
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. |
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 The latter change likely only applies to users that wrote custom matchers, and resulting compiler errors can be fixed by replacing Regarding the published artifacts, you'll find a summary here, and if you want full details you'll need to read the discussions in hamcrest/JavaHamcrest#224. |
Just to reiterate on the above points...
|
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. |
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 |
Ouch!
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 |
On a side note, @spencergibb, why don't you switch to the .then().statusCode(200).body(containsString("service-a") { |
@sbrannen yeah, I'm sure the test could change. I didn't write it but came from the community. |
Oh dear. Thanks for letting us know, @spencergibb. Sounds like we need to consider reverting this. |
Judging by this commit, it looks like I wonder how many people will be calling the constructors (which are not backwards compatible) directly rather than using the |
I've raised hamcrest/JavaHamcrest#259 to ask them to restore the old constructors. |
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. |
Ok, let's stick with this for now. We can see how things pan out prior to 2.2's GA. |
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. |
Awesome! Thanks for providing the fix so quickly, @tumbarumba. 👏 |
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:
hamcrest/JavaHamcrest#224
The text was updated successfully, but these errors were encountered: