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

proposal: testing: add a ParallelCount func that reports the go test -p flag value #50719

Closed
jeffwidman opened this issue Jan 20, 2022 · 3 comments

Comments

@jeffwidman
Copy link
Contributor

jeffwidman commented Jan 20, 2022

Summary:
Add a helper to the testing package that exposes the value of the go test -p flag. This is similar to testing.Verbose() or testing.Short() functions that expose the values of other flags.

Problem:
By default, go test runs with with the -p flag defaulting to GOMAXPROCS, normally the number of CPUs available. This results in tests within separate packages running in parallel, which is great, I get fast test runs.

However, occasionally I am forced to write tests that are racy with concurrent tests. For example when two tests in separate packages each call t.Setenv() to set the same env var.

The workaround is to run go test -p=1 to force the tests to run sequentially. But that drastically slows down the tests, plus most of the time I actually want tests to run concurrently to catch unexpected races etc.

Often there's just two or three tests out of hundreds within a package that need to be run sequentially. So it's not feasible to use the -run flag to exclude all the other tests... instead I end up with something like this in my code:

if os.GetEnv("TESTS_ARE_BEING_RUN_SERIALLY") != "true" {
	t.Skip("Skipping " + t.Name() + " as it is racy when run concurrently with other tests due to using t.Setenv().")
}

which I then have to invoke in CI via something like:

- go test ./...
- TESTS_ARE_BEING_RUN_SERIALLY="true" go test -p=1 -run <regex of tests that are racy> ./...

The TESTS_ARE_BEING_RUN_SERIALLY env var is clunky, and certainly confusing if you're not familiar with the code.

So it'd be great if the testing package had a helper similar to testing.Verbose() or testing.Short() functions that exposed the value of the -p flag:

func ParrallelCount() int {
	// return value of `-p` flag to `go test`
}

which I would use in my test like this:

if testing.ParrallelCount() > 1 {
	t.Skip("Skipping " + t.Name() + " as it is racy when run concurrently with other tests due to using t.Setenv().")
}

Notes:

  • For my use case, a bool would suffice, I don't actually care how many, just whether it's more than 1 or not. But I suspect there may be use cases where knowing the actual number of parallel test runners would be useful.
  • The parallel count of running test runners can change dynamically--ie, if some packages finish their tests before others. For my purposes, I don't want the "real time" count, but rather the max-allowed which is what -p flag controls.
  • The scope of this proposed helper is confusing because the parallel count of running tests is affected by both go test -p=n and t.Parallel() helper. Since t.Parallel() does nothing if -p=1 (at least that's my understanding), then for my purposes, I only care about the -p flag setting.
  • The naming is confusing... if I saw a testing.ParallelCount() helper, it's easy to think it's related to the t.Parallel() helper... But I struggled to come up with a more appropriate name. Perhaps the scoping of a pkg level helper (testing) vs a test-level helper (t) is enough disambiguation?
@gopherbot gopherbot added this to the Proposal milestone Jan 20, 2022
@jeffwidman jeffwidman changed the title proposal: Add a testing.ParallelCount() func that reports the number of parallel tests running proposal: Add a testing.ParallelCount() func that reports the go test -p flag value Jan 20, 2022
@bcmills
Copy link
Contributor

bcmills commented Jan 20, 2022

However, occasionally I am forced to write tests that are racy with concurrent tests. For example when two tests in separate packages each call t.Setenv() to set the same env var.

Tests in separate packages run in separate processes; Setenv applies per-process, not globally. Moreover, even if go test reports the number of processes it is running in parallel, you still don't know how many invocations of go test the user is running in parallel.

If a test needs to access a non-hermetic resource, it should coordinate access to that resource explicitly with other tests. For example, a test that needs access to a shared, mutable database, or a shared, mutable file, might need to lock the relevant records and/or files explicitly. (Better still, write the tests such that they do not mutate global resources!)

@jeffwidman
Copy link
Contributor Author

Tests in separate packages run in separate processes; Setenv applies per-process, not globally.

Oh, I didn't realize that!

For example, a test that needs access to a shared, mutable database, or a shared, mutable file, might need to lock the relevant records and/or files explicitly. (Better still, write the tests such that they do not mutate global resources!)

Yes, but sometimes the reality is that it's not that easy.

@jeffwidman
Copy link
Contributor Author

Closing, because this isn't relevant for my use case since my shared state is within the process, not cross-process.

@ianlancetaylor ianlancetaylor changed the title proposal: Add a testing.ParallelCount() func that reports the go test -p flag value proposal: testing: add a ParallelCount func that reports the go test -p flag value Jan 20, 2022
@golang golang locked and limited conversation to collaborators Jan 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants