From db8608ed63f5bd9abdd03f669f5bb80569c114b6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Olivier=20Mengu=C3=A9?= <dolmen@cpan.org>
Date: Thu, 9 Nov 2023 01:44:27 +0100
Subject: [PATCH] suite: fix subtest names (fix #1501) (#1504)

* suite: fix TestSubtestPanic failure (#1501)

The subtest of TestSubtestPanic is expected to fail, but that must not
make the testuite of package 'suite' to fail. So call Skip to make 'go
test' to ignore that expected failure.

* suite: fix subtests names

We are doing dirty things with testing.InternalTest. It looks like test
names should have the same prefix as the parent test, like if they were
true subtests (testing.T.Run).

So until we drop our usage of testing.InternamTest, let's rename the
tests.

Also added a few checks of the testing.RunTests result.
---
 suite/suite_test.go | 32 +++++++++++++++++---------------
 1 file changed, 17 insertions(+), 15 deletions(-)

diff --git a/suite/suite_test.go b/suite/suite_test.go
index a2b6b04d1..292dc298c 100644
--- a/suite/suite_test.go
+++ b/suite/suite_test.go
@@ -27,7 +27,7 @@ func TestSuiteRequireTwice(t *testing.T) {
 	ok := testing.RunTests(
 		allTestsFilter,
 		[]testing.InternalTest{{
-			Name: "TestSuiteRequireTwice",
+			Name: t.Name() + "/SuiteRequireTwice",
 			F: func(t *testing.T) {
 				suite := new(SuiteRequireTwice)
 				Run(t, suite)
@@ -104,31 +104,31 @@ func TestSuiteRecoverPanic(t *testing.T) {
 	ok := true
 	panickingTests := []testing.InternalTest{
 		{
-			Name: "TestPanicInSetupSuite",
+			Name: t.Name() + "/InSetupSuite",
 			F:    func(t *testing.T) { Run(t, &panickingSuite{panicInSetupSuite: true}) },
 		},
 		{
-			Name: "TestPanicInSetupTest",
+			Name: t.Name() + "/InSetupTest",
 			F:    func(t *testing.T) { Run(t, &panickingSuite{panicInSetupTest: true}) },
 		},
 		{
-			Name: "TestPanicInBeforeTest",
+			Name: t.Name() + "InBeforeTest",
 			F:    func(t *testing.T) { Run(t, &panickingSuite{panicInBeforeTest: true}) },
 		},
 		{
-			Name: "TestPanicInTest",
+			Name: t.Name() + "/InTest",
 			F:    func(t *testing.T) { Run(t, &panickingSuite{panicInTest: true}) },
 		},
 		{
-			Name: "TestPanicInAfterTest",
+			Name: t.Name() + "/InAfterTest",
 			F:    func(t *testing.T) { Run(t, &panickingSuite{panicInAfterTest: true}) },
 		},
 		{
-			Name: "TestPanicInTearDownTest",
+			Name: t.Name() + "/InTearDownTest",
 			F:    func(t *testing.T) { Run(t, &panickingSuite{panicInTearDownTest: true}) },
 		},
 		{
-			Name: "TestPanicInTearDownSuite",
+			Name: t.Name() + "/InTearDownSuite",
 			F:    func(t *testing.T) { Run(t, &panickingSuite{panicInTearDownSuite: true}) },
 		},
 	}
@@ -451,7 +451,7 @@ func TestSuiteLogging(t *testing.T) {
 	suiteLoggingTester := new(SuiteLoggingTester)
 	capture := StdoutCapture{}
 	internalTest := testing.InternalTest{
-		Name: "SomeTest",
+		Name: t.Name() + "/SuiteLoggingTester",
 		F: func(subT *testing.T) {
 			Run(subT, suiteLoggingTester)
 		},
@@ -552,14 +552,15 @@ func (s *suiteWithStats) TestPanic() {
 func TestSuiteWithStats(t *testing.T) {
 	suiteWithStats := new(suiteWithStats)
 
-	testing.RunTests(allTestsFilter, []testing.InternalTest{
+	suiteSuccess := testing.RunTests(allTestsFilter, []testing.InternalTest{
 		{
-			Name: "WithStats",
+			Name: t.Name() + "/suiteWithStats",
 			F: func(t *testing.T) {
 				Run(t, suiteWithStats)
 			},
 		},
 	})
+	require.False(t, suiteSuccess, "suiteWithStats should report test failure because of panic in TestPanic")
 
 	assert.True(t, suiteWithStats.wasCalled)
 	assert.NotZero(t, suiteWithStats.stats.Start)
@@ -596,7 +597,7 @@ func TestFailfastSuite(t *testing.T) {
 	ok := testing.RunTests(
 		allTestsFilter,
 		[]testing.InternalTest{{
-			Name: "TestFailfastSuite",
+			Name: t.Name() + "/FailfastSuite",
 			F: func(t *testing.T) {
 				Run(t, s)
 			},
@@ -669,9 +670,10 @@ func (s *subtestPanicSuite) TearDownSubTest() {
 }
 
 func (s *subtestPanicSuite) TestSubtestPanic() {
-	s.Run("subtest", func() {
+	ok := s.Run("subtest", func() {
 		panic("panic")
 	})
+	s.False(ok, "subtest failure is expected")
 }
 
 func TestSubtestPanic(t *testing.T) {
@@ -679,13 +681,13 @@ func TestSubtestPanic(t *testing.T) {
 	ok := testing.RunTests(
 		allTestsFilter,
 		[]testing.InternalTest{{
-			Name: "TestSubtestPanic",
+			Name: t.Name() + "/subtestPanicSuite",
 			F: func(t *testing.T) {
 				Run(t, suite)
 			},
 		}},
 	)
-	assert.False(t, ok)
+	assert.False(t, ok, "TestSubtestPanic/subtest should make the testsuite fail")
 	assert.True(t, suite.inTearDownSubTest)
 	assert.True(t, suite.inTearDownTest)
 	assert.True(t, suite.inTearDownSuite)