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: cmd/go: add a method to output test that might be worth running in parallel #65770

Open
qiulaidongfeng opened this issue Feb 17, 2024 · 4 comments
Milestone

Comments

@qiulaidongfeng
Copy link
Contributor

qiulaidongfeng commented Feb 17, 2024

Proposal Details

I am created https://go-review.googlesource.com/c/go/+/564995 and https://go-review.googlesource.com/c/go/+/564576, in this process, discover a way to find test possible worth running in parallel, which can be done automatically with go test .

This method is go test -v gets the run time for each of the tests, then excludes tests that have already been run in parallel and tests that have taken a short time, and the rest are tests that might be worth running in parallel.

For example:

get from go test -v runtime

=== RUN   TestSmhasherTwoNonzero
--- PASS: TestSmhasherTwoNonzero (9.09s)

This can determine that TestSmosherTwoNonzero may be worth running in parallel.

However, based on the experience gained in the aforementioned CL, it is more convenient for go test to perform this search.

because there may be many tests, there may be log and skip outputs, and there are clear rules for performing this lookup with go test.

Specific proposal content
Add a flag, name undetermined, use maybeparallel as placeholder for now, go test -maybeparallel only provides this semantics, which reports tests that may be worth running in parallel.
One way to achieve this is outputs the name and time of the test that did not run in parallel and took longer than 0.4 seconds.

Worth discussing:
Maybe also should add a flag, set test that is not complete after a certain time, go test -maybeparallel only output these test name and test run time.

@gopherbot gopherbot added this to the Proposal milestone Feb 17, 2024
@Jorropo
Copy link
Member

Jorropo commented Feb 17, 2024

That an interesting idea, should -maybeparallel analyse tests that can run in parallel statically ?

For example it wouldn't report tests that modify global state, but would report some test that only modify local objects.

@qiulaidongfeng
Copy link
Contributor Author

go test -maybeparallel the advantages and disadvantages of static analysis and analysis after running tests are as follows:

Static analysis has the advantage of avoiding testing that reports changes to global state, which often should not be parallelized, such as runtime_test.TestChan.
The disadvantage is that static analysis requires judging the time required for test runs, as if the time required for test runs is very small, it may not be worth parallelizing, making the implementation more complex.
The time required for test runs can be determined by running the test run without any need to analyze the code.

The advantage of analyzing after running the test is that it is simple to implement, and the implementation can be to determine whether the time taken exceeds a certain limit after the test is run.
The disadvantage is that some tests that change the global state may be reported.

Note that go test -maybeparallel only provides this semantics, which reports tests that may be worth running in parallel.
I think any implementation that can provide this semantic is acceptable.

@adonovan
Copy link
Member

adonovan commented Feb 19, 2024

Each call to T.Parallel is an assertion by the test's maintainer that the test is concurrency-safe with respect to all other tests marked as Parallel. In the absence of the assertion, the test runner must assume that the test is not concurrency-safe, as is often the case.

Inviting users to run tests in parallel that have not been designed for concurrency safety seems to me a bit like inviting them to test the top speed of a car whose seatbelts, brakes, and airbags have been removed to reduce weight. Faster but less safe is not the Go way.

@Jorropo it wouldn't report tests that modify global state, but would report some test that only modify local objects.

I don't think this is practically feasible within the current state of the art of static analysis.

@qiulaidongfeng
Copy link
Contributor Author

This proposal is not an invitation for users to run tests that are not designed for concurrency safety in parallel, but to help users find tests that may be worth parallelizing.

In packages with a large number of tests, such as runtime, it is not easy to manually analyze go test -v to find tests that might be worth running in parallel because there is too much invalid information such as a test running 0.00s.

If the application can identify tests that might be worth running in parallel, the developer can use the energy saved to study whether the tests should be run in parallel, rather than spend the energy searching for valid information in a pile of invalid information.

Ideal use case for this proposal:

  1. In a package with many tests,the CPU usage of the test is low, and there is no certainty that all tests are not worth parallel.
  2. run go test -maybeparallel
    (flag name undetermined, use maybeparallel as placeholder for now)
  3. Get test names and times that might be worth running in parallel
  4. Analyze the result.
    Exclude like runtime.TestChan that should not be parallel test because defer runtime.GOMAXPROCS(runtime.GOMAXPROCS(4)) or for other reasons.
    Find like the runtime.TestSmhasherTwoNonzero that can parallel, worthy of parallel, there is no parallel test.

@seankhliao seankhliao added the GoCommand cmd/go label Feb 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Incoming
Development

No branches or pull requests

5 participants