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.

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

testing: API for deferring a cleanup step when subtests use Parallel #31651

Open
RaduBerinde opened this issue Apr 24, 2019 · 2 comments

Comments

Projects
None yet
2 participants
@RaduBerinde
Copy link
Contributor

commented Apr 24, 2019

A relatively common pattern is to run a test that has multiple subtests that can be run in parallel:

func TestSomething(t *testing.T) {
  t.Run("case1", func (t *testing.T) { t.Parallel(); ... })
  t.Run("case2", func (t *testing.T) { t.Parallel(); ... })
}

But what if the parent test needs to set something up and clean it up at the end? You'd be tempted to do:

func TestSomething(t *testing.T) {
  setup()
  defer cleanup()

  t.Run("case1", func (t *testing.T) { t.Parallel(); ... })
  t.Run("case2", func (t *testing.T) { t.Parallel(); ... })
}

Unfortunately this is not correct - cleanup will run too early, before the subtests finish (in fact, before they even get passed the Parallel() call).

Most CockroachDB tests use a defer leaktest.AfterTest(t)() call in the beginning that checks for stray goroutines; using Parallel() in subtests breaks that pattern.

My proposal is to add an API to T that runs a cleanup step after all subtests finish; defer cleanup() would become t.Defer(cleanup()) or similar.

Also, this gotcha should be called out in the documentation (#23368 is related).

@RaduBerinde

This comment has been minimized.

Copy link
Contributor Author

commented Apr 24, 2019

Ah.. I found this in the higher-level documentation:

func TestTeardownParallel(t *testing.T) {
    // This Run will not return until the parallel tests finish.
    t.Run("group", func(t *testing.T) {
        t.Run("Test1", parallelTest1)
        t.Run("Test2", parallelTest2)
        t.Run("Test3", parallelTest3)
    })
    // <tear-down code>
}

This is a good workaround for this problem (though it would still be better if there was an explicit API). Either way, I think there should be a pointer to this information in the documentation of Parallel.

@katiehockman

This comment has been minimized.

Copy link
Contributor

commented Apr 29, 2019

/cc @mpvl

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.