go: testing/fstest: should use errors.Is instead of equality
What version of Go are you using (go version
)?
$ go version go version go1.16.5 linux/amd64
Does this issue reproduce with the latest release?
Yes
What did you do?
I wrote an fs.FS
implementation that wraps errors with a stacktrace. Therefore I don’t return io.EOF
but errors.WithStack(io.EOF)
.
fstest.TestFS()
fails at this line:
https://github.com/golang/go/blob/912f0750472dd4f674b69ca1616bfaf377af1805/src/testing/fstest/testfs.go#L186
because it uses !=
instead of errors.Is()
.
I think all error checks in that test should instead use errors.Is()
to check whether the right error is returned.
What did you expect to see?
No test failure.
What did you see instead?
Test failure.
About this issue
- Original URL
- State: closed
- Created 3 years ago
- Comments: 15 (7 by maintainers)
Based on the conversation, it doesn’t look like we’re going to be able to make this change because of
io.Reader
’s contract. There’s too much code out there that relies on== io.EOF
, so I’m inclined to close this issue. @zepatrik Please comment if you feel it should be reopened.An
io.Reader
is specified as returningio.EOF
at end-of-file, not an error wrappingio.EOF
. Thefstest
package is testing for a conformantio.Reader
, so it checks for anio.EOF
. It reporting an error in this circumstance is an example of it functioning as intended: It has correctly detected an incorrect implementation.I’m not certain if there are other interfaces in the standard library specified as returning a sentinel error. If there are, the rationale for
Read
returning an unwrappedio.EOF
would apply to them as well.As @seankhliao pointed out, there is more discussion of io.EOF and error wrapping in #39155.
Because the
io.Reader
interface was well-defined and widely used (to put it mildly) by the time error wrapping was introduced.Also, checks for
io.EOF
are likely to occur in a performance-critical hot path, which might have been sufficient reason to require an unwrapped sentinel anyway. Or possibly not; the existing definition ofio.Reader
made the question moot.There is no reason to do this. Callers who want a stack trace when they receive an error can easily generate one themselves. Callers who don’t want a stack trace should not be required to pay the costs of generating one.
Anyone know why io.EOF gets to be an exception to the advice in package errors about checking error identity?