assert: IsIncreasing et al can return false w/out failing #1430
Conversation
| ) | ||
|
|
||
| // isOrdered checks that collection contains orderable elements. | ||
| 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.
c30fce7 to
783b7c3
Compare
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é <[email protected]>
783b7c3 to
e2e5626
Compare
|
I've moved the branch for this PR to my personal fork and opened #1787 |
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