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

gh-127933: Add option to run regression tests in parallel #128003

Merged
merged 11 commits into from
Feb 4, 2025

Conversation

colesbury
Copy link
Contributor

@colesbury colesbury commented Dec 16, 2024

This adds a new command line argument, --parallel-threads to the regression test runner to allow it to run individual tests in multiple threads in parallel in order to find multithreading bugs.

Some tests pass when run with --parallel-threads, but there's still a bunch more work before the entire suite passes:

  • Mark non-thread safe tests with @thread_unsafe
  • Tests that rely on support.gc_collect() are sensitive to threads because gc.collect() returns immediately without running the GC if it's already running in a different thread

This adds a new command line argument, `--parallel-threads` to the
regression test runner to allow it to run individual tests in multiple
threads in parallel in order to find multithreading bugs.
colesbury and others added 2 commits December 16, 2024 18:22
Co-authored-by: Tomas R. <tomas.roun8@gmail.com>
@colesbury colesbury requested review from Yhg1s and mpage January 13, 2025 19:05
Copy link
Contributor

@mpage mpage left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me. I'm a little unsure about marking tests as thread-unsafe at the source level. I worry a bit that the use of thread_unsafe / __unittest_thread_unsafe__ will create too much noise, but I think we can see how it goes.

barrier = threading.Barrier(self.num_threads)
threads = []
for case, r in zip(cases, results):
thread = threading.Thread(target=self.run_worker,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be nice to give these threads an informative name, something containing the testcase and the Nth thread it is.


# Note: We can't call result.addError, result.addFailure, etc. because
# we no longer have the original exception, just the string format.
for r in results:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like something that might be worth adding to unittest.result.TestResult, something to aggregate multiple results. (Not for this PR, but it might be a nice small feature for someone to work on.)

result.addSuccess(self)

# Note: We can't call result.addError, result.addFailure, etc. because
# we no longer have the original exception, just the string format.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW, another reason not to call those is that they raise right away if failfast is set.

@@ -0,0 +1,3 @@
Add an option ``--parallel-threads=N`` to the regression test runner that
runs individual tests in multiple threads in parallel in order to find
concurrency bugs.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this (or other documentation) mention that most of the testsuite isn't appropriately reviewed/marked yet?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've updated the blurb. It might be worth adding something to the devguide when this gets further along.

@colesbury colesbury merged commit e5f10a7 into python:main Feb 4, 2025
47 checks passed
@colesbury colesbury deleted the gh-127933-parallel-tests branch February 4, 2025 22:45
srinivasreddy pushed a commit to srinivasreddy/cpython that referenced this pull request Feb 7, 2025
…ongh-128003)

This adds a new command line argument, `--parallel-threads` to the
regression test runner to allow it to run individual tests in multiple
threads in parallel in order to find multithreading bugs.

Some tests pass when run with `--parallel-threads`, but there's still
more work before the entire suite passes.
cmaloney pushed a commit to cmaloney/cpython that referenced this pull request Feb 8, 2025
…ongh-128003)

This adds a new command line argument, `--parallel-threads` to the
regression test runner to allow it to run individual tests in multiple
threads in parallel in order to find multithreading bugs.

Some tests pass when run with `--parallel-threads`, but there's still
more work before the entire suite passes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tests Tests in the Lib/test dir topic-free-threading
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants