suite.Run() triggers data race errors with parallel tests #1139

bstpierre · 2021-12-28T02:44:29Z

Running suite.Run() when test cases have called suite.T().Parallel() triggers a data race warning when using go test -race.

==> go.mod <==

module testify-race

go 1.17

require (
        github.com/davecgh/go-spew v1.1.0 // indirect
        github.com/pmezard/go-difflib v1.0.0 // indirect
        github.com/stretchr/objx v0.1.0 // indirect
        github.com/stretchr/testify v1.7.0 // indirect
        gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c // indirect
)

==> main.go <==

package main

import "fmt"

func add(x, y int) int {
        return x + y
}

func main() {
        fmt.Println("hello")
}

==> main_test.go <==

package main

import (
        "testing"

        "github.com/stretchr/testify/suite"
)

type MainSuite struct {
        suite.Suite
}

func (s *MainSuite) TestA() {
        s.T().Parallel()
        for x := 0; x < 100; x++ {
                s.Equal(x, add(x, 0))
        }
}

func (s *MainSuite) TestB() {
        s.T().Parallel()
        for x := 0; x < 100; x++ {
                s.Equal(x+1, add(x, 1))
        }
}

func Test_Main(t *testing.T) {
        suite.Run(t, new(MainSuite))
}

Running the test with the race detector:

$ go test -race .
==================                                                                                              
WARNING: DATA RACE        
Read at 0x00c0000b6138 by goroutine 9:            
  testify-race.(*MainSuite).TestB()
      /tmp/testify-bugreport/main_test.go:23 +0x73                                                              
  runtime.call16()                                                                                              
      /home/brian/sdk/go1.17.2/src/runtime/asm_amd64.s:625 +0x48                                     
  reflect.Value.Call()             
      /home/brian/sdk/go1.17.2/src/reflect/value.go:339 +0xd7
  github.com/stretchr/testify/suite.Run.func1()
      /home/brian/go/pkg/mod/github.com/stretchr/testify@v1.7.0/suite/suite.go:158 +0x71c
  testing.tRunner()   
      /home/brian/sdk/go1.17.2/src/testing/testing.go:1259 +0x22f
  testing.(*T).Run·dwrap·21()                  
      /home/brian/sdk/go1.17.2/src/testing/testing.go:1306 +0x47                                                                                                                                                                 

Previous write at 0x00c0000b6138 by goroutine 8:
  github.com/stretchr/testify/suite.(*Suite).SetT()
      /home/brian/go/pkg/mod/github.com/stretchr/testify@v1.7.0/suite/suite.go:36 +0xc4
  testify-race.(*MainSuite).SetT()
      <autogenerated>:1 +0x44
  github.com/stretchr/testify/suite.Run.func1.1()
      /home/brian/go/pkg/mod/github.com/stretchr/testify@v1.7.0/suite/suite.go:144 +0x2f4
  github.com/stretchr/testify/suite.Run.func1()
      /home/brian/go/pkg/mod/github.com/stretchr/testify@v1.7.0/suite/suite.go:159 +0x71f
  testing.tRunner()
      /home/brian/sdk/go1.17.2/src/testing/testing.go:1259 +0x22f
  testing.(*T).Run·dwrap·21()
      /home/brian/sdk/go1.17.2/src/testing/testing.go:1306 +0x47

Goroutine 9 (running) created at:
  testing.(*T).Run()
      /home/brian/sdk/go1.17.2/src/testing/testing.go:1306 +0x726
  github.com/stretchr/testify/suite.runTests()
      /home/brian/go/pkg/mod/github.com/stretchr/testify@v1.7.0/suite/suite.go:203 +0x18f
  github.com/stretchr/testify/suite.Run()
      /home/brian/go/pkg/mod/github.com/stretchr/testify@v1.7.0/suite/suite.go:176 +0x994
  testify-race.Test_Main()
      /tmp/testify-bugreport/main_test.go:28 +0x44
  testing.tRunner()
      /home/brian/sdk/go1.17.2/src/testing/testing.go:1259 +0x22f
  testing.(*T).Run·dwrap·21()
      /home/brian/sdk/go1.17.2/src/testing/testing.go:1306 +0x47

Goroutine 8 (finished) created at:
  testing.(*T).Run()
      /home/brian/sdk/go1.17.2/src/testing/testing.go:1306 +0x726
  github.com/stretchr/testify/suite.runTests()
      /home/brian/go/pkg/mod/github.com/stretchr/testify@v1.7.0/suite/suite.go:203 +0x18f
  github.com/stretchr/testify/suite.Run()
      /home/brian/go/pkg/mod/github.com/stretchr/testify@v1.7.0/suite/suite.go:176 +0x994
  testify-race.Test_Main()
      /tmp/testify-bugreport/main_test.go:28 +0x44
  testing.tRunner()
      /home/brian/sdk/go1.17.2/src/testing/testing.go:1259 +0x22f
  testing.(*T).Run·dwrap·21()
      /home/brian/sdk/go1.17.2/src/testing/testing.go:1306 +0x47
==================
==================
WARNING: DATA RACE
Read at 0x00c000233a40 by goroutine 9:
  github.com/stretchr/testify/assert.(*Assertions).Equal()
      /home/brian/go/pkg/mod/github.com/stretchr/testify@v1.7.0/assert/assertion_forward.go:128 +0x64
  testify-race.(*MainSuite).TestB()
      /tmp/testify-bugreport/main_test.go:23 +0x9d
  runtime.call16()
      /home/brian/sdk/go1.17.2/src/runtime/asm_amd64.s:625 +0x48
  reflect.Value.Call()
      /home/brian/sdk/go1.17.2/src/reflect/value.go:339 +0xd7
  github.com/stretchr/testify/suite.Run.func1()
      /home/brian/go/pkg/mod/github.com/stretchr/testify@v1.7.0/suite/suite.go:158 +0x71c
  testing.tRunner()
      /home/brian/sdk/go1.17.2/src/testing/testing.go:1259 +0x22f
  testing.(*T).Run·dwrap·21()
      /home/brian/sdk/go1.17.2/src/testing/testing.go:1306 +0x47

Previous write at 0x00c000233a40 by goroutine 8:
  github.com/stretchr/testify/assert.New()
      /home/brian/go/pkg/mod/github.com/stretchr/testify@v1.7.0/assert/forward_assertions.go:12 +0x84
  github.com/stretchr/testify/suite.(*Suite).SetT()
      /home/brian/go/pkg/mod/github.com/stretchr/testify@v1.7.0/suite/suite.go:36 +0xb6
  testify-race.(*MainSuite).SetT()
      <autogenerated>:1 +0x44
  github.com/stretchr/testify/suite.Run.func1.1()
      /home/brian/go/pkg/mod/github.com/stretchr/testify@v1.7.0/suite/suite.go:144 +0x2f4
  github.com/stretchr/testify/suite.Run.func1()
      /home/brian/go/pkg/mod/github.com/stretchr/testify@v1.7.0/suite/suite.go:159 +0x71f
  testing.tRunner()
      /home/brian/sdk/go1.17.2/src/testing/testing.go:1259 +0x22f
  testing.(*T).Run·dwrap·21()
      /home/brian/sdk/go1.17.2/src/testing/testing.go:1306 +0x47

Goroutine 9 (running) created at:
  testing.(*T).Run()
      /home/brian/sdk/go1.17.2/src/testing/testing.go:1306 +0x726
  github.com/stretchr/testify/suite.runTests()
      /home/brian/go/pkg/mod/github.com/stretchr/testify@v1.7.0/suite/suite.go:203 +0x18f
  github.com/stretchr/testify/suite.Run()
      /home/brian/go/pkg/mod/github.com/stretchr/testify@v1.7.0/suite/suite.go:176 +0x994
  testify-race.Test_Main()
      /tmp/testify-bugreport/main_test.go:28 +0x44
  testing.tRunner()
      /home/brian/sdk/go1.17.2/src/testing/testing.go:1259 +0x22f
  testing.(*T).Run·dwrap·21()
      /home/brian/sdk/go1.17.2/src/testing/testing.go:1306 +0x47

Goroutine 8 (finished) created at:
  testing.(*T).Run()
      /home/brian/sdk/go1.17.2/src/testing/testing.go:1306 +0x726
  github.com/stretchr/testify/suite.runTests()
      /home/brian/go/pkg/mod/github.com/stretchr/testify@v1.7.0/suite/suite.go:203 +0x18f
  github.com/stretchr/testify/suite.Run()
      /home/brian/go/pkg/mod/github.com/stretchr/testify@v1.7.0/suite/suite.go:176 +0x994
  testify-race.Test_Main()
      /tmp/testify-bugreport/main_test.go:28 +0x44
  testing.tRunner()
      /home/brian/sdk/go1.17.2/src/testing/testing.go:1259 +0x22f
  testing.(*T).Run·dwrap·21()
      /home/brian/sdk/go1.17.2/src/testing/testing.go:1306 +0x47
==================
--- FAIL: Test_Main (0.01s)
    --- FAIL: Test_Main/TestB (0.00s)
        testing.go:1152: race detected during execution of test
FAIL
FAIL    testify-race    0.022s
FAIL

Based on a quick look at the code, it seems like the usage of suite.SetT() in various places in suite.Run is the cause of the race.

The text was updated successfully, but these errors were encountered:

brackendawson · 2021-12-29T17:28:56Z

Parallel tests and subtests are broken in a number of ways with the suite package in the current release. This race, wrong subtest names in failed assertions, cleanup being run before the end of the test...

Does this branch work well for you? It is a breaking API change in the suite package but I hope that we can adopt it for testify v2 as it should allow us to fix a lot of the problems with parallel tests.

bstpierre · 2021-12-30T01:31:32Z

@brackendawson Yes. When I modify the test program above to match the new api and run against that branch, it runs without errors.

package main

import (
        "testing"

        "github.com/stretchr/testify/suite"
)

type MainSuite struct{}

func (s *MainSuite) TestA(t *suite.T) {
        t.Parallel()
        for x := 0; x < 1000000; x++ {
                t.Equal(x, add(x, 0))
        }
}

func (s *MainSuite) TestB(t *suite.T) {
        t.Parallel()
        for x := 0; x < 1000000; x++ {
                t.Equal(x+1, add(x, 1))
        }
}

func Test_Main(t *testing.T) {
        suite.Run(t, new(MainSuite))
}

MaggieMa21 · 2023-07-18T23:14:25Z

Any update on the ETA of this fix?

brackendawson · 2023-07-20T08:59:54Z

The fix for this wold require testify/v2/suite, this is not expected soon. For now; parallel tests and subtests are not supportable with testify/suite.

dolmen · 2023-07-25T05:05:11Z

Suite v2 will have to happen elsewhere. There is already too much maintenance work here on assert and mock.

pmenglund mentioned this issue Apr 4, 2023

run tests in parallel rockset/rockset-go-client#98

Closed

dolmen added bug concurrency pkg-suite Change related to package testify/suite labels Jul 25, 2023

dolmen mentioned this issue Aug 11, 2023

Use WaitGroup to ensure suite.Run() doesn't exit until all tests are done #1447

Closed

Antonboom mentioned this issue Nov 10, 2023

suite-subtest-run: report on usage suite.T().Run instead of suite.Run Antonboom/testifylint#35

Closed

Antonboom mentioned this issue Jun 7, 2024

suite-broken-parallel: ensure protection from t.Parallel() call inside suite tests and subtests Antonboom/testifylint#117

Closed

slickwarren mentioned this issue Jul 29, 2024

Parallelize Provisioning Automation rancher/qa-tasks#1398

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

suite.Run() triggers data race errors with parallel tests #1139

suite.Run() triggers data race errors with parallel tests #1139

bstpierre commented Dec 28, 2021

brackendawson commented Dec 29, 2021

bstpierre commented Dec 30, 2021

MaggieMa21 commented Jul 18, 2023

brackendawson commented Jul 20, 2023

dolmen commented Jul 25, 2023

suite.Run() triggers data race errors with parallel tests #1139

suite.Run() triggers data race errors with parallel tests #1139

Comments

bstpierre commented Dec 28, 2021

brackendawson commented Dec 29, 2021

bstpierre commented Dec 30, 2021

MaggieMa21 commented Jul 18, 2023

brackendawson commented Jul 20, 2023

dolmen commented Jul 25, 2023