Skip to content

Commit

Permalink
suite: fix deadlock in suite.Require()/Assert()
Browse files Browse the repository at this point in the history
As pointed out in issue #1520, if the suite is not initialised properly
(buy calling the Run function), then calling suite.Require() or
suite.Assert() will result in a deadlock.

This commit fixes that by panicking if the suite is not initialised
properly. This is justified because, the suite is intended to be
triggered in the right way. If the user does not do that, this panic will
nudge them in the right direction.

It has to be a panic because, at this point, we don't have access to any
testing.T context to gracefully call a t.Fail(). Also, these two
functions are not expected to return an error.

Fixes #1520
  • Loading branch information
arjunmahishi committed Feb 18, 2024
1 parent 0e75f99 commit c41592b
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 2 deletions.
4 changes: 2 additions & 2 deletions suite/suite.go
Expand Up @@ -58,7 +58,7 @@ func (suite *Suite) Require() *require.Assertions {
suite.mu.Lock()
defer suite.mu.Unlock()
if suite.require == nil {
suite.require = require.New(suite.T())
panic("'Require' must not be called before 'Run' or 'SetT'")
}
return suite.require
}
Expand All @@ -72,7 +72,7 @@ func (suite *Suite) Assert() *assert.Assertions {
suite.mu.Lock()
defer suite.mu.Unlock()
if suite.Assertions == nil {
suite.Assertions = assert.New(suite.T())
panic("'Assert' must not be called before 'Run' or 'SetT'")
}
return suite.Assertions
}
Expand Down
24 changes: 24 additions & 0 deletions suite/suite_test.go
Expand Up @@ -692,3 +692,27 @@ func TestSubtestPanic(t *testing.T) {
assert.True(t, suite.inTearDownTest)
assert.True(t, suite.inTearDownSuite)
}

type unInitialisedSuite struct {
Suite
}

// TestUnInitialisedSuites asserts the behaviour of the suite methods when the
// suite is not initialised
func TestUnInitialisedSuites(t *testing.T) {
t.Run("should panic on Require", func(t *testing.T) {
suite := new(unInitialisedSuite)

assert.Panics(t, func() {
suite.Require().True(true)
})
})

t.Run("should panic on Assert", func(t *testing.T) {
suite := new(unInitialisedSuite)

assert.Panics(t, func() {
suite.Assert().True(true)
})
})
}

0 comments on commit c41592b

Please sign in to comment.