stretchr
suite: TearDown isn't called if setup fails
I have a test suite
In the SetupSuite method I wait for some event to happen. If it doesn't happen within a certain timeout, I call t.FailNow()
The problem is that when SetupSuite fails, TearDownSuite doesn't get called, which is necessary in my case.
Is it intentional that TearDownSuite isn't called if setup fails?
My understanding after looking at the code is that TearDownSuite isn't actually deferred until SetupSuite completes (see this block of code).
I also checked the Go docs on (*testing.T).FailNow() and that method will call runtime.Goexit which causes defer statements to start executing. But as we've seen above the TearDownSuite hasn't been deferred at that point so your tear down isn't executed.
My best guess as to why it's written like this is some applications may have tear down code that absolutely should not be run if the setup did not complete successfully. You could get around the current behavior easily by doing something like the below:
type MySuite struct {
suite.Suite
suiteSetupDone bool
}
func (suite *MySuite) SetupSuite() {
defer func() {
// This is only run if SetupSuite fails or panic, otherwise TearDownSuite will call it
if !suite.suiteSetupDone {
runCriticallyNecessaryTearDown()
}
}
// ... do some actual setup ...
// This should be the last line of code, and will only run if nothing above has caused a failure or panic
suite.suiteSetupDone = true
}
func (suite *MySuite) TearDownSuite() {
// This body is only executed if the suite setup succeeds
runCriticallyNecessaryTearDown()
// ... maybe do some other tear down ...
}
Having the same case.
In my case I manage it like that:
func (s *Example) SetupSuite() {
s.database = NewDatabase()
require.Nil(s.T(), 1) // Failing
}
func (s *Example) TearDownSuite() {
if s.database != nil {
s.database.Close()
}
}
So not having TearDownSuite called is a big pain.
@colin-valentini do you have examples where not calling TearDownSuite is appropriate? Cannot figure it out... we could imagine they do not have to set the if s.database != nil statement but that's not a good reason.
From what I see it becomes really a no-go (not calling TearDownSuite) when initialising multiple variables, let's say:
func (s *Example) SetupSuite() {
var err error
s.database, err = NewDatabase()
require.Nil(s.T(), err) // PASSING
s.httpServer, err = NewHTTPServer()
require.Nil(s.T(), err) // FAILING
}
func (s *Example) TearDownSuite() {
if s.database != nil {
s.database.Close()
}
if s.httpServer != nil {
s.httpServer.Close()
}
}
In this case let's say s.database has been set but s.httpServer is failing to be set. It means closing properly the s.database will never be done whereas it exists.
I appreciate the workaround you provided but looks like a huge workaround to set everywhere.
@ernesto-jimenez @boyan-soubachov any thought on this? Do you have a recommended pattern?
To not break the library maybe we could use a variant?
suite.RunAssuringTearDown(t, testSuite)
Thank you,
My best guess as to why it's written like this is some applications may have tear down code that absolutely should not be run if the setup did not complete successfully. You could get around the current behavior easily by doing something like the below
In my opinion it should have been inverted. In most cases teardown is something you always want to happen no matter what. This proposed workaround could also easily be applied if your application shouldn't run teardown if setup fails.
For instance if you use something which starts background processes as part of an integration test. Then you might have started your background processes and then setup fails and teardown is never run, which in turn leaves orphaned background processes on your machine.
