Fix ResultMatchers#hasFailureContaining #1292
Conversation
Also, introduce hamcrest-library as a dependency. hamcrest-library contains more Matchers.
It should not match when the given PrintableResult has no failures.
| @@ -110,7 +110,7 @@ | |||
| <dependencies> | |||
| <dependency> | |||
| <groupId>org.hamcrest</groupId> | |||
| <artifactId>hamcrest-core</artifactId> | |||
| <artifactId>hamcrest-library</artifactId> | |||
There was a problem hiding this comment.
I don't think we should change this dependency.
|
@junit-team/junit-committers Does anyone know why |
|
My guess is that In any case, it's too late to move it now |
|
That's true, unfortunately. @alb-i986 Can you try to solve this without using |
also improve unit tests
Done, but why's that? |
|
I don't want to force regular users and IDEs like Eclipse (who bundle JUnit) to jump through hoops just so we can use |
|
I'm not sure what you mean by "jump through hoops", but ok, as it turned out I could do without it. But like at least we could add hamcrest-library as a test dependency for ourselves. But I'm not sure if that's easy to configure. |
|
Yeah, that would make sense and should be easy to configure. |
| /** | ||
| * Matches if the number of failures matches {@code countMatcher} | ||
| */ | ||
| public static Matcher<PrintableResult> failureCount(final Matcher<Integer> countMatcher) { |
There was a problem hiding this comment.
I'm curious why we need an overload that takes a Matcher<Integer>. Can you give an example of where this would be more useful than the version that takes an int?
Also, if we add this, shouldn't the parameter have a type of Matcher<? super Integer> (see http://hamcrest.org/JavaHamcrest/javadoc/1.3/org/hamcrest/Matchers.html#arrayWithSize(org.hamcrest.Matcher) for one example in Hamcrest)?
There was a problem hiding this comment.
The existing failureCountIs(final int count) only allows to do an equals match, whereas taking a Matcher of Integer allows clients to do any kind of integer matching, eg. <, >, >=, <=.
It came from the original implementation of this PR 9f5dfd8
But since I later changed implementation (69e8a8a), it's not needed for this PR. I've removed it and I'll open a new PR for that, as I think it still makes sense on its own.
Please let me know your thoughts
|
@alb-i986 would you please update the release notes? |
ResultMatchers.hasFailureContaining() should not match when the given PrintableResult has no failures.
It should not match when the given PrintableResult has no failures.
Please note that this change replaces the dependency hamcrest-core with hamcrest-library. I'm not sure if you are happy with that. If you are, we probably also need to update another couple of lines in the pom, see e.g. https://github.com/junit-team/junit4/blob/master/pom.xml#L308