Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Open
bcmills opened this issue Nov 30, 2023 · 1 comment
Open

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

bcmills opened this issue Nov 30, 2023 · 1 comment
Labels
NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Backlog

Comments

@bcmills
Copy link
Contributor

bcmills commented Nov 30, 2023 •
edited

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 bcmills self-assigned this Nov 30, 2023
@bcmills bcmills added the NeedsFix The path to resolution is known, but the work has not been done. label Nov 30, 2023
@bcmills bcmills added this to the Go1.22 milestone Nov 30, 2023
@bcmills
Copy link
Contributor Author

bcmills commented Nov 30, 2023

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.

@bcmills bcmills modified the milestones: Go1.22, Go1.23 Dec 1, 2023
@bcmills bcmills removed their assignment Mar 11, 2024
@bcmills bcmills modified the milestones: Go1.23, Backlog Mar 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
1 participant
@bcmills