Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

testing: timeout docs for "go test -h" are ambiguous #20090

Closed
kevinburke opened this issue Apr 23, 2017 · 4 comments

Comments

Projects
None yet
5 participants
@kevinburke
Copy link
Contributor

commented Apr 23, 2017

"go test -h" says this about timeouts:

	-timeout t
	    If a test runs longer than t, panic.
	    The default is 10 minutes (10m).

"A test" is ambiguous in this context. I took it to mean "A single test", that is, "Each test has a timeout of t". Test runners in other languages usually default to this behavior for tests. However, the flag output indicates the timeout covers the entire test run, not individual tests.

	timeout              = flag.Duration("test.timeout", 0, "fail test binary execution after duration `d` (0 means unlimited)")

This also tripped up at least one person recently in the #golang-newbies Slack channel. It would be good if the output was clear that the timeout covers the entire test run, not individual tests.

@josharian

This comment has been minimized.

Copy link
Contributor

commented Apr 23, 2017

Dup of #20025 and #18490?

@josharian

This comment has been minimized.

Copy link
Contributor

commented Apr 23, 2017

Oh no, not a dup. Sorry.

@GlenDC

This comment has been minimized.

Copy link

commented Jun 8, 2017

I just faced some confusing related to this ambiguous text message myself.

My colleague thought it applied to the entire test run. I thought he was wrong, as the usage message clearly says a test. He did however a test with the following setup:

package main

import (
	"testing"
	"time"
)

func Test1(t *testing.T) {
	time.Sleep(3 * time.Second)
}

func Test2(t *testing.T) {
	time.Sleep(3 * time.Second)
}

Which you can then run with go test -timeout=5s, which will indeed fail. OK this made me understand that clearly the timeout is not applied on an individual test level. But it still didn't define if it's applied on the entire test run, or on a package level. So I did some testing myself, inspired by my colleague, using the following setup:

// foo_test.go

package main

import (
	"testing"
	"time"
)

func Test1(t *testing.T) {
	time.Sleep(3 * time.Second)
}
// bar/bar_test.go

package bar

import (
	"testing"
	"time"
)

func Test1(t *testing.T) {
	time.Sleep(3 * time.Second)
}

And then run: go test -timeout=4s ./...

You'll see that this DOES run. So clearly the timeout value IS on a per package level. It is NOT applied on the entire test run, and it is not applied per individual level.

Which is why I propose that we change the message to something like:

	-timeout t
	    If a package runs longer than t, panic.
	    The default is 10 minutes (10m).

Or anything similar that mentions the word "package", as "a test" is really way too ambiguous, especially when left on its own.

@gopherbot

This comment has been minimized.

Copy link

commented Jun 15, 2017

CL https://golang.org/cl/45816 mentions this issue.

@gopherbot gopherbot closed this in e7c650b Jun 15, 2017

@golang golang locked and limited conversation to collaborators Jun 15, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.