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.

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

testing: update documentation to be clear about when parallel subtests run #23368

Open
zwass opened this issue Jan 8, 2018 · 6 comments
Open
Labels
Documentation help wanted NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@zwass
Copy link

zwass commented Jan 8, 2018

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

go version go1.9.2 darwin/amd64

Does this issue reproduce with the latest release?

Yes

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

GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/zwass/dev/go"
GORACE=""
GOROOT="/usr/local/Cellar/go/1.9.2/libexec"
GOTOOLDIR="/usr/local/Cellar/go/1.9.2/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/4k/2q3ctn7d5_xb03_zsb3v6lgr0000gn/T/go-build252941481=/tmp/go-build -gno-record-gcc-switches -fno-common"
CXX="clang++"
CGO_ENABLED="1"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"

What did you do?

go test -v with the following code:

package main

import (
	"fmt"
	"sync"
	"testing"
)

func TestHang(t *testing.T) {
	t.Parallel()
	var wg sync.WaitGroup
	wg.Add(1)
	fmt.Println("added wg")
	t.Run("", func(t *testing.T) {
		fmt.Println("deferred done")
		t.Parallel()
		fmt.Println("set parallel")
		wg.Done()
	})

	fmt.Println("waiting")
	wg.Wait()
	fmt.Println("waiting complete")
}

What did you expect to see?

All statements print and test completes

What did you see instead?

Test hangs after "deferred done" and "waiting" are printed.

This is explained by the following comment in the blog post introducing subtests.

A parallel test never runs concurrently with a sequential test and its execution is suspended until its calling test function, that of the parent test, has returned.

But I cannot find this mentioned anywhere in the documentation for the testing package.

It seems like we at least need to update those docs with the appropriate details.

@bradfitz bradfitz changed the title Update testing documentation to be clear about when parallel subtests run testing: update documentation to be clear about when parallel subtests run Jan 8, 2018
@bradfitz bradfitz added this to the Go1.11 milestone Jan 8, 2018
@bradfitz
Copy link
Contributor

bradfitz commented Jan 8, 2018

Note that Go 1.10 has updated wording here:

current: https://golang.org/pkg/testing/#T.Run
go1.10: https://tip.golang.org/pkg/testing/#T.Run

go1.10 says:

Run runs f as a subtest of t called name. It runs f in a separate goroutine and blocks until f returns or calls t.Parallel to become a parallel test. Run reports whether f succeeded (or at least did not fail before calling t.Parallel).

Run may be called simultaneously from multiple goroutines, but all such calls must return before the outer test function for t returns.

That was added in d452986 for #22993.

Is that sufficient?

@bradfitz bradfitz added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Jan 8, 2018
@zwass
Copy link
Author

zwass commented Jan 8, 2018

#22993 also bit me, and led to the code in this example that deadlocks.

I don't think the doc update is sufficient. There is still a critical bit of information missing:

its execution is suspended until its calling test function, that of the parent test, has returned.

@ALTree ALTree added NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. and removed WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. labels Apr 26, 2018
@ianlancetaylor ianlancetaylor added help wanted NeedsFix The path to resolution is known, but the work has not been done. labels Jun 29, 2018
@ianlancetaylor ianlancetaylor modified the milestones: Go1.11, Unplanned Jun 29, 2018
@gopherbot gopherbot removed the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Jun 29, 2018
@RaduBerinde
Copy link
Contributor

Also hit this. There is definitely a lot more detail necessary in the doc for Parallel(). The fact that the test does not proceed until the parent finishes is very important and not at all obvious.

@tommyknows
Copy link

Also, what may not be obvious on first look:

func TestMain(t *testing.T) {
    setup()
    defer teardown()
    for name, tt := range testCases {
        t.Run(name, func(t *testing.T) {
            t.Parallel()
            doStuff()
        })
    }
}

doStuff() will NEVER be executed before the defer teardown() kicks in.
Using sync.waitGroup doesn't help with this either, because the subtests only run once the parent has finished.

Is there any possibility to make this "work as expected"?

@ianlancetaylor
Copy link
Contributor

@tommyknows That is best discussed in a forum, not on this issue. Let's not clutter up the issue. Thanks. https://golang.org/wiki/Questions.

@miparnisari
Copy link

miparnisari commented Sep 12, 2023

@tommyknows put them in a group

func TestMain(t *testing.T) {
    setup()
    defer teardown()
    t.Run("group", func(t *testing.T) {
             for name, tt := range testCases {
                   t.Run(name, func(t *testing.T) {
                       t.Parallel()
                       doStuff()
                   })
           }
    })
}

https://go.dev/play/p/SlW20M84N_g

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation help wanted NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

8 participants