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.