Skip to content

proposal: cmd/test: add flag to run tests in parallel by default #24335

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

Closed
posener opened this issue Mar 10, 2018 · 5 comments
Closed

proposal: cmd/test: add flag to run tests in parallel by default #24335

posener opened this issue Mar 10, 2018 · 5 comments

Comments

@posener
Copy link

posener commented Mar 10, 2018

Summary

Following #21214

Add to the go test command a flag --default-parallel or similar, that would run all the tests in parallel - like the current t.Parallel() function behavior, until a t.Serial() or t.Serial(reason string) functions where invoked.

Details

When the new flag will be given, the test runner will invoke all the test functions and will skip the wait part before executing the next test function. Once a t.Serial function will be called in the test function, the go routing running the current test will be stopped and will be added to a serial tests queue. Once all the parallel test were done, test functions from the the serial queue will be invoked in a synchronous way.

When the flag --default-parallel won't be given, the t.Serial function will have no effect, and tests behavior will maintain it's current form.

Backward Compatibility

As far as I can see, this is not a breaking change.

Reason

The reason why this change can contribute to the language are given in #21214:

  • Faster tests: running all possible tests in parallel will result in shorter test time.
  • Explicitly: Having a programmer write an explicit reason for the test not to be able run in parallel: t.Serial("Modifies a database which is used by several tests").
  • Bugs detection: running tests in parallel is one of the ways to detect bugs in program design, which might be hidden in a serial invocation of the tests.
  • Less code/ Programmer friendlier: almost every test deceleration takes two lines of code: the test function deceleration and a following line t.Parallel(). It is a common case to see only the first, and the main reason for that is the the programmer forgot to type the second.
@gopherbot gopherbot added this to the Proposal milestone Mar 10, 2018
@mvdan
Copy link
Member

mvdan commented Mar 10, 2018

Explicitly

Could you not accomplish the same with a comment, or a func that does nothing?

Programmer friendlier

I believe that you could accomplish the same with a tool now - that is, a tool that would add t.Parallel() to all tests that don't call it yet.

I'm also worried that this would result in people using parallel tests a bit too agressively. For example, I've found that if one has to run thousands of subtests that are extremely cheap, marking each subtest as parallel actually hurts overall run-time slightly. I assume it's because of the extra goroutines and overhead.

Don't get me wrong, I also think that tests should in general be parallel. I'm simply not sure that this is worthwhile doing in the current testing package.

@posener
Copy link
Author

posener commented Mar 10, 2018

Could you not accomplish the same with a comment, or a func that does nothing?

Yes, a comment is an option - but that is optional, once a programmer must put t.Serial, it is not optional anymore. A func that does nothing? I don't understand any reason for that? or what kind of workaround is that - I think it won't make any sense.

I believe that you could accomplish the same with a tool now - that is, a tool that would add t.Parallel() to all tests that don't call it yet.

That is also an option, but to my opinion it is not a good solution, and kind of workaround to the problem I am raising here.

I'm also worried that this would result in people using parallel tests a bit too agressively. For example, I've found that if one has to run thousands of subtests that are extremely cheap, marking each subtest as parallel actually hurts overall run-time slightly. I assume it's because of the extra goroutines and overhead.

  • You might have a point here, though an actual benchmark would be a better case.
  • A solution for someone that finds this issue is to use the existing -parallel flag.
  • As one put today in the common case a t.Parallel, on the case you present here one could put a t.Serial for that case.

Don't get me wrong, I also think that tests should in general be parallel. I'm simply not sure that this is worthwhile doing in the current testing package.

Thanks! What do you mean current testing package? what is missing in the current testing package that we need to make this happen?

@ianlancetaylor
Copy link
Contributor

In general, different tests can only run in parallel if they are designed to work that way. So the time to decide whether to run a test in parallel is when writing the test. At that point you can write t.Parallel(). If tests are not designed to run in parallel, then running them in parallel anyhow will tend to produce rare bugs that will be hard to diagnose.

@posener
Copy link
Author

posener commented Mar 13, 2018

In general, different tests can only run in parallel if they are designed to work that way.

The idea is that tests should be designed to run in parallel in the first place. Why not design them in that way?

At that point you can write t.Parallel().

If we were to write the test package from scratch, will this be the chosen solution for running tests in parallel? I think that this is not unequivocal...

If tests are not designed to run in parallel, then running them in parallel anyhow will tend to produce rare bugs that will be hard to diagnose.

On the other hand, running tests in parallel that find bugs in the tested program because they ran in parallel can save a lot of time.

@ianlancetaylor
Copy link
Contributor

By all means design your tests to be parallel. But the Go project is not going to take the stance that all tests should be designed to be parallel.

If we were to write the test package from scratch, will this be the chosen solution for running tests in parallel?

That is the decision we have made, yes.

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

4 participants