rogpeppe / go-internal

Selected Go-internal packages factored out from the standard library
BSD 3-Clause "New" or "Revised" License
875 stars 76 forks source link

testscript: [net] is misleading, and a bit useless for most developers #75

Open mvdan opened 5 years ago

mvdan commented 5 years ago

The godoc reads:

  • [net] for whether the external network can be used

I initially thought - great! It must do some quick check to see if there's a network connection, and if the internet can be reached. I wrote many of my test scripts with lines like [!net] skip 'requires network access'.

Imagine my confusion when I peeked at the implementation:

func HasExternalNetwork() bool {
        return !testing.Short() && runtime.GOOS != "nacl" && runtime.GOOS != "js"
}

This makes sense for the Go project's trybots, but not much else. I think the docs should either discourage the use of [net], or implement one that actually does something useful.

I can think of multiple implementations:

$ ip route get 8.8.8.8
8.8.8.8 via 192.168.1.1 dev wlp2s0 src 192.168.1.111 uid 1000
    cache

This best-effort method could always return true on platforms that don't yet implement a decent check.

mvdan commented 5 years ago

I personally think that this should be a best-effort immediate check. The check should be "is the machine connected to the external network", and not "does the internet work right now". If a laptop is connected to the internet but the internet connection is slow or flaky, the test should fail, not get skipped. Otherwise the user could very easily be misled or confused.

I think [net] should only skip tests on machines that are not connected to the internet on purpose. Like an offline test machine, or a laptop on an airplane, and so on.

mvdan commented 5 years ago

I forgot to mention - testing.Short should make [net] fail immediately, just like the current implementation.