Add greaterThan and lessThan test framework assertions#619
Add greaterThan and lessThan test framework assertions#619asfgit merged 4 commits intoapache:masterfrom
Conversation
…lize It breaks output in tmux
aledsage
left a comment
aledsage
left a comment
There was a problem hiding this comment.
LGTM; only one minor comment that needs any attention I think.
There was a problem hiding this comment.
I don't think UNKNOWN_CONDITION should be included here in the knownConditions.
| return actual != null | ||
| && expected != null | ||
| && actual instanceof Comparable | ||
| && actual.getClass().equals(expected.getClass()); |
There was a problem hiding this comment.
I wonder if we should do any coercion, or perhaps even just give it a go to compare! For example, comparing (Integer)1 with (Long)2 might well be possible. However, on balance I think your code is right as it is the simplest (and avoid ClassCastException).
There was a problem hiding this comment.
Interesting one. This implementation has already bitten me: "webapp.reqs.perSec.last expected greaterThan 80 but found 96.8063872255489". I'm inclined to leave this as-is until there are more complaints. I do think that this is to restrictive though - actual and expected could be different types if Comparable is implemented by a shared superclass. Don't think it'll be a common problem though.
Relies on the two objects being compared being instances of Comparable<T>.
sjcorbett
force-pushed
the
test-assertions
branch
from
cbb73d1 to
c51c69a
Compare
|
LGTM, & @aledsage's comment has been addressed. Will merge. |
Documentation on greaterThan and lessThan test assertions Accompanies apache/brooklyn-server#619.
4 participants
Each relies on the two objects being compared being instances of the same
Comparable<T>.I also took the opportunity to simplify the logic in
TestAssertions.