Repository: stretchr/testify
Pull request #1165 (2022-03-03 15:46:58)

fix data race in the suit
Signed-off-by: Weizhen Wang <wangweizhen@pingcap.com>

## Summary
<!-- High-level, one sentence summary of what this PR accomplishes -->

we find a data race in the suit. so we think Mutex is added to the suit for solving it.

ref https://github.com/pingcap/tidb/issues/32747

```
==================
WARNING: DATA RACE
Write at 0x00c026a29588 by goroutine 117:
  github.com/stretchr/testify/suite.(*Suite).SetT()
      /home/prow/go/pkg/mod/github.com/stretchr/testify@v1.7.0/suite/suite.go:37 +0x148
  github.com/pingcap/tidb/executor_test.(*infosSchemaClusterTableSuite).SetT()
      <autogenerated>:1 +0x44
  github.com/stretchr/testify/suite.Run.func1()
      /home/prow/go/pkg/mod/github.com/stretchr/testify@v1.7.0/suite/suite.go:128 +0xcc
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:1259 +0x22f
  testing.(*T).Run·dwrap·21()
      /usr/local/go/src/testing/testing.go:1306 +0x47
Previous read at 0x00c026a29588 by goroutine 209:
  github.com/stretchr/testify/suite.(*Suite).Require()
      /home/prow/go/pkg/mod/github.com/stretchr/testify@v1.7.0/suite/suite.go:42 +0x5a
  github.com/pingcap/tidb/executor_test.(*infosSchemaClusterTableSuite).setUpRPCService.func1()
      /go/tidb/executor/infoschema_cluster_table_test.go:86 +0x46
Goroutine 117 (running) created at:
  testing.(*T).Run()
      /usr/local/go/src/testing/testing.go:1306 +0x726
  github.com/stretchr/testify/suite.runTests()
      /home/prow/go/pkg/mod/github.com/stretchr/testify@v1.7.0/suite/suite.go:203 +0x18f
  github.com/stretchr/testify/suite.Run()
      /home/prow/go/pkg/mod/github.com/stretchr/testify@v1.7.0/suite/suite.go:176 +0x994
  github.com/pingcap/tidb/executor_test.TestInfoSchemaClusterTable()
      /go/tidb/executor/infoschema_cluster_table_test.go:57 +0x44
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:1259 +0x22f
  testing.(*T).Run·dwrap·21()
      /usr/local/go/src/testing/testing.go:1306 +0x47
Goroutine 209 (running) created at:
  github.com/pingcap/tidb/executor_test.(*infosSchemaClusterTableSuite).setUpRPCService()
      /go/tidb/executor/infoschema_cluster_table_test.go:85 +0x5d2
  github.com/pingcap/tidb/executor_test.(*infosSchemaClusterTableSuite).SetupSuite()
      /go/tidb/executor/infoschema_cluster_table_test.go:62 +0x184
  github.com/stretchr/testify/suite.Run()
      /home/prow/go/pkg/mod/github.com/stretchr/testify@v1.7.0/suite/suite.go:118 +0x5b7
  github.com/pingcap/tidb/executor_test.TestInfoSchemaClusterTable()
      /go/tidb/executor/infoschema_cluster_table_test.go:57 +0x44
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:1259 +0x22f
  testing.(*T).Run·dwrap·21()
      /usr/local/go/src/testing/testing.go:1306 +0x47
================== 
```
## Changes

add Mutex to the suit.
<!-- * Description of change 1 -->
<!-- * Description of change 2 -->
<!-- ... -->

## Motivation
<!-- Why were the changes necessary. -->

fix data race.

<!-- ## Example usage (if applicable) -->

## Related issues
<!-- Put `Closes #XXXX` for each issue number this PR fixes/closes -->




hawkingrei: @boyan-soubachov PTAL

Patch:
diff --git a/README.md b/README.md
index c78250e..6b9ff9a 100644
--- a/README.md
+++ b/README.md
@@ -323,7 +323,7 @@ To update Testify to the latest version, use `go get -u github.com/stretchr/test
 Supported go versions
 ==================
 
-We support the three major Go versions, which are 1.13, 1.14 and 1.15 at the moment.
+We currently support the most recent major Go versions from 1.13 onward.
 
 ------
 
diff --git a/assert/assertion_compare_can_convert.go b/assert/assertion_compare_can_convert.go
index df22c47..da86790 100644
--- a/assert/assertion_compare_can_convert.go
+++ b/assert/assertion_compare_can_convert.go
@@ -9,7 +9,7 @@ package assert
 
 import "reflect"
 
-// Wrapper around reflect.Value.CanConvert, for compatability
+// Wrapper around reflect.Value.CanConvert, for compatibility
 // reasons.
 func canConvert(value reflect.Value, to reflect.Type) bool {
 	return value.CanConvert(to)
diff --git a/assert/assertions.go b/assert/assertions.go
index 230118b..0357b22 100644
--- a/assert/assertions.go
+++ b/assert/assertions.go
@@ -1004,27 +1004,21 @@ func Condition(t TestingT, comp Comparison, msgAndArgs ...interface{}) bool {
 type PanicTestFunc func()
 
 // didPanic returns true if the function passed to it panics. Otherwise, it returns false.
-func didPanic(f PanicTestFunc) (bool, interface{}, string) {
-
-	didPanic := false
-	var message interface{}
-	var stack string
-	func() {
-
-		defer func() {
-			if message = recover(); message != nil {
-				didPanic = true
-				stack = string(debug.Stack())
-			}
-		}()
-
-		// call the target function
-		f()
+func didPanic(f PanicTestFunc) (didPanic bool, message interface{}, stack string) {
+	didPanic = true
 
+	defer func() {
+		message = recover()
+		if didPanic {
+			stack = string(debug.Stack())
+		}
 	}()
 
-	return didPanic, message, stack
+	// call the target function
+	f()
+	didPanic = false
 
+	return
 }
 
 // Panics asserts that the code inside the specified PanicTestFunc panics.
diff --git a/suite/suite.go b/suite/suite.go
index b9b5d1c..1c402e8 100644
--- a/suite/suite.go
+++ b/suite/suite.go
@@ -7,6 +7,7 @@ import (
 	"reflect"
 	"regexp"
 	"runtime/debug"
+	"sync"
 	"testing"
 	"time"
 
@@ -21,17 +22,22 @@ var matchMethod = flag.String("testify.m", "", "regular expression to select tes
 // retrieving the current *testing.T context.
 type Suite struct {
 	*assert.Assertions
+	mu      sync.RWMutex
 	require *require.Assertions
 	t       *testing.T
 }
 
 // T retrieves the current *testing.T context.
 func (suite *Suite) T() *testing.T {
+	suite.mu.RLock()
+	defer suite.mu.RUnlock()
 	return suite.t
 }
 
 // SetT sets the current *testing.T context.
 func (suite *Suite) SetT(t *testing.T) {
+	suite.mu.Lock()
+	defer suite.mu.Unlock()
 	suite.t = t
 	suite.Assertions = assert.New(t)
 	suite.require = require.New(t)
@@ -39,6 +45,8 @@ func (suite *Suite) SetT(t *testing.T) {
 
 // Require returns a require context for suite.
 func (suite *Suite) Require() *require.Assertions {
+	suite.mu.Lock()
+	defer suite.mu.Unlock()
 	if suite.require == nil {
 		suite.require = require.New(suite.T())
 	}
@@ -51,6 +59,8 @@ func (suite *Suite) Require() *require.Assertions {
 // assert.Assertions with require.Assertions), this method is provided so you
 // can call `suite.Assert().NoError()`.
 func (suite *Suite) Assert() *assert.Assertions {
+	suite.mu.Lock()
+	defer suite.mu.Unlock()
 	if suite.Assertions == nil {
 		suite.Assertions = assert.New(suite.T())
 	}


Test patch:
diff --git a/assert/assertions_test.go b/assert/assertions_test.go
index 0b5b2c1..0f5de92 100644
--- a/assert/assertions_test.go
+++ b/assert/assertions_test.go
@@ -923,10 +923,18 @@ func TestCondition(t *testing.T) {
 
 func TestDidPanic(t *testing.T) {
 
-	if funcDidPanic, _, _ := didPanic(func() {
-		panic("Panic!")
-	}); !funcDidPanic {
-		t.Error("didPanic should return true")
+	const panicMsg = "Panic!"
+
+	if funcDidPanic, msg, _ := didPanic(func() {
+		panic(panicMsg)
+	}); !funcDidPanic || msg != panicMsg {
+		t.Error("didPanic should return true, panicMsg")
+	}
+
+	if funcDidPanic, msg, _ := didPanic(func() {
+		panic(nil)
+	}); !funcDidPanic || msg != nil {
+		t.Error("didPanic should return true, nil")
 	}
 
 	if funcDidPanic, _, _ := didPanic(func() {
@@ -963,6 +971,12 @@ func TestPanicsWithValue(t *testing.T) {
 		t.Error("PanicsWithValue should return true")
 	}
 
+	if !PanicsWithValue(mockT, nil, func() {
+		panic(nil)
+	}) {
+		t.Error("PanicsWithValue should return true")
+	}
+
 	if PanicsWithValue(mockT, "Panic!", func() {
 	}) {
 		t.Error("PanicsWithValue should return false")