junit4 icon indicating copy to clipboard operation
junit4 copied to clipboard

Update to hamcrest 2.2

Open juliocbcotta opened this issue 2 years ago • 16 comments

Can we have a version of junit4 with a recent version of hamcrest? Thanks

juliocbcotta avatar May 19 '22 12:05 juliocbcotta

Is it really possible to do this? Junit 4.13 requires a minimum JDK version of 1.5, but hamcrest 2.2 requires a minimum JDK version of 1.7.

linghengqian avatar May 30 '22 11:05 linghengqian

@linghengqian Thanks for pointing that out!

You're right, we won't be upgrading in that case but it should be possible to use Hamcrest 2.2 assertions by using the dependency management capabilities of your build tool of choice.

marcphilipp avatar May 31 '22 12:05 marcphilipp

Note that JDK 1.6 reached EOL June, 2017 so "JUnit 4.13 requires a minimum JDK version of 1.5" shouldn't block us from considering a task ("JUnit 4 is in maintenance mode" is, of course, a consideration).

Do the various build tools have a simple way to use JUnit 4.13 and Hamcrest 2.2? (I don't use maven at my work so I am unsure).

kcooney avatar Jun 01 '22 13:06 kcooney

Surely it's possible to specify newer version of Hamcrest in project dependencies. The latest Hamcrest has more features, so if one uses it - obviously that would be the latest version, not something from 10 years ago. Another approach could be removing the Hamcrest dependency at all, or making it compileOnly.

panchenko avatar Jun 01 '22 15:06 panchenko

Unfortunately some JUnit public APIs reference Hamcrest types, so removing the dependency would be a breaking change

kcooney avatar Jun 01 '22 17:06 kcooney

Let me try to ask this in a different way. What concrete problems do people see because JUnit depends on an older version of Hamcrest? Were there API changes in Hamcrest that were not backwards compatible?

kcooney avatar Jun 05 '22 17:06 kcooney

In Android, the espresso-contrib lib updated to hamcrest 2.2 which seems to cause this problem for me.

java.lang.NoClassDefFoundError: Failed resolution of: Lorg/hamcrest/Matchers;
at androidx.test.espresso.matcher.ViewMatchers.withId(ViewMatchers.java:1)

This issue https://github.com/AdevintaSpain/Barista/issues/357#issuecomment-1076357823 seems related.

Android devs can't move to junit5 officially and keeping junit4 with an old version of hamcrest seems problematic for the future. Forcing the usage of new version of hamcrest seems to solve the issue right now, but god knows how it will break next.

juliocbcotta avatar Jun 07 '22 21:06 juliocbcotta

Since upgrading Hamcrest would be a breaking change, if we want to resolve this I have two questions:

  1. What would be the version number for JUnit? 4.14? 5.0?
  2. If we are making a breaking change anyway, should we just remove all Hamcrest dependencies? To do this we would need a minor release to deprecate APIs and add replacements (see https://github.com/junit-team/junit4/commit/3289d9422b535666786aa3201ea0f31ad5e1f7e3 for affected APIs).

kcooney avatar Jun 08 '22 15:06 kcooney

To not break anything and have an easier transition, it would be better to have a new artefact and a new package, like what retrofit does.

https://jakewharton.com/java-interoperability-policy-for-major-version-updates/

A new artefact like junit:junit4:5.0.0. It would require every other package to migrate to the new dependency, but it would be the safest option where you do as many breaking changes as required and still have the possibility of having releases of the current artefact.

Please notice: I am not an expert on this stuff.

juliocbcotta avatar Jun 08 '22 15:06 juliocbcotta

JUnit4 has has a single artifact for over a decade and is in maintenance mode so I personally would vote against a new artifact. We will not be changing the package names for our classes.

JUnit4 is the most popular Java library in the world. Major changes are costly and affect countless open source projects with shoestring budgets.

kcooney avatar Jun 08 '22 16:06 kcooney

Note that Hamcrest has a documented workaround: http://hamcrest.org/JavaHamcrest/distributables#upgrading-from-hamcrest-1x

kcooney avatar Jun 09 '22 00:06 kcooney

  • I think the next version with a breaking update should be defined as 4.14. This avoids the possible definitional conflict with Junit5 after being defined as 5.0.0 in the first place.

  • 4.13 has been released for a long time, and almost everyone is moving away from org.junit.Assert.assertThat, many people have planned to migrate to org.hamcrest.MatcherAssert.assertThat or org.assertj.core.api.Assertions.assertThat, these people are just waiting for Junit4 to completely remove Hamcrest Related deprecated APIs.

linghengqian avatar Aug 04 '22 11:08 linghengqian

I had some free time so I did a bit of analysis. There are a few places where JUnit uses Hamcrest outside of the assert classes, and some of them would be hard to remove in a way that remained backwards compatible.

  • Assume.assumeThat() would have to be removed (and, ideally, replaced with something else, possibly something that looks like ErrorCollector.checkSucceeds())
  • ErrorCollector.checkThat() would have to be removed (checkSucceeds() and checkThrows() look like they would cover that use case)
  • org.junit.experimental.results.ResultMatchers (used in numerous internal tests) would need to move to the "test" tree
  • org.junit.internal.AssumptionViolatedException (parent of org.junit.AssumptionViolatedException) has a field of type org.hamcrest.Matcher that is included in its serialized form

The last one looks particularly problematic. I'm not sure why AssumptionViolatedException implements SelfDescribing (it seems to have been there from the beginning). To remove the fMatcher field we would need to first have a version of JUnit that still had the field but stopped serializing it.

Given all that, I'm not sure how doable "removing Hamcrest dependencies" would be.

kcooney avatar Nov 27 '22 03:11 kcooney

  • It seems that for a long time no one expressed an opinion on AssumptionViolatedException and SelfDescribing. Maybe work on it first and release 4.13.3. If we set aside similar questions, over time, perhaps similar inquiries will arise outside of Android.

linghengqian avatar Dec 04 '22 18:12 linghengqian

I'm not sure eight days constitutes "a long time ago" 🙂

@dsaff @marcphilipp any ideas why AssumptionViolatedException implements SelfDescribing?

kcooney avatar Dec 04 '22 18:12 kcooney

My guess is that at the time, we were excited about the way that hamcrest Description objects allowed for ways to be more structured than toString. Seems like something we might have been excited about at the time.

(A recent-ish job transition has meant that it's more likely than it's been in at least 10 years that "People working on Android test cases experience pain because of JUnit4 issues" is something I could use paid time to look into, but I have not begun to actually do so.)

dsaff avatar Dec 20 '22 20:12 dsaff