Incorrect require.Error() docs

[andig]

Description

The error docs read:

// Error asserts that a function returned an error (i.e. not `nil`).
//
//	  actualObj, err := SomeFunction()
//	  if assert.Error(t, err) {
//		   assert.Equal(t, expectedError, err)
//	  }

This indicates that Error() has a bool return value which it hasn't. There is no way to check expectedError like this.

Expected behavior

Error docs should show actual usage. It would be nice if Error() actually allowed for checking the specific error value.

[hendrywiranto]

hello @andig

is this the Error() that you meant? https://github.com/stretchr/testify/blob/master/assert/assertions.go#L1596
Error has a bool return value, please do elaborate more if I misunderstood your meanings.

[andig]

Thank you for the quick reply. Seems I was actually looking at require.Error(). Thinking about it, the if case doesn't really make sense for require, so only the comment is off?

// Error asserts that a function returned an error (i.e. not `nil`).
//
//	  actualObj, err := SomeFunction()
//	  if assert.Error(t, err) {
//		   assert.Equal(t, expectedError, err)
//	  }
func Error(t TestingT, err error, msgAndArgs ...interface{}) {
	if h, ok := t.(tHelper); ok {
		h.Helper()
	}
	if assert.Error(t, err, msgAndArgs...) {
		return
	}
	t.FailNow()
}

[hendrywiranto]

yea I think so, but I checked the docs for require, they all used assert as example in the comments, I don't know if it's intended or not
I think if we want to change we need to adjust all the comments on the package.

[brackendawson]

The doc-string for Error needs to be made into one which works for both the assert and require packages. Especially after #1610 is merged.

I'd suggest simply removing the if statement and the Equal call from the example. Especially since EqualError exists.

[architagr]

Hi @brackendawson,

I can fix this doc-string for assert and require. To reconfirm we want to update the doc not to have an example that uses if block and then Equal function

// Error asserts that a function returned an error (i.e. not `nil`).
//
//	  actualObj, err := SomeFunction()
//	  assert.Error(t, err) 
func Error(t TestingT, err error, msgAndArgs ...interface{}) bool {
	if err == nil {
		if h, ok := t.(tHelper); ok {
			h.Helper()
		}
		return Fail(t, "An error is expected but got nil.", msgAndArgs...)
	}

	return true
}

and similar change in https://github.com/stretchr/testify/blob/master/require/require.go#L280

[brackendawson]

Yes. The change in require is automatic, just run go generate ./....

[architagr]

Yes. The change in require is automatic, just run go generate ./....

@brackendawson, I have created pull request #1675, can you please help me with review on this.