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.

Sign up for GitHub

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

Jump to bottom

proposal: testing: run tests in parallel by default #21214

Closed
posener opened this issue Jul 29, 2017 · 7 comments
Closed

proposal: testing: run tests in parallel by default #21214

posener opened this issue Jul 29, 2017 · 7 comments
Labels
FrozenDueToAge Proposal v2 A language change or incompatible library change
Milestone
Proposal

Comments

@posener
Copy link

posener commented Jul 29, 2017 •
edited

The testing framework of go is powerful, one of it's strong points is it's natural ability to run tests in parallel.
I assert that most tests should be able to run in parallel, and if not - a detailed explanation should be given why this test should run on its own. I can gather several reasons for this proposal:

  • 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.

I can propose two ways to accept this change:

  1. Breaking change: save it to Go2, since Go1 is good enough as it is. 😄
  2. Can add an additional flag for the test command that sets the default behavior to parallel, unless the Serial function was called on a test.
@gopherbot gopherbot added this to the Proposal milestone Jul 29, 2017
@ianlancetaylor ianlancetaylor added the v2 A language change or incompatible library change label Jul 29, 2017
@bcmills
Copy link
Contributor

bcmills commented Aug 4, 2017 •
edited

See also #17751 and the attached boilerplate "parallelize" changes.

@ianlancetaylor
Copy link
Contributor

It would be very difficult to make this change with the existing testing package, as that would be certain to break many existing tests.

Writing t.Parallel() is not hard.

This might be something to consider in a new testing package, but I don't see any path from the current package to a place where tests are run in parallel by default.

I could imagine permitting people to say t.AllParallel() in their TestMain function, but that ought to be a separate proposal.

Closing this issue.

@posener
Copy link
Author

posener commented Mar 9, 2018

Writing t.Parallel() is not hard.

Right, but as said in the proposal: one might forget it. and it is the same line wring over and over again. Also, when not used, race detection are less likely to happened.

This might be something to consider in a new testing package, but I don't see any path from the current package to a place where tests are run in parallel by default.

What do you mean by "new testing package"?

I could imagine permitting people to say t.AllParallel() in their TestMain function, but that ought to be a separate proposal.

What is TestMain function?

Closing this issue.

Closing this means it won't be considered to what is called "Go2"?

Thanks!

@mvdan
Copy link
Member

mvdan commented Mar 9, 2018

What is TestMain function?

https://golang.org/pkg/testing/#hdr-Main

@ianlancetaylor
Copy link
Contributor

What do you mean by "new testing package"?

For Go 2 it is possible that we could create a new testing package under a different import name. However, there are no current plans to do so.

Closing this means it won't be considered to what is called "Go2"?

Yes. Sorry.

Go 2 has to be largely backward compatible with Go 1. It can only break compatibility where necessary. This benefit of this change is not high enough.

@posener
Copy link
Author

posener commented Mar 9, 2018

OK, thanks @ianlancetaylor .
One last question, sorry about your time. What about an additional flag to go test, as proposed in no. 2 in the proposal? It is not breaking anything as far as I can see, and I am willing to implement it if it is acceptable.

@ianlancetaylor
Copy link
Contributor

ianlancetaylor commented Mar 9, 2018 •
edited

An additional flag seems possible and doesn't break compatibility. Why don't you open a separate proposal for that and we can see what the proposal review committee thinks. Thanks.

@posener posener mentioned this issue Mar 10, 2018
Closed
@talkol talkol mentioned this issue Aug 9, 2018
Merged
@golang golang locked and limited conversation to collaborators Mar 9, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge Proposal v2 A language change or incompatible library change
Projects
None yet
Milestone
Proposal
Development

No branches or pull requests
5 participants
@posener @ianlancetaylor @mvdan @bcmills @gopherbot