assert: IsIncreasing et al can return false w/out failing#1430
assert: IsIncreasing et al can return false w/out failing#1430brackendawson wants to merge 3 commits into
Conversation
dolmen
added
bug
Changes Requested
pkg-assert
| @@ -9,7 +9,7 @@ import ( | |||
| func isOrdered(t TestingT, object interface{}, allowedComparesResults []CompareType, failMessage string, msgAndArgs ...interface{}) bool { | |||
There was a problem hiding this comment.
isOrdered is now an Helper. It must have the helper block before calling Fail.
There was a problem hiding this comment.
The call to Helper must be inside IsDecreasing() et al, not inside isOrdered(), this is not a regression as the functions are not currently helpers. We should have a separate issue to add Helper() calls to exported functions which need them.
There was a problem hiding this comment.
The call to Helper must be inside IsDecreasing() et al, not inside isOrdered()
isOrdered now calls assert.Fail so errors will be reported there. But that not what we want. It is now an Helper while it wasn't the case before.
this is not a regression as the functions are not currently helpers.
But you are changing the behaviour.
We should have a separate issue to add Helper() calls to exported functions which need them.
On this matter #1423 is waiting for your approval.
So I'm asking again to fix this.
There was a problem hiding this comment.
Ok, I will fix the Helper method calls in this PR.
brackendawson
force-pushed
the
1419-fail-assertions
branch
from
c30fce7 to
783b7c3
Compare
dolmen
changed the title
If you passed a non-collection to IsIncreasing or any of its compatriots then the assertion would return false without failing the test.
Because maps are collections but not ordered. Co-authored-by: Olivier Mengué <dolmen@cpan.org>
brackendawson
force-pushed
the
1419-fail-assertions
branch
from
783b7c3 to
e2e5626
Compare
|
I've moved the branch for this PR to my personal fork and opened #1787 |
2 participants
Summary
If you passed a non-collection to IsIncreasing or any of its compatriots then the assertion would return false without failing the test.
Changes
isOrdered()returnFail()rather thanfalsein validation so that the test will fail.Motivation
The following invalid usage would previously pass:
Related issues
Closes #1419