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: Verbose test output has cpu number appended to test name because of GOMAXPROCS default change in Go 1.5; the number lacks an explanation/source/unit #11200

Closed
michael-schaller opened this issue Jun 13, 2015 · 10 comments

Comments

Projects
None yet
7 participants
@michael-schaller
Copy link
Contributor

commented Jun 13, 2015

With Go 1.5 the default GOMAXPROCS value has changed:
https://docs.google.com/document/d/1At2Ls5_fhJQ59kDK2DFVhFu3g5mATSXqqV5QrxinasI/preview?sle=true

With Go 1.4.2 tests always ran with GOMAXPROCS=1 unless someone used the -cpu command line parameter. This resulted in stable verbose test output like this one:

$ go test -v
=== RUN   TestExample
--- PASS: TestExample (0.00s)
...

With Go 1.5 the verbose test output has a number appended to the test name:

$ go test -v
=== RUN   TestExample-4
--- PASS: TestExample-4 (0.00s)
...

Some users could wonder where this appended number is coming from...

Explanation:
The additional -4 in my verbose test output is because GOMAXPROCS is set to 4 on my quad-core machine. On a different machine with a different CPU count GOMAXPROCS would be different and hence the test name would be different.

Possible fix:
We could change the test name to something more understandable. I would prefer TestExample-4Procs over TestExample-4 to make clear that the test ran with GOMAXPROCS=4. I'm happy to send a patch.

Source code location:
https://github.com/golang/go/blob/fddc3ca11c38b063cb24e14d99cc023e746d0e20/src/testing/testing.go#L548-551

@josharian josharian added this to the Go1.5 milestone Jun 13, 2015

@robpike

This comment has been minimized.

Copy link
Contributor

commented Jun 13, 2015

I am unhappy about this change, too, but am unsure what to do about it.

@michael-schaller

This comment has been minimized.

Copy link
Contributor Author

commented Jun 13, 2015

Rob, do you see any harm in changing "%s-%d" to "%s-%dProcs" or "%s-%dCPUs" here?

testName = fmt.Sprintf("%s-%d", tests[i].Name, procs)

That would at least make it a bit clearer where that number is coming from...

@cespare

This comment has been minimized.

Copy link
Contributor

commented Jun 13, 2015

Can the -N part not be shown unless the user gives -cpu explicitly, or maybe even not unless the user provides more than one value for -cpu?

@michael-schaller

This comment has been minimized.

Copy link
Contributor Author

commented Jun 13, 2015

Theoretically yes, but in case a test fails on one machine and doesn't on another it can take ages to nail it down to a different default GOMAXPROCS value. So I personally would keep -N but would add a unit like Procs or CPUs. I would even go as far as to always show -N to be explicit.

@michael-schaller

This comment has been minimized.

Copy link
Contributor Author

commented Jun 14, 2015

How about I send a change? Maybe that makes discussing this issue easier... ;-)

@robpike

This comment has been minimized.

Copy link
Contributor

commented Jun 14, 2015

Please don't yet.

@adg

This comment has been minimized.

Copy link
Contributor

commented Jun 14, 2015

Perhaps the GOMAXPROCS value could be in a parenthetical beside the test
name. There are other things that may affect the running of the test (goos,
goarch, build tags, etc) should we also display those in the verbose test
output?

It might make remote debugging (via mail or otherwise) easier.

On 14 June 2015 at 13:24, Rob Pike notifications@github.com wrote:

Please don't yet.


Reply to this email directly or view it on GitHub
#11200 (comment).

@michael-schaller

This comment has been minimized.

Copy link
Contributor Author

commented Jun 15, 2015

@robpike Alrighty. My intention for this bug was to fix this Go 1.5 specific issue before Go 1.5 is released. So I only had a very small change in mind...

@adg Great idea but shouldn't this be a separate bug? IMHO details like GOOS, GOARCH, build tags, ... can be printed in the beginning of the verbose test output as these don't change during the whole test run. GOMAXPROCS is different as -cpu can receive a list of values.

@rsc

This comment has been minimized.

Copy link
Contributor

commented Jun 15, 2015

We should probably drop the suffix from the test names.
The number of CPUs involved in a test rarely matters,
and it's distracting.

We should keep the suffix in benchmark names.
The number of CPUs involved in a benchmark nearly always matters.

We should not change the suffix for benchmark names.
It has been this way since Go 1.0, and there are almost certainly
programs that understand the meaning. It's also short,
which matters when benchmarks are presented in contexts
like benchstat/benchcmp and so on.

@robpike robpike closed this in 1f9026c Jun 15, 2015

@michael-schaller

This comment has been minimized.

Copy link
Contributor Author

commented Jun 15, 2015

Thanks for the fix, Rob. I just hope that broken tests because of the GOMAXPROCS default change, like #11157, aren't too common.

@golang golang locked and limited conversation to collaborators Jun 25, 2016

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.