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

Sync with most recent hamcrest? #36

Closed
dsaff opened this issue Oct 23, 2009 · 16 comments · Fixed by #404
Closed

Sync with most recent hamcrest? #36

dsaff opened this issue Oct 23, 2009 · 16 comments · Fixed by #404

Comments

@dsaff
Copy link
Member

dsaff commented Oct 23, 2009

No description provided.

@chrisvest
Copy link
Contributor

I noticed that an attempt to upgrade to hamcrest 1.2 was rolled back. I wonder why that was.

@dsaff
Copy link
Member Author

dsaff commented Dec 2, 2009

Hamcrest 1.2 changed the type signatures of its built-in matchers to be incompatible with Hamcrest 1.1, and all versions of JUnit so far. We are investigating a non-hamcrest-based assertThat expression to be used in future versions.

@leoherbie
Copy link

Hamcrest 1.2 is now in the central maven repository if you wanted junit to depend on it.

@dsaff
Copy link
Member Author

dsaff commented Sep 30, 2010

Yes, and the signatures are still incompatible.

@dsyer
Copy link

dsyer commented Nov 12, 2010

Why doesn't JUnit just re-package the Hamcrest code it wants to use internally with different package names, so that people can use what ever version of Hamcrest they like at the same time?

@dsaff
Copy link
Member Author

dsaff commented Nov 12, 2010

dsyer,

It's not quite that simple. We use hamcrest very little internally (and I hope to expunge it entirely soon). Since whatever we do is going to break someone, I'd like to just find the quickest path to the least pain that involves a full purge of hamcrest from the JUnit jars entirely. More to come.

@gokhanozbulak
Copy link

Is there any update on this? We still can't use hamcrest 1.2 with a stable version of junit?

Thanks

@drothmaler
Copy link
Contributor

yes you can use hamcrest 1.2; you simple have to use the "junit-dep" library (junit without hamcrest") instead of the default "junit"; than you can include the hamcrest 1.2/1.3 library youself.
The only thing you would have to change in you code, is a search and replace from:
org.junit.Assert.assertThat
to:
org.hamcrest.MatcherAssert.assertThat
This should not be a big deal...
EDIT:
Sorry, I think you don't even have to do the MatcherAssert replacement; but if hamcrest could be stripped from junit, it still might be a good idea

@gjoseph
Copy link

gjoseph commented Oct 13, 2011

You do have to user MatcherAssert.assertThat() if you want the assertion error to make use of the describeMismatch method (i.e method which allows matchers to describe what is wrong, not just state that it is).
I just wish either junit or hamcrest would rename their method (or junit remove it altogether?) so that static imports could be used ...

@tradej
Copy link

tradej commented Oct 19, 2011

Hey, guys, I am working on updating hamcrest and jUnit in Fedora and as was stated, they don't work together because of the different Matcher signatures. I would like to know what stance and ideas you have on this - we need it in Fedora and we certainly don't want to fork unless necessary, so if we agree on some changes, I can push the update your way.

@dsaff
Copy link
Member Author

dsaff commented Oct 21, 2011

We've decided to move ahead with a modified plan of attack. In https://github.com/KentBeck/junit/commit/8c2f7890154618b9409ef2baf7d96ccebec8d1ba
we changed JUnit's definition of assertThat(), and also added a little documentation about in which situations JUnit considers Matcher to be correctly typed for various definitions of T.

At this point, we need someone to volunteer to figure out what further changes would need to be made in JUnit to bring it up to date.

@tradej
Copy link

tradej commented Oct 25, 2011

Well, I think you found your volunteer. I'll look into this and come up with a plan of what to do.

@dsaff
Copy link
Member Author

dsaff commented Oct 26, 2011

Thanks, Tomas!

@tradej
Copy link

tradej commented Nov 11, 2011

I have looked into the matter and come up with 2 versions of what can be done. I assume you intend to keep JUnit working as it has been until now, therefore no drammatic changes like JUnit 3 > JUnit 4.

  1. As issue Bug in assertNull #28 filed for Hamcrest (http://code.google.com/p/hamcrest/issues/detail?id=28&can=1) indicates, the change of signatures (related here mostly to classes IsCollectionContaining and JUnitMatchers) was necessary as '? extends T' typing is not correct. Therefore, I would propose changing the signatures of JUnit accordingly. This, however, would keep the dependence on Hamcrest, which you said you want to eliminate. Changing corresponding signatures of JUnit classes to accept '? super T' seems to be functioning quite well as most of the calls are done exactly with arguments of the type T, apart from the ExpectedException class, whose method expect(Matcher<?> matcher) would be subject to significant alterations.

  2. The second option would be abandoning Hamcrest library and duplicating its functionality in internal classes. However, even with this option, I would suggest changing the signatures (the reason is explained in the Hamcrest issue page linked above) instead of replicating old Hamcrest's behavior.

On a related note, may I know what makes you want to remove the dependency on Hamcrest?

@tradej
Copy link

tradej commented Jan 3, 2012

Hi, I was wondering if anyone's looking into this. If you're not happy with my idea, feel free to comment on it.

@marcphilipp
Copy link
Member

Done in #404.

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

Successfully merging a pull request may close this issue.

9 participants