Skip to content
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

Update to hamcrest 2.2 #1741

Open
juliocbcotta opened this issue May 19, 2022 · 16 comments
Open

Update to hamcrest 2.2 #1741

juliocbcotta opened this issue May 19, 2022 · 16 comments

Comments

@juliocbcotta
Copy link

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

@linghengqian
Copy link

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.

@marcphilipp
Copy link
Member

@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 marcphilipp closed this as not planned Won't fix, can't repro, duplicate, stale May 31, 2022
@kcooney
Copy link
Member

kcooney commented Jun 1, 2022

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 kcooney reopened this Jun 1, 2022
@panchenko
Copy link
Contributor

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.

@kcooney
Copy link
Member

kcooney commented Jun 1, 2022

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

@kcooney
Copy link
Member

kcooney commented Jun 5, 2022

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?

@juliocbcotta
Copy link
Author

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 AdevintaSpain/Barista#357 (comment) 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.

@kcooney
Copy link
Member

kcooney commented Jun 8, 2022

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 3289d94 for affected APIs).

@juliocbcotta
Copy link
Author

juliocbcotta commented Jun 8, 2022

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.

@kcooney
Copy link
Member

kcooney commented Jun 8, 2022

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
Copy link
Member

kcooney commented Jun 9, 2022

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

@linghengqian
Copy link

  • 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.

@kcooney
Copy link
Member

kcooney commented Nov 27, 2022

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.

@linghengqian
Copy link

  • 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.

@kcooney
Copy link
Member

kcooney commented Dec 4, 2022

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

@dsaff @marcphilipp any ideas why AssumptionViolatedException implements SelfDescribing?

@dsaff
Copy link
Member

dsaff commented Dec 20, 2022

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.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants