llorllale / cactoos-matchers

Elegant object-oriented hamcrest matchers
Other
32 stars 21 forks source link

New constructors for Assertion #161

Open fabriciofx opened 4 years ago

fabriciofx commented 4 years ago

Let's add new constructors do Assertion to make it easier to use:

Assertion(final T test, final Matcher<T> matcher) {
    this("", test, matcher);
}
Assertion(final Text msg, final T test, final Matcher<T> matcher) {
   ....
}
fabriciofx commented 4 years ago

@victornoel WDYT?

victornoel commented 4 years ago

@fabriciofx I don't think we should allow for no message, since it is also enforced in qulice that there should be a message passed to assertions (even though it is only enforced for MatcherAssert, the intention is the same).

Concerning the second one, what would be the need? I don't see any

andreoss commented 4 years ago

it is also enforced in qulice that there should be a message passed to assertions @victornoel It is forced by forbidden-apis

@fabriciofx There could be a different Assertion for that (See: https://github.com/llorllale/cactoos-matchers/issues/156). For example, it would make sense to omit message if several assertions are grouped

new AllOf("must be ...", new Assertion<>(...), new Assertion<>(...))
llorllale commented 4 years ago

@andreoss

@fabriciofx There could be a different Assertion for that (See: #156). For example, it would make sense to omit message if several assertions are grouped

new AllOf("must be ...", new Assertion<>(...), new Assertion<>(...))

Your example is inverting the dependencies; it should be new Assertion<>("text", text, new AllOf<Text>(......))

andreoss commented 4 years ago

@llorllale Good point, though it depends on how readable the output of matcher AllOfwill be I was suggesting junit's approach https://github.com/junit-team/junit5/blob/main/junit-jupiter-api/src/main/java/org/junit/jupiter/api/Assertions.java#L2910 https://github.com/junit-team/junit5/blob/main/junit-jupiter-api/src/main/java/org/junit/jupiter/api/AssertAll.java