moricho / tparallel

tparallel finds inappropriate usage of `t.Parallel()` method in your Go test codes
MIT License
24 stars 4 forks source link

Run sequential sub-tests inside parallel tests #14

Open gnuletik opened 2 years ago

gnuletik commented 2 years ago

Hi,

Thanks for this linter! I just discovered it by upgrading golangci-lint. However, when running it on our codebase, it seems that some use case should not be an error.

We may want run multiple Test in parallel which contains sequential sub-tests.

For example:

func TestGet(t *testing.T) {
  t.Paralell()
  // creates a new isolated database like
  // https://github.com/allaboutapps/integresql
  db := createNewDB(t)

  t.Run("GetEmpty", func () {
    // make a GET request and check that it returns an empty slice
  })

  t.Run("GetOne", func() {
    // create a row in database
    // make a GET request and check that it returns a slice of size 1
  })
}

func TestUpdate(t *testing.T) {
  t.Parallel()
  db := createNewDB(t)

  t.Run("UpdateFirstName", func() {
    // make a PATCH request and check that only the first name was updated
  })

  t.Run("UpdateLastName", func () {
    // make a PATCH request and check that only the last name was updated
  })
}
TestGet's subtests should call t.Parallel (tparallel)
TestUpdate's subtests should call t.Parallel (tparallel)

But this example does not seems to be an inappropriate usage of t.Parallel(). We want to be able to run TestGet and TestUpdate in parallel (because they run with their own database) without running their subtests in parallel (because they use the same database instance).

We could also have use-cases for sequential tests with parallel sub-tests.

Or am I missing something?

Thanks!

POlczak-ITTouch commented 1 year ago

I've independently came up with the same conclusion and stumbled upon this ticket after a lot of googling.

In our codebase it's common to build many mocked structures in the setup section of the top-level test functions. The mocks can then be used by all of their sub-tests, but only if the sub-tests are sequential, so that they don't write in parallel to the same structures. However the number of test functions within a package is quite big so we want to parallelize them.

Therefor the linter check:

is something we have to disable. And since there is no configuration of this linter we have to disable the second rule as well:

despite it actually being really useful for us.