testing: allow marking subtest function as Helper

[larhun]

Please answer these questions before submitting your issue. Thanks!

What version of Go are you using (go version)?

go version go1.10 windows/amd64

What did you do?

Run the following program with "go test":

helper.go:

package helper

helper_test.go:

package helper

import (
	"testing"
)

func TestRun(t *testing.T) {
	run(t, []string{"a", "b", "c"}) // this is line 8
}

func run(t *testing.T, values []string) {
	t.Helper()
	for _, value := range values {
		t.Run("", func(t *testing.T) {
			t.Helper()
			t.Fatal(value) // this is line 16
		})
	}
}

What did you expect to see?

--- FAIL: TestRun (0.00s)
    --- FAIL: TestRun/#00 (0.00s)
    	...\subtest_test.go:8: a
    ...

What did you see instead?

--- FAIL: TestRun (0.00s)
    --- FAIL: TestRun/#00 (0.00s)
    	...\subtest_test.go:16: a
    ...

The reported error line is 16 instead of 8, where the test data are defined. This makes it difficult to use the Run method to define a generic test on a list of cases. I would expect the Run method to be marked as a test helper by default. If the subtesting function is marked as helper, the reported line should be the line of the function that calls the Run method (unless skipped by the Helper mark).

[ALTree]

Dup of #23249

[larhun]

Not a duplicate. #23249 deals with using Helper within a closure over a testing.TB value. This issue deals with the constraint stated by the documentation of the Run method:

Helper has no effect if it is called directly from a TestXxx/BenchmarkXxx function or a subtest/sub-benchmark function.

[ALTree]

It seems to me that the concrete issue you are reporting in this thread is the bad lines in the failure messages

The reported error line is 16 instead of 8, where the test data are defined. This makes it difficult to use the Run method [...]

If the issue reported in the thread I linked above (Helper's highest function call's information should be maintained for any subsequent/nested calls) is fixed in a general way, wouldn't this also fix the problem you are reporting here?

[larhun]

Both issues have similar errors, bad lines in the failure messages. However the reasons are very different. Therefore a solution to #23249 may not be a solution to my issue, that basically requires removing the effect of the following lines in the Helper documentation (wrongly reported as Run documentation in my last comment):

Helper has no effect if it is called directly from a TestXxx/BenchmarkXxx function or a subtest/sub-benchmark function.

[ALTree]

I see. Leaving this open, then.

[rsc]

It seems like this should work. The intent of the code is clear, even though it will require a special case at the intersection of subtests and Helper.

/cc @mpvl

[gopherbot]

Change https://golang.org/cl/108658 mentions this issue: testing: allow marking subtest and subbenchmark functions as Helpers

[bradfitz]

Marking release-blocker as there's a pending CL that needs review. /cc @rsc @mpvl