Skip to content

Commit b8520a1

Browse files
committed
suite: skip AfterTest when SetupTest or BeforeTest skips
If SetupTest or BeforeTest calls t.Skip(), the deferred cleanup still invoked AfterTest even though the paired BeforeTest never completed. Track whether BeforeTest finished and only run AfterTest when it did, so AfterTest always has a matching BeforeTest. TearDownTest still runs in all cases to preserve the existing contract that teardown can clean up partially initialized state. Fixes #1781
1 parent 5f80e4a commit b8520a1

2 files changed

Lines changed: 143 additions & 1 deletion

File tree

suite/suite.go

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,7 @@ func Run(t *testing.T, suite TestingSuite) {
181181
parentT := suite.T()
182182
suite.SetT(t)
183183
defer recoverAndFailOnPanic(t)
184+
var beforeTestFinished bool
184185
defer func() {
185186
t.Helper()
186187

@@ -189,7 +190,12 @@ func Run(t *testing.T, suite TestingSuite) {
189190
stats.end(method.Name, !t.Failed() && r == nil)
190191

191192
if afterTestSuite, ok := suite.(AfterTest); ok {
192-
afterTestSuite.AfterTest(suiteName, method.Name)
193+
// Only run AfterTest if BeforeTest finished without
194+
// calling Skip. This mirrors the guarantee that
195+
// AfterTest has a matching BeforeTest.
196+
if beforeTestFinished {
197+
afterTestSuite.AfterTest(suiteName, method.Name)
198+
}
193199
}
194200

195201
if tearDownTestSuite, ok := suite.(TearDownTestSuite); ok {
@@ -206,6 +212,7 @@ func Run(t *testing.T, suite TestingSuite) {
206212
if beforeTestSuite, ok := suite.(BeforeTest); ok {
207213
beforeTestSuite.BeforeTest(methodFinder.Elem().Name(), method.Name)
208214
}
215+
beforeTestFinished = true
209216

210217
stats.start(method.Name)
211218

suite/suite_test.go

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -813,3 +813,138 @@ func TestSuiteSignatureValidation(t *testing.T) {
813813
assert.True(t, suiteTester.setUp, "SetupSuite should have been executed")
814814
assert.True(t, suiteTester.toreDown, "TearDownSuite should have been executed")
815815
}
816+
817+
// skipInBeforeTestSuite is used to verify that AfterTest is not called when
818+
// BeforeTest skips the test (#1781).
819+
type skipInBeforeTestSuite struct {
820+
Suite
821+
beforeTestCount int
822+
afterTestCount int
823+
testRunCount int
824+
}
825+
826+
func (s *skipInBeforeTestSuite) BeforeTest(_, _ string) {
827+
s.beforeTestCount++
828+
s.T().Skip()
829+
}
830+
831+
func (s *skipInBeforeTestSuite) AfterTest(_, _ string) {
832+
s.afterTestCount++
833+
}
834+
835+
func (s *skipInBeforeTestSuite) TestA() {
836+
s.testRunCount++
837+
}
838+
839+
// TestSkipInBeforeTest asserts that calling Skip from BeforeTest skips the
840+
// test and prevents AfterTest from running. See #1781.
841+
func TestSkipInBeforeTest(t *testing.T) {
842+
s := new(skipInBeforeTestSuite)
843+
ok := testing.RunTests(
844+
allTestsFilter,
845+
[]testing.InternalTest{{
846+
Name: t.Name() + "/skipInBeforeTestSuite",
847+
F: func(t *testing.T) {
848+
Run(t, s)
849+
},
850+
}},
851+
)
852+
assert.True(t, ok, "suite should not fail when BeforeTest skips")
853+
assert.Equal(t, 1, s.beforeTestCount, "BeforeTest should run once")
854+
assert.Equal(t, 0, s.testRunCount, "test body should not run when BeforeTest skips")
855+
assert.Equal(t, 0, s.afterTestCount, "AfterTest should not run when BeforeTest skips")
856+
}
857+
858+
// skipInSetupTestSuite is used to verify that AfterTest is not called when
859+
// SetupTest skips the test (#1781).
860+
type skipInSetupTestSuite struct {
861+
Suite
862+
setupTestCount int
863+
beforeTestCount int
864+
afterTestCount int
865+
tearDownTestCount int
866+
testRunCount int
867+
}
868+
869+
func (s *skipInSetupTestSuite) SetupTest() {
870+
s.setupTestCount++
871+
s.T().Skip()
872+
}
873+
874+
func (s *skipInSetupTestSuite) BeforeTest(_, _ string) {
875+
s.beforeTestCount++
876+
}
877+
878+
func (s *skipInSetupTestSuite) AfterTest(_, _ string) {
879+
s.afterTestCount++
880+
}
881+
882+
func (s *skipInSetupTestSuite) TearDownTest() {
883+
s.tearDownTestCount++
884+
}
885+
886+
func (s *skipInSetupTestSuite) TestA() {
887+
s.testRunCount++
888+
}
889+
890+
// TestSkipInSetupTest asserts that calling Skip from SetupTest skips the
891+
// test, skips BeforeTest, and prevents AfterTest from running. TearDownTest
892+
// still runs so dependencies set up before the skip can be cleaned up.
893+
// See #1781.
894+
func TestSkipInSetupTest(t *testing.T) {
895+
s := new(skipInSetupTestSuite)
896+
ok := testing.RunTests(
897+
allTestsFilter,
898+
[]testing.InternalTest{{
899+
Name: t.Name() + "/skipInSetupTestSuite",
900+
F: func(t *testing.T) {
901+
Run(t, s)
902+
},
903+
}},
904+
)
905+
assert.True(t, ok, "suite should not fail when SetupTest skips")
906+
assert.Equal(t, 1, s.setupTestCount, "SetupTest should run once")
907+
assert.Equal(t, 0, s.beforeTestCount, "BeforeTest should not run when SetupTest skips")
908+
assert.Equal(t, 0, s.testRunCount, "test body should not run when SetupTest skips")
909+
assert.Equal(t, 0, s.afterTestCount, "AfterTest should not run when SetupTest skips")
910+
assert.Equal(t, 1, s.tearDownTestCount, "TearDownTest should still run when SetupTest skips")
911+
}
912+
913+
// skipInTestBodySuite is used to verify that AfterTest is still called when
914+
// the test body itself calls Skip (regression guard for existing behavior).
915+
type skipInTestBodySuite struct {
916+
Suite
917+
beforeTestCount int
918+
afterTestCount int
919+
}
920+
921+
func (s *skipInTestBodySuite) BeforeTest(_, _ string) {
922+
s.beforeTestCount++
923+
}
924+
925+
func (s *skipInTestBodySuite) AfterTest(_, _ string) {
926+
s.afterTestCount++
927+
}
928+
929+
func (s *skipInTestBodySuite) TestA() {
930+
s.T().Skip()
931+
}
932+
933+
// TestSkipInTestBodyStillRunsAfterTest asserts that calling Skip from inside
934+
// a test method still triggers AfterTest — only skips originating before the
935+
// test method is entered should suppress AfterTest (#1781).
936+
func TestSkipInTestBodyStillRunsAfterTest(t *testing.T) {
937+
s := new(skipInTestBodySuite)
938+
ok := testing.RunTests(
939+
allTestsFilter,
940+
[]testing.InternalTest{{
941+
Name: t.Name() + "/skipInTestBodySuite",
942+
F: func(t *testing.T) {
943+
Run(t, s)
944+
},
945+
}},
946+
)
947+
assert.True(t, ok, "suite should not fail when test body skips")
948+
assert.Equal(t, 1, s.beforeTestCount, "BeforeTest should run once")
949+
assert.Equal(t, 1, s.afterTestCount, "AfterTest should still run when the test body skips")
950+
}

0 commit comments

Comments
 (0)