Overload ResultMatchers.failureCountIs: take a Matcher of integer

[alb-i986]

This allows clients to assert that the failure count is greaterThan, lessThan, etc.

[kcooney]

Can we make this a Matcher<Number> to ensure that the code compiles?

[alb-i986]

What's wrong?
Compilation looks fine to me:
https://travis-ci.org/junit-team/junit4/builds/131156153

[kcooney]

Nothing is wrong but nothing verifies that you can use this new API with Matcher<Number> or Matcher<Object>

[alb-i986]

I see what you mean.

I understand this may be a bit OT but if we take one step back, are we sure we need a lower bound?

failureCountIs(Matcher<? super Integer>)

I think we need quite the opposite: an upper bound, so as to allow any Matcher<Integer> and below, more specific; we do not want to allow something generic like Matcher<Object>, do we?

See also https://docs.oracle.com/javase/tutorial/java/generics/wildcardGuidelines.html:

An "in" variable is defined with an upper bounded wildcard, using the extends keyword.

But since Integer is final, we may just go for no bound at all: Matcher<Integer> or at most Matcher<? extends Comparable<Integer>>, which happens to be the return type of greaterThan.

    ResultMatchers.failureCountIs(allOf(equalTo(2), greaterThan(2)));

The above line compiles with both Matcher<Integer> and Matcher<? extends Comparable<Integer>>.

Then, again, I understand that Matcher<? super Integer> is what hamcrest uses in cases like this (e.g. in arrayWithSize) , so I'll understand if you prefer to comply.

[kcooney]

Edit: I am sure you do not want an upper bound. Note that hasSize() uses Matcher<? super Integer> (Javadoc)

If the thing you were matching was a Car, You would want a compile error if someone passed in a Matcher<Tesla> because Matcher<Testla> allows you to express constraints that would never pass for a Edsel (ex: expectations on the battery size). But you would want to support passing in Matcher<Vehicle> (for "has engine" and "has passenger capacity) or ``Matcher` (for "is not null").

[marcphilipp]

I absolutely agree with @kcooney.

[kcooney]

@alb-i986 Can the type of matcherTestStub be changed to Matcher<Number>?

Otherwise this LGTM

[kcooney]

@marcphilipp at one point we talked about reducing dependencies from JUnit to Hamcrest. Is that still the plan, or did plans change with JUnit Lambda?

[marcphilipp]

As far as I remember that was at a time when we thought we would evolve the JUnit 4 codebase into JUnit 5. Since that plan has become obsolete I don't see a reason why we shouldn't improve existing Hamcrest-based APIs.

[kcooney]

@marcphilipp Does that mean we should remove the deprecations we did in #1150 ?

[marcphilipp]

On second thought, I don't think we should add additional APIs that rely on Hamcrest. I'm not sure about Hamcrest's progress towards a new version, but IMHO since `ResultMatchers´ should never have been a public API in the first place, I think we should leave it as is.

[kcooney]

Closing

Thanks for the contribution, @alb-i986; sorry we couldn't merge it