Repository: stretchr/testify (go)

Issue:
Test name refers to parent test name in TearDownSubTest
When having tests with subtests, the calling of the `s.T().Name()` in the `TearDownSubTest` will return the parent test name and not the subtest that has been executed
Having this test executed will print the correct sub-test names:
```go
func (s *TestingSuite) TestWithSubtests() {
  s.Run("my subtest", func() {
    println("my subtest -> " + s.T().Name())
    s.Run("my sub subtest", func() {
      println("my sub subtest -> " + s.T().Name())
    })
  })
}
```
Output:
```
=== RUN   TestingSuite/TestWithSubtests
=== RUN   TestingSuite/TestWithSubtests/my_subtest
my subtest -> TestSuite/TestHazelcastExternalSmartClientLoadBalancer/my_subtest
=== RUN   TestingSuite/TestWithSubtests/my_subtest/my_sub_subtest
my sub subtest -> TestSuite/TestHazelcastExternalSmartClientLoadBalancer/my_subtest/my_sub_subtest
```

And I would expect that in the `TearDownSubTest`, the `s.T().Name()` should be consistent with these names. However, it always refers to the parent test name, i.e., `TestingSuite/TestWithSubtests` for the `TestingSuite/TestWithSubtests/my_subtest`, and `TestingSuite/TestWithSubtests/my_subtest` for the `TestingSuite/TestWithSubtests/my_subtest/my_sub_subtest`.

```go
func (s *TestingSuite) TearDownSubTest() {
  println("TearDownSubTest -> " + s.T().Name())
}
```

Output:
```
TearDownSubTest -> TestSuite/TestHazelcastExternalSmartClientLoadBalancer/my_subtest
TearDownSubTest -> TestSuite/TestHazelcastExternalSmartClientLoadBalancer
```

Environment:
Go 1.19
Testify 1.8.4

Pull request description:
suite: fix SetupSubTest and TearDownSubTest execution order
## Summary
The order of execution of SetupSubTest and TearDownSubTest methods was changed so that the testing context inside these methods is correct.

## Changes
* The SetupSubTest was moved to after the SetT that set the child testing context
* The TearDownSubTest was moved to before the deferred SetT that reset the testing context (execution wise)
* The TearDownSubTest and SetT were deferred in one block. This was fine beforehand because a panic in the TearDownSubTest would have had no influence on the "suite.SetT(oldT)". Now since a panic in the TearDownSubTest would lead to omitting the "suite.SetT(oldT)" this defer was split into two separate defers.

## Motivation
There were two problems with the order of execution in the Suite.Run() method:
* One could not access the correct testing context ("s.T()") inside the SetupSubTest and TearDownSubTest methods. If the testing context was used for e.g. assertions of mocks in the TearDownSubTest, the results would not be "attached" to the correct test in the test output.
* The behavior was different to the order of execution for "root" tests regarding the SetupTest and TearDownTest methods (see lines 167-201). This could confuse users of the library.

## Related issues
Closes #1465


Patch:
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index 08796ae..be8b4af 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -7,8 +7,8 @@ jobs:
     strategy:
       matrix:
         go_version:
-          - "1.19"
           - "1.20"
+          - "1.21"
     steps:
       - uses: actions/checkout@v4
       - name: Setup Go
@@ -28,6 +28,7 @@ jobs:
           - "1.18"
           - "1.19"
           - "1.20"
+          - "1.21"
     steps:
       - uses: actions/checkout@v4
       - name: Setup Go
diff --git a/assert/assertions.go b/assert/assertions.go
index 1e55fbf..6843d42 100644
--- a/assert/assertions.go
+++ b/assert/assertions.go
@@ -150,6 +150,8 @@ func copyExportedFields(expected interface{}) interface{} {
 // structures.
 //
 // This function does no assertion of any kind.
+//
+// Deprecated: Use [EqualExportedValues] instead.
 func ObjectsExportedFieldsAreEqual(expected, actual interface{}) bool {
 	expectedCleaned := copyExportedFields(expected)
 	actualCleaned := copyExportedFields(actual)
diff --git a/suite/suite.go b/suite/suite.go
index 8b4202d..f3aa9a1 100644
--- a/suite/suite.go
+++ b/suite/suite.go
@@ -96,19 +96,20 @@ func failOnPanic(t *testing.T, r interface{}) {
 func (suite *Suite) Run(name string, subtest func()) bool {
 	oldT := suite.T()
 
-	if setupSubTest, ok := suite.s.(SetupSubTest); ok {
-		setupSubTest.SetupSubTest()
-	}
+	return oldT.Run(name, func(t *testing.T) {
+		suite.SetT(t)
+		defer suite.SetT(oldT)
+
+		defer recoverAndFailOnPanic(t)
+
+		if setupSubTest, ok := suite.s.(SetupSubTest); ok {
+			setupSubTest.SetupSubTest()
+		}
 
-	defer func() {
-		suite.SetT(oldT)
 		if tearDownSubTest, ok := suite.s.(TearDownSubTest); ok {
-			tearDownSubTest.TearDownSubTest()
+			defer tearDownSubTest.TearDownSubTest()
 		}
-	}()
 
-	return oldT.Run(name, func(t *testing.T) {
-		suite.SetT(t)
 		subtest()
 	})
 }


Test patch:
diff --git a/suite/suite_test.go b/suite/suite_test.go
index d684f52..a2b6b04 100644
--- a/suite/suite_test.go
+++ b/suite/suite_test.go
@@ -34,7 +34,7 @@ func TestSuiteRequireTwice(t *testing.T) {
 			},
 		}},
 	)
-	assert.Equal(t, false, ok)
+	assert.False(t, ok)
 }
 
 func (s *SuiteRequireTwice) TestRequireOne() {
@@ -162,6 +162,9 @@ type SuiteTester struct {
 	SetupSubTestRunCount    int
 	TearDownSubTestRunCount int
 
+	SetupSubTestNames    []string
+	TearDownSubTestNames []string
+
 	SuiteNameBefore []string
 	TestNameBefore  []string
 
@@ -258,10 +261,12 @@ func (suite *SuiteTester) TestSubtest() {
 }
 
 func (suite *SuiteTester) TearDownSubTest() {
+	suite.TearDownSubTestNames = append(suite.TearDownSubTestNames, suite.T().Name())
 	suite.TearDownSubTestRunCount++
 }
 
 func (suite *SuiteTester) SetupSubTest() {
+	suite.SetupSubTestNames = append(suite.SetupSubTestNames, suite.T().Name())
 	suite.SetupSubTestRunCount++
 }
 
@@ -301,13 +306,13 @@ func TestRunSuite(t *testing.T) {
 
 	// The suite was only run once, so the SetupSuite and TearDownSuite
 	// methods should have each been run only once.
-	assert.Equal(t, suiteTester.SetupSuiteRunCount, 1)
-	assert.Equal(t, suiteTester.TearDownSuiteRunCount, 1)
+	assert.Equal(t, 1, suiteTester.SetupSuiteRunCount)
+	assert.Equal(t, 1, suiteTester.TearDownSuiteRunCount)
 
-	assert.Equal(t, len(suiteTester.SuiteNameAfter), 4)
-	assert.Equal(t, len(suiteTester.SuiteNameBefore), 4)
-	assert.Equal(t, len(suiteTester.TestNameAfter), 4)
-	assert.Equal(t, len(suiteTester.TestNameBefore), 4)
+	assert.Len(t, suiteTester.SuiteNameAfter, 4)
+	assert.Len(t, suiteTester.SuiteNameBefore, 4)
+	assert.Len(t, suiteTester.TestNameAfter, 4)
+	assert.Len(t, suiteTester.TestNameBefore, 4)
 
 	assert.Contains(t, suiteTester.TestNameAfter, "TestOne")
 	assert.Contains(t, suiteTester.TestNameAfter, "TestTwo")
@@ -319,6 +324,12 @@ func TestRunSuite(t *testing.T) {
 	assert.Contains(t, suiteTester.TestNameBefore, "TestSkip")
 	assert.Contains(t, suiteTester.TestNameBefore, "TestSubtest")
 
+	assert.Contains(t, suiteTester.SetupSubTestNames, "TestRunSuite/TestSubtest/first")
+	assert.Contains(t, suiteTester.SetupSubTestNames, "TestRunSuite/TestSubtest/second")
+
+	assert.Contains(t, suiteTester.TearDownSubTestNames, "TestRunSuite/TestSubtest/first")
+	assert.Contains(t, suiteTester.TearDownSubTestNames, "TestRunSuite/TestSubtest/second")
+
 	for _, suiteName := range suiteTester.SuiteNameAfter {
 		assert.Equal(t, "SuiteTester", suiteName)
 	}
@@ -338,20 +349,20 @@ func TestRunSuite(t *testing.T) {
 	// There are four test methods (TestOne, TestTwo, TestSkip, and TestSubtest), so
 	// the SetupTest and TearDownTest methods (which should be run once for
 	// each test) should have been run four times.
-	assert.Equal(t, suiteTester.SetupTestRunCount, 4)
-	assert.Equal(t, suiteTester.TearDownTestRunCount, 4)
+	assert.Equal(t, 4, suiteTester.SetupTestRunCount)
+	assert.Equal(t, 4, suiteTester.TearDownTestRunCount)
 
 	// Each test should have been run once.
-	assert.Equal(t, suiteTester.TestOneRunCount, 1)
-	assert.Equal(t, suiteTester.TestTwoRunCount, 1)
-	assert.Equal(t, suiteTester.TestSubtestRunCount, 1)
+	assert.Equal(t, 1, suiteTester.TestOneRunCount)
+	assert.Equal(t, 1, suiteTester.TestTwoRunCount)
+	assert.Equal(t, 1, suiteTester.TestSubtestRunCount)
 
-	assert.Equal(t, suiteTester.TearDownSubTestRunCount, 2)
-	assert.Equal(t, suiteTester.SetupSubTestRunCount, 2)
+	assert.Equal(t, 2, suiteTester.TearDownSubTestRunCount)
+	assert.Equal(t, 2, suiteTester.SetupSubTestRunCount)
 
 	// Methods that don't match the test method identifier shouldn't
 	// have been run at all.
-	assert.Equal(t, suiteTester.NonTestMethodRunCount, 0)
+	assert.Equal(t, 0, suiteTester.NonTestMethodRunCount)
 
 	suiteSkipTester := new(SuiteSkipTester)
 	Run(t, suiteSkipTester)
@@ -359,8 +370,8 @@ func TestRunSuite(t *testing.T) {
 	// The suite was only run once, so the SetupSuite and TearDownSuite
 	// methods should have each been run only once, even though SetupSuite
 	// called Skip()
-	assert.Equal(t, suiteSkipTester.SetupSuiteRunCount, 1)
-	assert.Equal(t, suiteSkipTester.TearDownSuiteRunCount, 1)
+	assert.Equal(t, 1, suiteSkipTester.SetupSuiteRunCount)
+	assert.Equal(t, 1, suiteSkipTester.TearDownSuiteRunCount)
 
 }
 
@@ -481,7 +492,7 @@ func (s *CallOrderSuite) SetupSuite() {
 
 func (s *CallOrderSuite) TearDownSuite() {
 	s.call("TearDownSuite")
-	assert.Equal(s.T(), "SetupSuite;SetupTest;Test A;TearDownTest;SetupTest;Test B;TearDownTest;TearDownSuite", strings.Join(s.callOrder, ";"))
+	assert.Equal(s.T(), "SetupSuite;SetupTest;Test A;SetupSubTest;SubTest A1;TearDownSubTest;SetupSubTest;SubTest A2;TearDownSubTest;TearDownTest;SetupTest;Test B;SetupSubTest;SubTest B1;TearDownSubTest;SetupSubTest;SubTest B2;TearDownSubTest;TearDownTest;TearDownSuite", strings.Join(s.callOrder, ";"))
 }
 func (s *CallOrderSuite) SetupTest() {
 	s.call("SetupTest")
@@ -491,12 +502,32 @@ func (s *CallOrderSuite) TearDownTest() {
 	s.call("TearDownTest")
 }
 
+func (s *CallOrderSuite) SetupSubTest() {
+	s.call("SetupSubTest")
+}
+
+func (s *CallOrderSuite) TearDownSubTest() {
+	s.call("TearDownSubTest")
+}
+
 func (s *CallOrderSuite) Test_A() {
 	s.call("Test A")
+	s.Run("SubTest A1", func() {
+		s.call("SubTest A1")
+	})
+	s.Run("SubTest A2", func() {
+		s.call("SubTest A2")
+	})
 }
 
 func (s *CallOrderSuite) Test_B() {
 	s.call("Test B")
+	s.Run("SubTest B1", func() {
+		s.call("SubTest B1")
+	})
+	s.Run("SubTest B2", func() {
+		s.call("SubTest B2")
+	})
 }
 
 type suiteWithStats struct {
@@ -571,7 +602,7 @@ func TestFailfastSuite(t *testing.T) {
 			},
 		}},
 	)
-	assert.Equal(t, false, ok)
+	assert.False(t, ok)
 	if failFast {
 		// Test A Fails and because we are running with failfast Test B never runs and we proceed straight to TearDownSuite
 		assert.Equal(t, "SetupSuite;SetupTest;Test A Fails;TearDownTest;TearDownSuite", strings.Join(s.callOrder, ";"))
@@ -617,3 +648,45 @@ func (s *FailfastSuite) Test_B_Passes() {
 	s.call("Test B Passes")
 	s.Require().True(true)
 }
+
+type subtestPanicSuite struct {
+	Suite
+	inTearDownSuite   bool
+	inTearDownTest    bool
+	inTearDownSubTest bool
+}
+
+func (s *subtestPanicSuite) TearDownSuite() {
+	s.inTearDownSuite = true
+}
+
+func (s *subtestPanicSuite) TearDownTest() {
+	s.inTearDownTest = true
+}
+
+func (s *subtestPanicSuite) TearDownSubTest() {
+	s.inTearDownSubTest = true
+}
+
+func (s *subtestPanicSuite) TestSubtestPanic() {
+	s.Run("subtest", func() {
+		panic("panic")
+	})
+}
+
+func TestSubtestPanic(t *testing.T) {
+	suite := new(subtestPanicSuite)
+	ok := testing.RunTests(
+		allTestsFilter,
+		[]testing.InternalTest{{
+			Name: "TestSubtestPanic",
+			F: func(t *testing.T) {
+				Run(t, suite)
+			},
+		}},
+	)
+	assert.False(t, ok)
+	assert.True(t, suite.inTearDownSubTest)
+	assert.True(t, suite.inTearDownTest)
+	assert.True(t, suite.inTearDownSuite)
+}