Change https://go.dev/cl/494498 mentions this issue: cmd/go/internal/test: add -label flag

As far as I am aware, the only use-case we have for this at the moment is in cmd/dist. But cmd/dist runs cmd/go as a subprocess — couldn't it parse and transform the JSON output to add the labels for each subprocess itself?

One advantage of doing it in cmd/go is that it can easily transform the text format as well. See, for example, CL 494407.

Before this change, dist test prints inscrutable blocks like:

##### Testing cgo
...
ok  	cmd/cgo/internal/test	0.889s
ok  	cmd/cgo/internal/test	0.921s
ok  	cmd/cgo/internal/test	0.968s
ok  	cmd/cgo/internal/testtls	0.013s
ok  	cmd/cgo/internal/testtls	0.008s
ok  	cmd/cgo/internal/testtls	0.009s

With this change, it prints:

##### Testing cgo
...
ok  	cmd/cgo/internal/test:auto	0.847s
ok  	cmd/cgo/internal/test:internal	0.718s
ok  	cmd/cgo/internal/test:external-g0	0.720s
ok  	cmd/cgo/internal/testtls:auto	0.012s
ok  	cmd/cgo/internal/testtls:external	0.004s
ok  	cmd/cgo/internal/testtls:external-static	0.003s

These package names are also dist's internal test names now, so if you want to reproduce, you can do, e.g., dist test -run cmd/cgo/internal/test:auto. Currently it's reliably extremely frustrating to figure out how to get dist to re-run the right test in these variant cases.

(This doesn't do much for your argument that dist may be the only user of this.)

A possible alternative proposal for cmd/go that would make this flag unnecessary would be to add first-class support for test variants. That is, if there was a way for any Go package to declare that it should be tested multiple times with alternative different settings. To illustrate the point:

As a possible line directive:

//go:testvariant <variant name> <alternative build or test settings for the test>

Or registered at runtime:

package p_test

import "testing"

func init() {
	testing.RegisterTestAlternative("<variant name>", [...])
}

func TestA(t *testing .T) { [...] }

Such that by default go test <import path pattern> would test all alternatives, but presumably there'd be some way to control that, e.g., go test <import path pattern>:<variant pattern> or via new flags. This is meant only to illustrate the point, the proposal would need to think this through more and evaluate the effects on the ecosystem.

If something like that were available, the special case registration done in dist test for some Go packages could be moved over into the packages themselves, and this flag wouldn't be needed.

Of course this -label (or -testlabel as written in proposal title) flag for go test has the benefit of being much smaller compared to the aforementioned alternative, and solves the problem for dist/test better than dist could itself (as @aclements points out above). In addition, it doesn't preclude that larger proposal from possibly happening in the future; the flag can simply stay around or become deprecated if it stops being needed.

Perhaps some large Go projects that currently have a Makefile with multiple targets of differing go test invocations will find use for it too.

For the proposed flag and behavior, this seems very reasonable and cleaner than dist doing the JSON event post-processing itself, at the cost of one additional fairly unobtrusive flag added to go test.

It might help to initially constrain the permitted characters (and possibly max length) in the label value. It'll be easy to expand the allowed set if allowing more characters turns out to be worthwhile, not so easy to take it back otherwise.

@aclements, I think that for the inscrutable dist blocks I would rather see the actual go test command anyway, so I don't have to go digging through the cmd/dist source code to figure out how to reproduce the bug using go test.

Maybe something more like:

##### Testing cgo
...
# go test cmd/cgo/internal/test cmd/cgo/internal/testtls -linkmode=auto
ok  	cmd/cgo/internal/test	0.847s
ok  	cmd/cgo/internal/testtls	0.012s
# go test cmd/cgo/internal/test cmd/cgo/internal/testtls -linkmode=external
ok  	cmd/cgo/internal/test	0.720s
ok  	cmd/cgo/internal/testtls	0.004s
# go test cmd/cgo/internal/test -linkmode=internal
ok  	cmd/cgo/internal/test	0.718s
# CGO_CFLAGS='-g0 -fdiagnostics-color' go test cmd/cgo/internal/test -linkmode=external
ok  	cmd/cgo/internal/test	0.720s

@dmitshur

A possible alternative proposal for cmd/go that would make this flag unnecessary would be to add first-class support for test variants.

See previously #39005.

@bcmills, that's pretty reasonable for the text output. We still need some way to distinguish the test variants in the JSON output, preferably in a way that existing CI systems can immediately take advantage of. Were you thinking we'd still do the pkg:variant renaming in the JSON output, but just show the command for the text output? (If we have the logic to print the command, maybe we also inject some Action:"output" record to the JSON with the command...)

For some context, originally I'd proposed test label support in cmd/go to @rsc as a dist-specific back door, largely because we control both cmd/dist and cmd/go and it's much less annoying to implement this in cmd/go and also much easier to test.

Were you thinking we'd still do the pkg:variant renaming in the JSON output, but just show the command for the text output?

I hadn't really though that far into it, but that sounds like a pretty reasonable approach to me. 😅

Retracted for now; we'll do what we need in cmd/dist and get some experience.

For reference, CL 494958 does what we need in dist using a stdout filter.

This proposal has been declined as retracted.
— rsc for the proposal review group