testing: fail if T.Setenv is called via T.Run in a parallel test

The existing implementation can call to T.Setenv in T.Run even after
calling to T.Parallel, so I changed it to cause a panic in that case.

Fixes #55128

Change-Id: Ib89d998ff56f00f96a5ca218af071bd35fdae53a
Reviewed-on: https://go-review.googlesource.com/c/go/+/431101
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Bryan Mills <bcmills@google.com>

src/testing/testing.go

@@ -547,6 +547,7 @@ type common struct {
 	hasSub     atomic.Bool    // whether there are sub-benchmarks.
 	raceErrors int            // Number of races detected during test.
 	runner     string         // Function name of tRunner running the test.
+	isParallel bool           // Whether the test is parallel.
 
 	parent   *common
 	level    int       // Nesting depth of test or benchmark.
@@ -823,9 +824,8 @@ var _ TB = (*B)(nil)
 // may be called simultaneously from multiple goroutines.
 type T struct {
 	common
-	isParallel bool
-	isEnvSet   bool
-	context    *testContext // For running tests and subtests.
+	isEnvSet bool
+	context  *testContext // For running tests and subtests.
 }
 
 func (c *common) private() {}
@@ -1326,7 +1326,19 @@ func (t *T) Parallel() {
 //
 // This cannot be used in parallel tests.
 func (t *T) Setenv(key, value string) {
-	if t.isParallel {
+	// Non-parallel subtests that have parallel ancestors may still
+	// run in parallel with other tests: they are only non-parallel
+	// with respect to the other subtests of the same parent.
+	// Since SetEnv affects the whole process, we need to disallow it
+	// if the current test or any parent is parallel.
+	isParallel := false
+	for c := &t.common; c != nil; c = c.parent {
+		if c.isParallel {
+			isParallel = true
+			break
+		}
+	}
+	if isParallel {
 		panic("testing: t.Setenv called after t.Parallel; cannot set environment variables in parallel tests")
 	}
 

src/testing/testing_test.go

@@ -200,3 +200,35 @@ func TestSetenvWithParallelBeforeSetenv(t *testing.T) {
 
 	t.Setenv("GO_TEST_KEY_1", "value")
 }
+
+func TestSetenvWithParallelParentBeforeSetenv(t *testing.T) {
+	t.Parallel()
+
+	t.Run("child", func(t *testing.T) {
+		defer func() {
+			want := "testing: t.Setenv called after t.Parallel; cannot set environment variables in parallel tests"
+			if got := recover(); got != want {
+				t.Fatalf("expected panic; got %#v want %q", got, want)
+			}
+		}()
+
+		t.Setenv("GO_TEST_KEY_1", "value")
+	})
+}
+
+func TestSetenvWithParallelGrandParentBeforeSetenv(t *testing.T) {
+	t.Parallel()
+
+	t.Run("child", func(t *testing.T) {
+		t.Run("grand-child", func(t *testing.T) {
+			defer func() {
+				want := "testing: t.Setenv called after t.Parallel; cannot set environment variables in parallel tests"
+				if got := recover(); got != want {
+					t.Fatalf("expected panic; got %#v want %q", got, want)
+				}
+			}()
+
+			t.Setenv("GO_TEST_KEY_1", "value")
+		})
+	})
+}