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

Jump to bottom

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
Closed

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

michael-schaller opened this issue Jun 13, 2015 · 10 comments
Labels
FrozenDueToAge
Milestone
Go1.5

Comments

@michael-schaller
Copy link
Contributor

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
Copy link
Contributor

robpike commented Jun 13, 2015

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

@michael-schaller
Copy link
Contributor Author

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

go/src/testing/testing.go

Line 550 in fddc3ca

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
Copy link
Contributor

cespare 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
Copy link
Contributor Author

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
Copy link
Contributor Author

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

@robpike
Copy link
Contributor

robpike commented Jun 14, 2015

Please don't yet.

@adg
Copy link
Contributor

adg 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).

@alexbrainman alexbrainman mentioned this issue Jun 14, 2015
Closed
@michael-schaller
Copy link
Contributor Author

@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
Copy link
Contributor

rsc 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.

@michael-schaller
Copy link
Contributor Author

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

@ianlancetaylor ianlancetaylor mentioned this issue Aug 21, 2015
Closed
@golang golang locked and limited conversation to collaborators Jun 25, 2016
@rsc rsc unassigned robpike Jun 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
Go1.5
Development

No branches or pull requests
7 participants
@josharian @rsc @cespare @michael-schaller @robpike @adg @gopherbot