testing: parallel tests exceed `-test.parallel` flag if `T.Run` is called from multiple goroutines

[bcmills]

Go version

playground

What operating system and processor architecture are you using (go env)?

N/A

What did you do?

https://go.dev/play/p/wKFVA6gmUV9?v=gotip

package main

import (
	"fmt"
	"os"
	"os/exec"
	"sync"
	"sync/atomic"
	"testing"
	"time"
)

func TestConcurrentRunParallelLimit(t *testing.T) {
	if os.Getenv("GO_WANT_HELPER_PROCESS") == "1" {
		var running atomic.Int32
		var wg sync.WaitGroup
		for i := 0; i < 2; i++ {
			wg.Add(1)
			go t.Run(fmt.Sprint(i), func(t *testing.T) {
				t.Cleanup(wg.Done)

				t.Run("inner", func(t *testing.T) {
					t.Parallel()

					if n := running.Add(1); n > 1 {
						t.Errorf("%v parallel tests running; want only 1", n)
					}
					time.Sleep(2 * time.Millisecond)
					running.Add(-1)
				})
			})
		}
		wg.Wait()

		if running.Load() != 0 {
			t.Errorf("running subtest leaked")
		}
		return
	}

	cmd := exec.Command(os.Args[0], "-test.run=^"+t.Name()+"$", "-test.parallel=1", "-test.v")
	cmd.Env = append(cmd.Environ(), "GO_WANT_HELPER_PROCESS=1")
	out, err := cmd.CombinedOutput()
	t.Logf("%v:\n%s", cmd, out)
	if err != nil {
		t.Fatal(err)
	}
}

What did you expect to see?

The total number of running test functions that have called t.Parallel() should not exceed the limit given by the -test.parallel flag.

What did you see instead?

--- FAIL: TestConcurrentRunParallelLimit (0.01s)
    main_test.go:44: /tmp/go-build1661219910/b001/example.test -test.run=^TestConcurrentRunParallelLimit$ -test.parallel=1 -test.v:
        === RUN   TestConcurrentRunParallelLimit
        === RUN   TestConcurrentRunParallelLimit/1
        === RUN   TestConcurrentRunParallelLimit/1/inner
        === PAUSE TestConcurrentRunParallelLimit/1/inner
        === CONT  TestConcurrentRunParallelLimit/1/inner
        === RUN   TestConcurrentRunParallelLimit/0
        === RUN   TestConcurrentRunParallelLimit/0/inner
        === PAUSE TestConcurrentRunParallelLimit/0/inner
        === CONT  TestConcurrentRunParallelLimit/0/inner
            main_test.go:26: 2 parallel tests running; want only 1
        --- FAIL: TestConcurrentRunParallelLimit (0.00s)
            --- FAIL: TestConcurrentRunParallelLimit/0 (0.00s)
                --- FAIL: TestConcurrentRunParallelLimit/0/inner (0.00s)
            --- PASS: TestConcurrentRunParallelLimit/1 (0.00s)
                --- PASS: TestConcurrentRunParallelLimit/1/inner (0.00s)
        FAIL
    main_test.go:46: exit status 1
FAIL
FAIL    example 0.022s
FAIL

In addition, if I modify testing.testContext.release to panic if the running count goes negative, the above test triggers that panic.

[bcmills]

This is really unfortunate. testing.T.Run claims: “Run may be called simultaneously from multiple goroutines, but all such calls must return before the outer test function for t returns.”

The alternate sequential and parallel test cases seem to encode an assumption that a test should not be considered “running in parallel” while it is blocked on a call to t.Run — but, of course, determining whether a call to t.Run in a goroutine blocks its parent from continuing to run is itself quite difficult.