Skip to content

assert: IsIncreasing et al can return false w/out failing #1430

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
brackendawson wants to merge 3 commits into from
Closed

assert: IsIncreasing et al can return false w/out failing #1430

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8b4aef0
IsIncreasing et al can return false w/out failing
brackendawson Oct 29, 2021
685783a
Clarify error message for incorrect use of IsIncreasing et al
brackendawson Aug 8, 2023
e2e5626
Add missing Helper calls to IsIncreasing et al
brackendawson Feb 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion assert/assertion_order.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (
func isOrdered(t TestingT, object interface{}, allowedComparesResults []CompareType, failMessage string, msgAndArgs ...interface{}) bool {
Copy link
Copy Markdown
Collaborator

@dolmen dolmen Jul 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isOrdered is now an Helper. It must have the helper block before calling Fail.

Copy link
Copy Markdown
Collaborator Author

@brackendawson brackendawson Aug 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Collaborator

@dolmen dolmen Oct 16, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Collaborator Author

@brackendawson brackendawson Feb 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I will fix the Helper method calls in this PR.

objKind := reflect.TypeOf(object).Kind()
if objKind != reflect.Slice && objKind != reflect.Array {
return false
return Fail(t, fmt.Sprintf("object %T is not an ordered collection", object), msgAndArgs...)
}

objValue := reflect.ValueOf(object)
Expand Down Expand Up @@ -50,6 +50,9 @@ func isOrdered(t TestingT, object interface{}, allowedComparesResults []CompareT
// assert.IsIncreasing(t, []float{1, 2})
// assert.IsIncreasing(t, []string{"a", "b"})
func IsIncreasing(t TestingT, object interface{}, msgAndArgs ...interface{}) bool {
if h, ok := t.(tHelper); ok {
h.Helper()
}
return isOrdered(t, object, []CompareType{compareLess}, "\"%v\" is not less than \"%v\"", msgAndArgs...)
}

Expand All @@ -59,6 +62,9 @@ func IsIncreasing(t TestingT, object interface{}, msgAndArgs ...interface{}) boo
// assert.IsNonIncreasing(t, []float{2, 1})
// assert.IsNonIncreasing(t, []string{"b", "a"})
func IsNonIncreasing(t TestingT, object interface{}, msgAndArgs ...interface{}) bool {
if h, ok := t.(tHelper); ok {
h.Helper()
}
return isOrdered(t, object, []CompareType{compareEqual, compareGreater}, "\"%v\" is not greater than or equal to \"%v\"", msgAndArgs...)
}

Expand All @@ -68,6 +74,9 @@ func IsNonIncreasing(t TestingT, object interface{}, msgAndArgs ...interface{})
// assert.IsDecreasing(t, []float{2, 1})
// assert.IsDecreasing(t, []string{"b", "a"})
func IsDecreasing(t TestingT, object interface{}, msgAndArgs ...interface{}) bool {
if h, ok := t.(tHelper); ok {
h.Helper()
}
return isOrdered(t, object, []CompareType{compareGreater}, "\"%v\" is not greater than \"%v\"", msgAndArgs...)
}

Expand All @@ -77,5 +86,8 @@ func IsDecreasing(t TestingT, object interface{}, msgAndArgs ...interface{}) boo
// assert.IsNonDecreasing(t, []float{1, 2})
// assert.IsNonDecreasing(t, []string{"a", "b"})
func IsNonDecreasing(t TestingT, object interface{}, msgAndArgs ...interface{}) bool {
if h, ok := t.(tHelper); ok {
h.Helper()
}
return isOrdered(t, object, []CompareType{compareLess, compareEqual}, "\"%v\" is not less than or equal to \"%v\"", msgAndArgs...)
}
37 changes: 25 additions & 12 deletions assert/assertion_order_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package assert

import (
"bytes"
"fmt"
"testing"
)

Expand Down Expand Up @@ -43,10 +44,13 @@ func TestIsIncreasing(t *testing.T) {
{collection: []uint64{2, 1}, msg: `"2" is not less than "1"`},
{collection: []float32{2.34, 1.23}, msg: `"2.34" is not less than "1.23"`},
{collection: []float64{2.34, 1.23}, msg: `"2.34" is not less than "1.23"`},
{collection: struct{}{}, msg: `object struct {} is not an ordered collection`},
} {
out := &outputT{buf: bytes.NewBuffer(nil)}
False(t, IsIncreasing(out, currCase.collection))
Contains(t, out.buf.String(), currCase.msg)
t.Run(fmt.Sprintf("%#v", currCase.collection), func(t *testing.T) {
out := &outputT{buf: bytes.NewBuffer(nil)}
False(t, IsIncreasing(out, currCase.collection))
Contains(t, out.buf.String(), currCase.msg)
})
}
}

Expand Down Expand Up @@ -88,10 +92,13 @@ func TestIsNonIncreasing(t *testing.T) {
{collection: []uint64{1, 2}, msg: `"1" is not greater than or equal to "2"`},
{collection: []float32{1.23, 2.34}, msg: `"1.23" is not greater than or equal to "2.34"`},
{collection: []float64{1.23, 2.34}, msg: `"1.23" is not greater than or equal to "2.34"`},
{collection: struct{}{}, msg: `object struct {} is not an ordered collection`},
} {
out := &outputT{buf: bytes.NewBuffer(nil)}
False(t, IsNonIncreasing(out, currCase.collection))
Contains(t, out.buf.String(), currCase.msg)
t.Run(fmt.Sprintf("%#v", currCase.collection), func(t *testing.T) {
out := &outputT{buf: bytes.NewBuffer(nil)}
False(t, IsNonIncreasing(out, currCase.collection))
Contains(t, out.buf.String(), currCase.msg)
})
}
}

Expand Down Expand Up @@ -133,10 +140,13 @@ func TestIsDecreasing(t *testing.T) {
{collection: []uint64{1, 2}, msg: `"1" is not greater than "2"`},
{collection: []float32{1.23, 2.34}, msg: `"1.23" is not greater than "2.34"`},
{collection: []float64{1.23, 2.34}, msg: `"1.23" is not greater than "2.34"`},
{collection: struct{}{}, msg: `object struct {} is not an ordered collection`},
} {
out := &outputT{buf: bytes.NewBuffer(nil)}
False(t, IsDecreasing(out, currCase.collection))
Contains(t, out.buf.String(), currCase.msg)
t.Run(fmt.Sprintf("%#v", currCase.collection), func(t *testing.T) {
out := &outputT{buf: bytes.NewBuffer(nil)}
False(t, IsDecreasing(out, currCase.collection))
Contains(t, out.buf.String(), currCase.msg)
})
}
}

Expand Down Expand Up @@ -178,10 +188,13 @@ func TestIsNonDecreasing(t *testing.T) {
{collection: []uint64{2, 1}, msg: `"2" is not less than or equal to "1"`},
{collection: []float32{2.34, 1.23}, msg: `"2.34" is not less than or equal to "1.23"`},
{collection: []float64{2.34, 1.23}, msg: `"2.34" is not less than or equal to "1.23"`},
{collection: struct{}{}, msg: `object struct {} is not an ordered collection`},
} {
out := &outputT{buf: bytes.NewBuffer(nil)}
False(t, IsNonDecreasing(out, currCase.collection))
Contains(t, out.buf.String(), currCase.msg)
t.Run(fmt.Sprintf("%#v", currCase.collection), func(t *testing.T) {
out := &outputT{buf: bytes.NewBuffer(nil)}
False(t, IsNonDecreasing(out, currCase.collection))
Contains(t, out.buf.String(), currCase.msg)
})
}
}

Expand Down