testing/iotest: ErrTimeout fails os.IsTimeout

[dolmen]

What version of Go are you using (go version)?

$ go version
1.13.7

Does this issue reproduce with the latest release?

Yes.

https://play.golang.org/p/T4bwPszSzM0

What did you do?

package main

import (
	"fmt"
	"os"
	"testing/iotest"
)

func main() {
	fmt.Println(os.IsTimeout(iotest.ErrTimeout))
}

What did you expect to see?

true

What did you see instead?

false

[changkun]

This result is consistent since the day that @ianlancetaylor introduces os.IsTimeout in Go 1.10.
However iotest.ErrTimeout lives way longer than io.IsTimeout.

The question is whether the Go team will recognize this as expected behavior with more documentation or a bug that should be fixed.

[dolmen]

I think that iotest.TimeoutReader is just useless for writing I/O tests if iotest.ErrTimeout isn't matched by os.IsTimeout.

Making it match is just switching ErrTimeout from being built from errors.New to using a specific type that has a Timeout method.

type errTimeout struct{}

func (errTimeout) Error() string {
    return "timeout"
}

func (errTimeout) Timeout() bool {
    return true
}

var ErrTimeout error = errTimeout{}

I can provide this as a PR (with added documentation).

[toothrot]

Also /cc-ing @mpvl @josharian as owners of testing

Just for some context, os.ErrTimeout was removed in #33411. There's also some discussion that seems related in #30322.

[rsc]

os.IsTimeout does what it does and won't be changing. The comments need to be updated to mark it as deprecated - use errors.Is instead. Fixing the doc comments is #41122 .