Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
13 changes: 13 additions & 0 deletions assert/assertion_format.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

26 changes: 26 additions & 0 deletions assert/assertion_forward.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

36 changes: 36 additions & 0 deletions assert/assertions.go
Original file line number Diff line number Diff line change
Expand Up @@ -2274,3 +2274,39 @@ func buildErrorChainString(err error, withType bool) string {
}
return chain
}

// NoFieldIsZero asserts that object, which must be a struct or eventually
// reference to one, has no field with a value that is zero.
//
// The assertion is not recursive, meaning it only checks that the fields
Copy link
Collaborator

Choose a reason for hiding this comment

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

The change to use zero value rather than the existing but extremely weird testify "empty" definition is good and resolves my previous comments.

I think this should be recursive though, it doesn't solve the example use case well when it is not.

Copy link
Author

Choose a reason for hiding this comment

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

If you think it is an improvement I can change the implementation (my original implementations were).

I am slightly hesitant about it though due to naming and go conventions. The convention of the reflect package is the fields of a struct do not include fields of nested structs. Taking the recursive approach could lead to confusion.

Copy link
Collaborator

Choose a reason for hiding this comment

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

The use case you provided as an example would fail as soon as any future developer nests a struct, it could fail to store/load and your NoFielIsZero asserion would not protect your test.

If following the conventions of reflect is a hard requirement, and there is no other use case for which this feature works well, then we should not add this feature.

I don't think testify is at all consistent with depth in its assertions. assert.Equal uses reflect.DeepEqual, but assert.ElementsMatch is shallow.

// of the struct (embedded structs are considered fields) are not zero values.
// It does not check the fields of nested or embedded structs.
func NoFieldIsZero(t TestingT, object interface{}, msgAndArgs ...interface{}) bool {
if h, ok := t.(tHelper); ok {
h.Helper()
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Move this block above at the top of the function. This is important for the case of pointer to struct to be reported at the right place.


if reflect.TypeOf(object).Kind() == reflect.Ptr {
return NoFieldIsZero(t, reflect.ValueOf(object).Elem().Interface(), msgAndArgs)
}

objectType := reflect.TypeOf(object)
if objectType.Kind() != reflect.Struct {
return Fail(t, "Input must be a struct or eventually reference one", msgAndArgs...)
}

var emptyFields []string
objectValue := reflect.ValueOf(object)
for i := 0; i < objectType.NumField(); i++ {
field := objectType.Field(i)
if objectValue.Field(i).IsZero() {
emptyFields = append(emptyFields, field.Name)
}
}

if len(emptyFields) > 0 {
return Fail(t, fmt.Sprintf("Object contained fields with zero values: %s", strings.Join(emptyFields, ", ")), msgAndArgs...)
}

return true
}
140 changes: 140 additions & 0 deletions assert/assertions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3664,3 +3664,143 @@ func TestNotErrorAs(t *testing.T) {
})
}
}

func TestNoFieldIsZero(t *testing.T) {
str := "a string"
type Embeddable struct {
StringA string
StringB string
}

tests := []struct {
name string
input interface{}
result bool
resultErrMsg string
}{
{
name: "pass_exported_fields",
input: struct {
Float64 float64
Func func()
Int int
Interface interface{}
Pointer *string
Slice []string
String string
Struct struct{ StringA, StringB string }
}{
Float64: 1.5,
Func: func() {},
Int: 1,
Interface: "interface",
Pointer: &str,
Slice: []string{"a", "b"},
String: "a string",
Struct: struct{ StringA, StringB string }{StringA: "a nested string"},
},
result: true,
},
{
name: "pass_unexported_fields",
input: struct {
aFloat64 float64
aFunc func()
aInt int
aInterface interface{}
aPointer *string
aSlice []string
aString string
aStruct struct{ stringA, stringB string }
}{
aFloat64: 1.5,
aFunc: func() {},
aInt: 1,
aInterface: "interface",
aPointer: &str,
aSlice: []string{"a", "b"},
aString: "a string",
aStruct: struct{ stringA, stringB string }{stringA: "a nested string"},
},
result: true,
},
{
name: "pass_embedded",
input: struct {
Embeddable
}{
Embeddable: Embeddable{StringA: "string"}, // For Embeddable to be non-zero, only one field its fields needs to be non-zero
},
result: true,
},
{
name: "pass_pointer",
input: &struct {
String string
}{
String: "a string",
},
result: true,
},
{
name: "fail_exported_fields",
input: struct {
Float64 float64
Func func()
Int int
Interface interface{}
Pointer *string
Slice []string
String string
Struct struct{ String string }
}{},
result: false,
resultErrMsg: "Object contained fields with zero values: Float64, Func, Int, Interface, Pointer, Slice, String, Struct\n",
},
{
name: "fail_unexported_fields",
input: struct {
aFloat64 float64
aFunc func()
aInt int
aInterface interface{}
aPointer *string
aSlice []string
aString string
aStruct struct{ stringA, stringB string }
}{},
result: false,
resultErrMsg: "Object contained fields with zero values: aFloat64, aFunc, aInt, aInterface, aPointer, aSlice, aString, aStruct\n",
},
{
name: "failure_embedded",
input: struct {
Embeddable
}{},
result: false,
resultErrMsg: "Object contained fields with zero values: Embeddable\n",
},
{
name: "fail_some_fields_non_zero",
input: struct {
StringA string
StringB string
}{StringA: "not_empty"},
result: false,
resultErrMsg: "Object contained fields with zero values: StringB\n",
},
{
name: "fail_wrong_type",
input: "a string is not a struct",
result: false,
resultErrMsg: "Input must be a struct or eventually reference one\n",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
mockT := new(captureTestingT)
result := NoFieldIsZero(mockT, tt.input)
mockT.checkResultAndErrMsg(t, tt.result, result, tt.resultErrMsg)
})
}
}
32 changes: 32 additions & 0 deletions require/require.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

26 changes: 26 additions & 0 deletions require/require_forward.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.