spectrumbdd / spectrum-expectations

MIT License
1 stars 0 forks source link

Design question: matcher extensibility model #2

Open greghaskins opened 7 years ago

greghaskins commented 7 years ago

See also: wiki page on design goals

I propose splitting the difference between Hamcrest and AssertJ matchers.

Hamcrest matchers...

AssertJ matchers...

So what about...

Syntax sketches (just ideas, not implemented)

expect(foo).to(equal(false));
expect(foo).not.to(equal(true)); // bonus: most syntax highlighters will make the ".not." stand out since it is a field, not a method
expect(() -> obj.doSomething()).to(raise(DoSomethingException.class));
expect(obj::doSomething).to(raise(DoSomethingException.class));

expect(foo).to(equal("bar")).or(equal("baz"));
expect(foo).to(contain("hello world")).and.not(contain("goodbye")); // crazy?
ashleyfrieze commented 7 years ago

See ArgumentMatcher in Mockito. This used to be a hamcrest matcher, but in Mockito 2 it became simpler, it is essentially a Predicate.

The shame is that hamcrest was great at explaining why something didn't match. I kind of solved this in my ExceptionExpectation implementation over in Spectrum, by using a Predicate and a String.

greghaskins commented 7 years ago

I'm definitely on board with having our matcher interface be as simple as possible. A boolean and String is all you really need.

Hamcrest and AssertJ use two different extensibility mechanisms, and I'd like input on which to choose (or possible another option altogether). This would be used for both for built-in and custom matchers.

ashleyfrieze commented 7 years ago

Hi Greg

Hamcrest matchers suck. There are two reasons.

  1. They require you to implement a whole class to make your own (there are shortcuts, but then you end up violating the point of the interface)
  2. The actual match method takes Object rather than <T> - this is a total swine in practice.

Mockito moved away from it to ditch Description from their ArgumentMatcher and I snuck in and converted Object to T so that you could define an argument matcher for Mockito in Java 8 with just a lambda. The lambda looks the same as a Predicate as both are of type boolean something(T obj).

To my mind, an extensibility for assertions which takes (String message, Predicate<T> isValid) is the most obvious approach for Java 8. AssertJ has other goals, including compatibility with 6 etc.

However, before you go ahead and build an expectation library, I think you have a better question to answer. What's the point? Are you trying to do AssertJ better? If so, then you've a big hurdle to jump over. Are you trying to do hamcrest 2017? If so, again, how much differentiation are you going to get from this very googlable solution?

I suspect you're trying to provide the familiar syntax of expect from RSpec, Jasmine, etc. In that case, I think you should be trying to implement as many of the familiar keywords from that as possible and to hell with fresh ideas for those things. For extensibility, I'd say that composition and predicates are the easy way forward.

Perhaps, though, you're trying to make an expectation library that's got a new idea in it. If so, then look at this - https://dannorth.net/2016/09/03/scratching-a-junit-itch/ - as Dan North has some additional ideas about accumulating failures, which your library could also support.

In terms of how I can help...

  1. I suspect you're going to tell me to drop my expectedexception PR from Spectrum - that's cool - I may turn it into some code cleaning improvements that I noticed while making it, and not worry about the effort to get it to a PR... I have to assume that some % of the coding I do on Spectrum is not going to make it ;)

  2. If you create the docs saying how the expectation library is expected to work, then I don't mind banging out an implementation. This is familiarly territory.

One last note. I think in JavaScript or Ruby, the idea of using .and. or .not. would be fine, but in Java, that sucks big style. We'd have to decide on and(...) or and(). for such a thing.