junit-team / junit4

A programmer-oriented testing framework for Java.
https://junit.org/junit4
Eclipse Public License 1.0
8.51k stars 3.24k forks source link

Update to hamcrest 2.2 #1741

Open juliocbcotta opened 2 years ago

juliocbcotta commented 2 years ago

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

linghengqian commented 2 years ago

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 commented 2 years ago

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

kcooney commented 2 years ago

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

panchenko commented 2 years ago

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 commented 2 years ago

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

kcooney commented 2 years ago

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 commented 2 years ago

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.

kcooney commented 2 years ago

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).
juliocbcotta commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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

linghengqian commented 1 year ago
kcooney commented 1 year ago

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.

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 commented 1 year ago
kcooney commented 1 year ago

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

@dsaff @marcphilipp any ideas why AssumptionViolatedException implements SelfDescribing?

dsaff commented 1 year ago

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