Upgrade to Hamcrest 1.3 #404

marcphilipp · 2012-03-21T21:52:57Z

Matchers in JUnitMatchers that are now provided by hamcrest-core are deprecated.
The static methods now delegate to CoreMatchers
The four Matcher classes copied from Hamcrest have been deleted.
Using CoreMatchers where possible

Important:
In some but not all cases, this change will cause compile errors for people
using JUnitMatchers.hasItem(s), both(), and either().

Merging this pull request would fix #36.

- Matchers in JUnitMatchers that are now provided by hamcrest-core are deprecated - Using CoreMatchers where possible *Important:* In some but not all cases, this change will cause compiler errors for people using JUnitMatchers.hasItem(s), both(), and either().

marcphilipp · 2012-03-21T22:06:21Z

@tradej Can you please take a look?

tradej · 2012-03-22T15:53:57Z

@marcphilipp I certainly can and will, but since I don't have any rights in this project, I can't do anything about it. Thank you, though, for the contribution.

dsaff · 2012-03-27T17:51:56Z

Do we anywhere have a link to the Hamcrest javadoc, so we can reference it from the deprecated comments? This would help resolve whether this patch will resolve issues like #272

marcphilipp · 2012-03-27T18:16:32Z

The reason the methods in JUnitMatchers are deprecated is that these matchers are now provided by Hamcrest Code, i.e. CoreMatchers.

Are you looking for the reason Hamcrest changed the signature from Matcher<? extends T> to Matcher<? super T>?

Here's a link to the Hamcrest commit where this change was introduced: http://code.google.com/p/hamcrest/source/detail?r=194&path=/trunk/hamcrest-java/hamcrest-library/src/main/java/org/hamcrest/collection/IsCollectionContaining.java

marcphilipp · 2012-03-27T18:25:21Z

Regarding #272: We are now using CoreMatchers.both and CoreMatchers.either. Both return a CombinableMatcher which allows calls of and as well as or. So, botheither().and()andboth().or()` would still be possible. This has to be changed in Hamcrest Core IMHO.

Edit: A patch has already been submitted to Hamcrest, http://code.google.com/p/hamcrest/issues/detail?id=154.

dsaff · 2012-03-28T00:27:41Z

@marcphilipp, I understand why the methods are deprecated. And I had forgotten that I believe when you run "ant javadoc", javadoc links like {@link CoreMatchers#hasItem(Object)} should work, so scratch that comment.

dsaff · 2012-03-28T00:33:44Z

src/main/java/org/junit/internal/matchers/Each.java

 import org.hamcrest.BaseMatcher;
 import org.hamcrest.Description;
 import org.hamcrest.Matcher;

 public class Each {
-	public static <T> Matcher<Iterable<T>> each(final Matcher<T> individual) {
-		final Matcher<Iterable<T>> allItemsAre = not(hasItem(not(individual)));
+	public static <T> Matcher<Iterable<? super T>> each(final Matcher<? super T> individual) {


Here, and everywhere, I'm trusting you on the typing. I'd recommend you reach out to the junit and hamcrest lists, to get people to compile this with their favorite Java SDKs, to make sure that the generics match up on all compilers (it could be that everything's just that much more standards-compliant than the last time I tried it. If not, there's bound to be someone broken, it seems).

I changed the signature to be the same as for the hasItem matcher (which is also used by Each internally).

dsaff · 2012-03-28T00:36:38Z

Thanks for this. A couple comments inline.

marcphilipp · 2012-03-30T18:54:58Z

I posted a request for feedback to both the JUnit and Hamcrest mailing lists.

sf105 · 2012-04-02T21:58:33Z

for the record, hamcrest java is now in hamcrest/JavaHamcrest on git hub, so you can clone and send us pull requests

As pointed out by Steve Freeman of Hamcrest, the Matcher interface now has an additional method describeMismatch. To be safe to catch such improvements in the future, MatcherAssert is used instead of duplicating its implementation.

marcphilipp · 2012-04-03T19:42:30Z

@dsaff I've found even more code to delete... ;-)

The Each (everyItem) matcher is also shipped with Hamcrest core. Therefore, I deprecated it in JUnitMatchers.
TypeSafeMatcher is also shipped with Hamcrest core now. I wasn't sure whether I could delete the class right away. Some people might have written custom matchers using it. I know I have... Thus, I decided to leave it as is but deprecate it as well.
As pointed out by @sf105, I've used MatcherAssert in Assert.assertThat.

Eagerly waiting for your feedback.

There seems to be a problem compiling calls of the new assertThat(T actual, Matcher<? super T> matcher) using javac and matchers like `hasItem` and `both` that also use `Matcher<? super T>` as parameter.

dsaff · 2012-04-12T19:09:21Z

Marc,

It looks like you need to merge with KentBeck/junit/master. Can you do that? Thanks.

Conflicts: src/main/java/org/junit/rules/ExpectedException.java src/test/java/org/junit/tests/experimental/rules/ExpectedExceptionRuleTest.java

marcphilipp · 2012-04-12T20:44:38Z

@dsaff Done, merged KentBeck/master.

dsaff · 2012-04-12T20:56:30Z

src/test/java/org/junit/tests/running/methods/AnnotationTest.java

-		assertThat(runner.getFailures().toString(), both(containsString("hereAfter")).and(containsString("inTest")));
+		assertThat(
+				runner.getFailures().toString(),
+				CoreMatchers.<String> both(containsString("hereAfter")).and(


Ugh. All of these explicit type parameters still make me think that the typing in hamcrest is more trouble than its worth. What do you think?

Definitely!

As @sf105 has written via email, for both() a different solution would have been to replace it with allOf() which javac seems to like better. I did it this way to demonstrate the problem. I could replace it with allOf() now. What do you think?

On 12 Apr 2012, at 21:56, David Saff wrote:

@@ -143,7 +143,10 @@ public void testTestAndTeardownFailure() throws Exception {
Result runner= core.run(TestAndTeardownFailureTest.class);
assertEquals(1, runner.getRunCount());
assertEquals(2, runner.getFailureCount());

assertThat(runner.getFailures().toString(), both(containsString("hereAfter")).and(containsString("inTest")));

assertThat(

runner.getFailures().toString(),

CoreMatchers.<String> both(containsString("hereAfter")).and(

Ugh. All of these explicit type parameters still make me think that the typing in hamcrest is more trouble than its worth. What do you think?

my point exactly. That's why I use allof() instead of both()

In real life, it works most of the time and I find myself writing little helper methods that often end up improving the code. Anything with typed collections, though, is unpleasant.

S.

Steve Freeman

Winner of the Agile Alliance Gordon Pask award 2006
Book: http://www.growing-object-oriented-software.com

+44 797 179 4105
Twitter: @sf105
Higher Order Logic Limited
Registered office. 2 Church Street, Burnham, Bucks, SL1 7HZ.
Company registered in England & Wales. Number 7522677

OK with allOf if it compiles here.

Fixed in latest commit 2d886e2.

dsaff · 2012-04-12T20:56:52Z

Looks good in general, pending beta testing.

StefanPenndorf · 2012-04-30T21:24:54Z

The upgrade from hamcrest 1.1 to hamcrest 1.2 was rejected because hamcrest 1.2 was not fully backwards compatible with hamcrest 1.1 forcing all JUnit users to possibly upgrade their matchers or other code. See Issue #36.

Has this incompatibility been resolved? Is hamcrest 1.3 backwards compatible with hamcrest 1.1? Or do you expect the junit users to upgrade their tests and libraries?

dsaff · 2012-07-19T18:20:52Z

src/main/java/org/junit/internal/matchers/TypeSafeMatcher.java

 */
+@Deprecated


Why not delete this one, as well?

I thought about deleting it a couple of month ago in a comment:

TypeSafeMatcher is also shipped with Hamcrest core now. I wasn't sure whether I could delete the class right away. Some people might have written custom matchers using it. I know I have... Thus, I decided to leave it as is but deprecate it as well.

Would you rather delete it right away?

No, I think leaving it deprecated makes sense. I've probably reused it, as well. :-)

dsaff · 2012-07-19T18:28:29Z

This is a lot of great work. Thanks, Marc! Some comments inline.

sf105 · 2012-07-20T08:34:15Z

Suggestions:

if StacktracePrintingMatcher is useful, then make it accessible. I would call it isException() because withStacktrace() reads like it's asserting against the stacktrace--and what will you call the matcher that does that when you get around to writing it?
the alsoHasX() chain is not really in the hamcrest model as it's not a composable API. At the least,

andAlso(hasMessage(equalTo(...))) andAlso(hasCause())))

It looks like ExpectedException is really a matcher builder than a matcher itself.

also, hasMessage/hasCause matchers do not report the mismatch, which is the one thing we learned to do, dumping the stack trace isn't always the easiest way to get to the point.

@sf105

Following suggestion by @sf105

marcphilipp · 2012-07-20T12:26:13Z

@sf105 I've followed your suggestions. Would you mind taking another look?

dsaff · 2012-07-20T16:47:38Z

Couple more comments. Looking good!

sf105 · 2012-07-22T08:15:37Z

Looking better. Thinking that some of this eventually belongs in Hamcrest

marcphilipp · 2012-07-24T18:22:54Z

@dsaff So, what's the idea for testing and merging this pull request? Should we publish a snapshot release before merging it or first merge it and then publish a beta version/snapshot?

dsaff · 2012-08-02T20:02:15Z

I'll go through one more review, and merge, and then we'll publish a snapshot for testing. It's easy enough to back up, but I don't want to have an "unofficial branch".

Upgrade to Hamcrest 1.3

dsaff · 2012-08-02T20:12:21Z

Thanks, Marc. This was a monster effort, and putting up with my intermittent attention span was no small part of the difficulty.

Do you have time to spin up a snapshot for community testing?

marcphilipp · 2012-08-05T11:11:32Z

New snapshots are available through Maven and GitHub. I will write an announcement to the JUnit mailing list.

sf105 · 2012-08-05T14:30:13Z

Thanks

S.

On 5 Aug 2012, at 07:11, Marc Philipp wrote:

New snapshots are available through Maven and GitHub. I will write an announcement to the JUnit mailing list.

marcphilipp added 3 commits March 21, 2012 22:48

Remove test for functionality provided by Hamcrest

3cab133

Remove deleted BothTest from AllTests

8d63a24

dsaff mentioned this pull request Mar 27, 2012

Issue / Feature 236 #272

Closed

dsaff reviewed Mar 28, 2012
View reviewed changes

Simplify matcher expression in ParameterSignatureTest

f4df6d5

marcphilipp added 3 commits April 3, 2012 21:24

Remove everyItem matcher now provided by Hamcrest

4f1dbfc

Deprecate TypeSafeMatcher which is now provided by Hamcrest

8b9dfef

assertThat uses org.hamcrest.MatcherAssert

a0eea13

As pointed out by Steve Freeman of Hamcrest, the Matcher interface now has an additional method describeMismatch. To be safe to catch such improvements in the future, MatcherAssert is used instead of duplicating its implementation.

marcphilipp added 2 commits April 9, 2012 20:59

Update references to Hamcrest JAR

a26d3de

Fix javac build

c2bdb1a

There seems to be a problem compiling calls of the new assertThat(T actual, Matcher<? super T> matcher) using javac and matchers like `hasItem` and `both` that also use `Matcher<? super T>` as parameter.

marcphilipp added 2 commits April 12, 2012 21:55

Merge remote-tracking branch 'KentBeck/master' into hamcrest-1.3

e01ccf1

Conflicts: src/main/java/org/junit/rules/ExpectedException.java src/test/java/org/junit/tests/experimental/rules/ExpectedExceptionRuleTest.java

Replace JUnitMatchers with CoreMatchers

fa80b68

dsaff reviewed Apr 12, 2012
View reviewed changes

marcphilipp mentioned this pull request Apr 30, 2012

4.9 and 4.10 "junit" artifacts in Maven Central have hamcrest as dependency defined while in "junit" artifact the hamcrest classes are already included #332

Closed

dsaff reviewed Jul 19, 2012
View reviewed changes

marcphilipp added 4 commits July 19, 2012 20:51

Fix javac compile error

1489b8e

Use allOf() instead of both() to avoid type parameter

2d886e2

Lazy init for composite matcher

6bea82d

Rename and methods to andAlso

872faea

marcphilipp added 6 commits July 20, 2012 11:42

Rename withStacktrace() to isException()/isThrowable()

a37fd9a

Following suggestion by @sf105

Both hasCause() and hasMessage() describe mismatches

5e9467c

Extract matchers to separate classes (refactoring)

a7c4d03

ExpectedException uses builder instead of custom matcher

ec1a5c3

Linear control flow in ExpectedExceptionStatement (refactoring)

e77e1c4

Fix typo in test name

fe10c8a

djh82 mentioned this pull request Jul 31, 2012

support for assertEmpty #465

Closed

dsaff pushed a commit that referenced this pull request Aug 2, 2012

Merge pull request #404 from marcphilipp/hamcrest-1.3

185a219

Upgrade to Hamcrest 1.3

dsaff merged commit 185a219 into junit-team:master Aug 2, 2012

marcphilipp mentioned this pull request Aug 5, 2012

Sync with most recent hamcrest? #36

Closed

marcphilipp mentioned this pull request Aug 22, 2012

Use org.junit.Assert instead of junit.framework.Assert #490

Merged

marcphilipp deleted the hamcrest-1.3 branch January 3, 2013 14:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade to Hamcrest 1.3 #404

Upgrade to Hamcrest 1.3 #404

marcphilipp commented Mar 21, 2012

marcphilipp commented Mar 21, 2012

tradej commented Mar 22, 2012

dsaff commented Mar 27, 2012

marcphilipp commented Mar 27, 2012

marcphilipp commented Mar 27, 2012

dsaff commented Mar 28, 2012

dsaff Mar 28, 2012

marcphilipp Mar 28, 2012

dsaff commented Mar 28, 2012

marcphilipp commented Mar 30, 2012

sf105 commented Apr 2, 2012

marcphilipp commented Apr 3, 2012

dsaff commented Apr 12, 2012

marcphilipp commented Apr 12, 2012

dsaff Apr 12, 2012

marcphilipp Apr 12, 2012

sf105 Apr 12, 2012

dsaff Jul 19, 2012

marcphilipp Jul 19, 2012

dsaff commented Apr 12, 2012

StefanPenndorf commented Apr 30, 2012

dsaff Jul 19, 2012

marcphilipp Jul 19, 2012

dsaff Jul 19, 2012

dsaff commented Jul 19, 2012

sf105 commented Jul 20, 2012

marcphilipp commented Jul 20, 2012

dsaff commented Jul 20, 2012

sf105 commented Jul 22, 2012

marcphilipp commented Jul 24, 2012

dsaff commented Aug 2, 2012

dsaff commented Aug 2, 2012

marcphilipp commented Aug 5, 2012

sf105 commented Aug 5, 2012

		*/
		@Deprecated

Upgrade to Hamcrest 1.3 #404

Upgrade to Hamcrest 1.3 #404

Conversation

marcphilipp commented Mar 21, 2012

marcphilipp commented Mar 21, 2012

tradej commented Mar 22, 2012

dsaff commented Mar 27, 2012

marcphilipp commented Mar 27, 2012

marcphilipp commented Mar 27, 2012

dsaff commented Mar 28, 2012

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dsaff commented Mar 28, 2012

marcphilipp commented Mar 30, 2012

sf105 commented Apr 2, 2012

marcphilipp commented Apr 3, 2012

dsaff commented Apr 12, 2012

marcphilipp commented Apr 12, 2012

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dsaff commented Apr 12, 2012

StefanPenndorf commented Apr 30, 2012

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dsaff commented Jul 19, 2012

sf105 commented Jul 20, 2012

marcphilipp commented Jul 20, 2012

dsaff commented Jul 20, 2012

sf105 commented Jul 22, 2012

marcphilipp commented Jul 24, 2012

dsaff commented Aug 2, 2012

dsaff commented Aug 2, 2012

marcphilipp commented Aug 5, 2012

sf105 commented Aug 5, 2012