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)

Commits related to this issue

io/fs: document requirement that ReadDir return an unwrapped io.EOF This requirement ensures that ReadDir implementations are as compatible as possible with "*os.File".ReadDir. The testing/fstest pa... — committed to golang/go by neild 3 years ago

Most upvoted comments

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.

mknyszek on Jul 7, 2021

An io.Reader is specified as returning io.EOF at end-of-file, not an error wrapping io.EOF. The fstest package is testing for a conformant io.Reader, so it checks for an io.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 unwrapped io.EOF would apply to them as well.

As @seankhliao pointed out, there is more discussion of io.EOF and error wrapping in #39155.

neild on Jul 7, 2021

Anyone know why io.EOF gets to be an exception to the advice in package errors about checking error identity?

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 of io.Reader made the question moot.

neild on Jul 6, 2021

wraps errors with a stacktrace

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.

peterbourgon on Jul 6, 2021