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.

Sign up for GitHub

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

Jump to bottom

suite: fix SetupSubTest and TearDownSubTest execution order #1471

Merged
merged 8 commits into from
Oct 30, 2023
Merged

suite: fix SetupSubTest and TearDownSubTest execution order #1471

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
916ce79
fix: SetupSubTest and TearDownSubTest execution order
linusbarth Oct 16, 2023
83e44bc
improve: defer-style in Suite.Run()
linusbarth Oct 16, 2023
4657972
test: call order of setup/teardown for subtests
linusbarth Oct 16, 2023
aa01ed0
test: testing.T correctness in subtests setup/teardown
linusbarth Oct 16, 2023
ac5cd69
improve: move comment to msgAndArgs-param in test
linusbarth Oct 16, 2023
c8efd3c
improve: tests for asserting test names in subtests
linusbarth Oct 17, 2023
91478fe
fix: panic behavior for subtests
linusbarth Oct 18, 2023
c18759c
test: add suite tests for panicking of subtests
linusbarth Oct 18, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 9 additions & 9 deletions suite/suite.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,19 +96,19 @@ 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)
dolmen marked this conversation as resolved.
Show resolved Hide resolved

defer suite.SetT(oldT)

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()
})
}
Expand Down
28 changes: 25 additions & 3 deletions suite/suite_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,8 +242,8 @@ func (suite *SuiteTester) TestSubtest() {
for _, t := range []struct {
testName string
}{
{"first"},
{"second"},
{"first-subtest"},
{"second-subtest"},
} {
suiteT := suite.T()
suite.Run(t.testName, func() {
Expand All @@ -259,10 +259,12 @@ func (suite *SuiteTester) TestSubtest() {

func (suite *SuiteTester) TearDownSubTest() {
suite.TearDownSubTestRunCount++
suite.Contains(suite.T().Name(), "subtest", "We should get the *testing.T for the test that is to be torn down")
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updating the test suite attributes here and performing assertions alongside the others in the main test body will be more consistent with the current testing setup:

testify/suite/suite_test.go

Lines 154 to 172 in ac5cd69

SetupSuiteRunCount int
TearDownSuiteRunCount int
SetupTestRunCount int
TearDownTestRunCount int
TestOneRunCount int
TestTwoRunCount int
TestSubtestRunCount int
NonTestMethodRunCount int
SetupSubTestRunCount int
TearDownSubTestRunCount int
SuiteNameBefore []string
TestNameBefore []string
SuiteNameAfter []string
TestNameAfter []string
TimeBefore []time.Time
TimeAfter []time.Time

See this for reference:
https://github.com/stretchr/testify/pull/1393/files#diff-ca4ffdb7050ad570420ac5682fefad8481384709c2884051cd57071283ce9ca4

Copy link
Contributor Author

@linusbarth linusbarth Oct 17, 2023 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your review! You're right, that is more consistent. I mostly just copied your referenced testing code now into my branch.

}

func (suite *SuiteTester) SetupSubTest() {
suite.SetupSubTestRunCount++
suite.Contains(suite.T().Name(), "subtest", "We should get the *testing.T for the test that is to be set up")
}

type SuiteSkipTester struct {
Expand Down Expand Up @@ -481,7 +483,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")
Expand All @@ -491,12 +493,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 {
Expand Down