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

Refactor test.libregrtest #109162

Closed
vstinner opened this issue Sep 8, 2023 · 4 comments
Closed

Refactor test.libregrtest #109162

vstinner opened this issue Sep 8, 2023 · 4 comments
Labels
tests Tests in the Lib/test dir

Comments

@vstinner
Copy link
Member

vstinner commented Sep 8, 2023

I propose to refactor test.libregrtest to make it easier to maintain and to prepare adding type annotations.

The regrtest project has a long history. It was added in 1996 by commit 152494a. When it was created, it was 170 lines long and had 4 command line options: -v (verbose), -q (quiet), -g (generate) and -x (exclude). Slowly, it got more and more features:

  • Better command line interface with argparse (it used getopt at the begining)
  • Run tests in parallel with multiple processes (this code caused me a lot of headaches!)
  • Detect when the "environment" is altered: warnings filters, loggers, etc.
  • Re-run failed tests in verbose mode (now they are run in fresh processes)
  • Detect memory, reference and file descriptor leaks
  • Detect leaked files by creating a temporary directory for each test worker process
  • Best effort to restore the machine to its previous state: wait until threads and processes complete, remove temporary files, etc.
  • etc.

Some of these features were implemented in test.support + test.libregrtest.

A few years ago, I decided to split the giant mono regrtest.py file (for example, it was 2 200 lines of Python code in Python 2.7) into sub-files (!). To make it possible, I passed ns argument which is a bag of "global variables" (technically, it's a Namespace class, see cmdline.py).

The problem is that for type annotation, it's very unclear what a Namespace contains. It may or may not have arguments (see my commit message of this PR: Add missing attributes to Namespace: coverage, threshold, wait.), argument types are weakly defined, etc. Moreover, ns is not only used to "get" variables, but also to set variables! For example, find_tests() overrides ns.args. How is it possible to know which ns attributes are used? Are they "read-only"? We don't know just by reading a function prototype.

This large refactoring cleans up everything in a serie of small changes to pass simple types like bool, str or tuple[str]. It's easier to guess the purpose of a function and its behavior just from its prototype.

I tried to create only short files, the longest is still sadly main.py with 891 lines.

$ wc -l *.py|sort -n
     2 __init__.py
    56 pgo.py
   124 win_utils.py
   159 setup.py
   202 refleak.py
   307 utils.py
   329 save_env.py
   451 cmdline.py
   575 runtest.py
   631 runtest_mp.py
   891 main.py
  3727 total

To understand where the ns magic bag of global variables, look at regrtest.py monster in Python 2.7. Its main() functions defines not less than 34 functions inside the main() function! Variables are defined in the main() prototype!

def main(tests=None, testdir=None, verbose=0, quiet=False,
         exclude=False, single=False, randomize=False, fromfile=None,
         findleaks=False, use_resources=None, trace=False, coverdir='coverage',
         runleaks=False, huntrleaks=False, verbose2=False, print_slow=False,
         random_seed=None, use_mp=None, verbose3=False, forever=False,
         header=False, pgo=False, failfast=False, match_tests=None):

I supposed that it was designed to be able to use regrtest as an API: pass parameters to the main() function, without having to use the command line interface. In the main branch, this feature is still supported, the magic **kwargs bag:

def main(tests=None, **kwargs):
    Regrtest().main(tests=tests, **kwargs)

Linked PRs

vstinner added a commit to vstinner/cpython that referenced this issue Sep 8, 2023
* main() now calls _parse_args() and pass 'ns' to Regrtest
  constructor.  Remove kwargs argument from Regrtest.main().
* _parse_args() checks ns.huntrleaks.
* set_temp_dir() is now responsible to call expanduser().
* Regrtest.main() sets self.tests earlier.
* Add TestTuple and TestList types.
* Rename MatchTests to MatchTestList and rename MatchTestsDict
  to MatchTestDict.
* RunTests.tests type becomes TestTuple.
vstinner added a commit to vstinner/cpython that referenced this issue Sep 8, 2023
* main() now calls _parse_args() and pass 'ns' to Regrtest
  constructor.  Remove kwargs argument from Regrtest.main().
* _parse_args() checks ns.huntrleaks.
* set_temp_dir() is now responsible to call expanduser().
* Regrtest.main() sets self.tests earlier.
* Add TestTuple and TestList types.
* Rename MatchTests to FilterTuple and rename MatchTestsDict
  to FilterTestDict.
* TestResult.get_rerun_match_tests() return type
  is now FilterTuple: return a tuple instead of a list.
  RunTests.tests type becomes TestTuple.
vstinner added a commit that referenced this issue Sep 8, 2023
* main() now calls _parse_args() and pass 'ns' to Regrtest
  constructor.  Remove kwargs argument from Regrtest.main().
* _parse_args() checks ns.huntrleaks.
* set_temp_dir() is now responsible to call expanduser().
* Regrtest.main() sets self.tests earlier.
* Add TestTuple and TestList types.
* Rename MatchTests to FilterTuple and rename MatchTestsDict
  to FilterTestDict.
* TestResult.get_rerun_match_tests() return type
  is now FilterTuple: return a tuple instead of a list.
  RunTests.tests type becomes TestTuple.
vstinner added a commit to vstinner/cpython that referenced this issue Sep 8, 2023
* Regrtest.__init__() now copies 'ns' namespace attributes to
  Regrtest attributes. Regrtest match_tests and ignore_tests
  attributes have type FilterTuple (tuple), instead of a list.
* Add RunTests.copy(). Regrtest._rerun_failed_tests() now uses
  RunTests.copy().
* Replace Regrtest.all_tests (list) with Regrtest.first_runtests
  (RunTests).
vstinner added a commit to vstinner/cpython that referenced this issue Sep 8, 2023
* Regrtest.__init__() now copies 'ns' namespace attributes to
  Regrtest attributes. Regrtest match_tests and ignore_tests
  attributes have type FilterTuple (tuple), instead of a list.
* Add RunTests.copy(). Regrtest._rerun_failed_tests() now uses
  RunTests.copy().
* Replace Regrtest.all_tests (list) with Regrtest.first_runtests
  (RunTests).
* Make random_seed maximum 10x larger (9 digits, instead of 8).
vstinner added a commit that referenced this issue Sep 8, 2023
* Regrtest.__init__() now copies 'ns' namespace attributes to
  Regrtest attributes. Regrtest match_tests and ignore_tests
  attributes have type FilterTuple (tuple), instead of a list.
* Add RunTests.copy(). Regrtest._rerun_failed_tests() now uses
  RunTests.copy().
* Replace Regrtest.all_tests (list) with Regrtest.first_runtests
  (RunTests).
* Make random_seed maximum 10x larger (9 digits, instead of 8).
vstinner added a commit to vstinner/cpython that referenced this issue Sep 8, 2023
Refator Regrtest class:

* Rename finalize() finalize_tests().
* Pass tracer to run_test() and finalize_tests(). Remove Regrtest.tracer.
* run_test() does less things: move code to its caller.
vstinner added a commit that referenced this issue Sep 9, 2023
Refator Regrtest class:

* Rename finalize() finalize_tests().
* Pass tracer to run_test() and finalize_tests(). Remove Regrtest.tracer.
* run_test() does less things: move code to its caller.
vstinner added a commit to vstinner/cpython that referenced this issue Sep 9, 2023
* Rename --worker-args command line option to --worker-json.
* Rename _parse_worker_args() to _parse_worker_json().
* WorkerJob:

  * Add runtests attribute
  * Remove test_name and rerun attribute

* Rename run_test_in_subprocess() to create_worker_process().
* Rename run_tests_worker() to worker_process().
* create_worker_process() uses json.dump(): write directly JSON to
  stdout.
* Convert MultiprocessResult to a frozen dataclass.
* Rename RunTests.match_tests to RunTests.match_tests_dict.
vstinner added a commit that referenced this issue Sep 9, 2023
* Rename --worker-args command line option to --worker-json.
* Rename _parse_worker_args() to _parse_worker_json().
* WorkerJob:

  * Add runtests attribute
  * Remove test_name and rerun attribute

* Rename run_test_in_subprocess() to create_worker_process().
* Rename run_tests_worker() to worker_process().
* create_worker_process() uses json.dump(): write directly JSON to
  stdout.
* Convert MultiprocessResult to a frozen dataclass.
* Rename RunTests.match_tests to RunTests.match_tests_dict.
vstinner added a commit to vstinner/cpython that referenced this issue Sep 9, 2023
* Rename runtest() to run_single_test().
* Pass runtests to run_single_test().
* Add type annotation to Regrtest attributes. Add missing attributes
  to Namespace.
* Add attributes to Regrtest and RunTests:

  * fail_fast
  * ignore_tests
  * match_tests
  * output_on_failure
  * pgo
  * pgo_extended
  * timeout

* Get pgo from 'runtests', rather than from 'ns'.
* Remove WorkerJob.match_tests.
* setup_support() now gets pgo_extended from runtests.
* save_env(): change parameter order, pass test_name first.
* Add setup_test_dir() function.
* Pass runtests to setup_tests().
vstinner added a commit that referenced this issue Sep 9, 2023
* Rename runtest() to run_single_test().
* Pass runtests to run_single_test().
* Add type annotation to Regrtest attributes. Add missing attributes
  to Namespace.
* Add attributes to Regrtest and RunTests:

  * fail_fast
  * ignore_tests
  * match_tests
  * output_on_failure
  * pgo
  * pgo_extended
  * timeout

* Get pgo from 'runtests', rather than from 'ns'.
* Remove WorkerJob.match_tests.
* setup_support() now gets pgo_extended from runtests.
* save_env(): change parameter order, pass test_name first.
* Add setup_test_dir() function.
* Pass runtests to setup_tests().
vstinner added a commit to vstinner/cpython that referenced this issue Sep 9, 2023
* Rename dash_R() runtest_refleak(). The function now gets
  huntrleaks and quiet arguments, instead of 'ns' argument.
* Add attributes to Regrtest and RunTests:

  * verbose
  * quiet
  * huntrleaks
  * test_dir

* Add HuntRefleak class.
vstinner added a commit to vstinner/cpython that referenced this issue Sep 9, 2023
* Rename dash_R() runtest_refleak(). The function now gets
  huntrleaks and quiet arguments, instead of 'ns' argument.
* Add attributes to Regrtest and RunTests:

  * verbose
  * quiet
  * huntrleaks
  * test_dir

* Add HuntRefleak class.
@vstinner
Copy link
Member Author

vstinner commented Sep 9, 2023

cc @serhiy-storchaka @AlexWaygood: I'm pushing my large refactoring a serie of small changes. I'm half way I suppose. I will keep you in touch ;-)

vstinner added a commit that referenced this issue Sep 9, 2023
* Rename dash_R() runtest_refleak(). The function now gets
  huntrleaks and quiet arguments, instead of 'ns' argument.
* Add attributes to Regrtest and RunTests:

  * verbose
  * quiet
  * huntrleaks
  * test_dir

* Add HuntRefleak class.
@AlexWaygood AlexWaygood added the tests Tests in the Lib/test dir label Sep 9, 2023
vstinner added a commit to vstinner/cpython that referenced this issue Sep 9, 2023
Decode also HuntRefleak() object inside the RunTests object.

Add an unit test on huntrleaks with multiprocessing (-R -jN).
vstinner added a commit that referenced this issue Sep 9, 2023
Decode also HuntRefleak() object inside the RunTests object.

Add an unit test on huntrleaks with multiprocessing (-R -jN).
vstinner added a commit to vstinner/cpython that referenced this issue Sep 9, 2023
* Add attributes to Regrtest and RunTests:

  * gc_threshold
  * memory_limit
  * python_cmd
  * use_resources

* Remove WorkerJob class. Add as_json() and from_json() methods to
  RunTests. A worker process now only uses RunTests for all
  parameters.
* Add tests on support.set_memlimit() in test_support. Create
  _parse_memlimit() and also adds tests on it.
* Remove 'ns' parameter from runtest.py.
vstinner added a commit that referenced this issue Sep 9, 2023
* Add attributes to Regrtest and RunTests:

  * gc_threshold
  * memory_limit
  * python_cmd
  * use_resources

* Remove WorkerJob class. Add as_json() and from_json() methods to
  RunTests. A worker process now only uses RunTests for all
  parameters.
* Add tests on support.set_memlimit() in test_support. Create
  _parse_memlimit() and also adds tests on it.
* Remove 'ns' parameter from runtest.py.
vstinner added a commit to vstinner/cpython that referenced this issue Sep 10, 2023
Add new worker.py file:

* Move create_worker_process() and worker_process() to this file.
* Add main() function to worker.py. create_worker_process() now
  runs the command: "python -m test.libregrtest.worker JSON".
* create_worker_process() now starts the worker process in the
  current working directory. Regrtest now gets the absolute path of
  the reflog.txt filename: -R command line option filename.
* Remove --worker-json command line option.
  Remove test_regrtest.test_worker_json().

Related changes:

* Add write_json() and from_json() methods to TestResult.
* Rename select_temp_dir() to get_temp_dir() and move it to utils.
* Rename make_temp_dir() to get_work_dir() and move it to utils.
  It no longer calls os.makedirs(): Regrtest.main() now calls it.
* Move fix_umask() to utils. The function is now called by
  setup_tests().
* Move StrPath to utils.
* Add exit_timeout() context manager to utils.
* RunTests: Replace junit_filename (StrPath) with use_junit (bool).
vstinner added a commit to vstinner/cpython that referenced this issue Sep 10, 2023
Add new worker.py file:

* Move create_worker_process() and worker_process() to this file.
* Add main() function to worker.py. create_worker_process() now
  runs the command: "python -m test.libregrtest.worker JSON".
* create_worker_process() now starts the worker process in the
  current working directory. Regrtest now gets the absolute path of
  the reflog.txt filename: -R command line option filename.
* Remove --worker-json command line option.
  Remove test_regrtest.test_worker_json().

Related changes:

* Add write_json() and from_json() methods to TestResult.
* Rename select_temp_dir() to get_temp_dir() and move it to utils.
* Rename make_temp_dir() to get_work_dir() and move it to utils.
  It no longer calls os.makedirs(): Regrtest.main() now calls it.
* Move fix_umask() to utils. The function is now called by
  setup_tests().
* Move StrPath to utils.
* Add exit_timeout() context manager to utils.
* RunTests: Replace junit_filename (StrPath) with use_junit (bool).
vstinner added a commit to vstinner/cpython that referenced this issue Sep 10, 2023
Add new worker.py file:

* Move create_worker_process() and worker_process() to this file.
* Add main() function to worker.py. create_worker_process() now
  runs the command: "python -m test.libregrtest.worker JSON".
* create_worker_process() now starts the worker process in the
  current working directory. Regrtest now gets the absolute path of
  the reflog.txt filename: -R command line option filename.
* Remove --worker-json command line option.
  Remove test_regrtest.test_worker_json().

Related changes:

* Add write_json() and from_json() methods to TestResult.
* Rename select_temp_dir() to get_temp_dir() and move it to utils.
* Rename make_temp_dir() to get_work_dir() and move it to utils.
  It no longer calls os.makedirs(): Regrtest.main() now calls it.
* Move fix_umask() to utils. The function is now called by
  setup_tests().
* Move StrPath to utils.
* Add exit_timeout() context manager to utils.
* RunTests: Replace junit_filename (StrPath) with use_junit (bool).
vstinner added a commit that referenced this issue Sep 10, 2023
Add new worker.py file:

* Move create_worker_process() and worker_process() to this file.
* Add main() function to worker.py. create_worker_process() now
  runs the command: "python -m test.libregrtest.worker JSON".
* create_worker_process() now starts the worker process in the
  current working directory. Regrtest now gets the absolute path of
  the reflog.txt filename: -R command line option filename.
* Remove --worker-json command line option.
  Remove test_regrtest.test_worker_json().

Related changes:

* Add write_json() and from_json() methods to TestResult.
* Rename select_temp_dir() to get_temp_dir() and move it to utils.
* Rename make_temp_dir() to get_work_dir() and move it to utils.
  It no longer calls os.makedirs(): Regrtest.main() now calls it.
* Move fix_umask() to utils. The function is now called by
  setup_tests().
* Move StrPath to utils.
* Add exit_timeout() context manager to utils.
* RunTests: Replace junit_filename (StrPath) with use_junit (bool).
vstinner added a commit to vstinner/cpython that referenced this issue Sep 10, 2023
* Add single.py and result.py files.
* Rename runtest.py to runtests.py.
* Move run_single_test() function and its helper functions to
  single.py.
* Move remove_testfn(), abs_module_name() and normalize_test_name()
  to utils.py.
* Move setup_support() to setup.py.
* Move type hints like TestName to utils.py.
* Rename runtest.py to runtests.py.
vstinner added a commit that referenced this issue Sep 11, 2023
* Add single.py and result.py files.
* Rename runtest.py to runtests.py.
* Move run_single_test() function and its helper functions to
  single.py.
* Move remove_testfn(), abs_module_name() and normalize_test_name()
  to utils.py.
* Move setup_support() to setup.py.
* Move type hints like TestName to utils.py.
* Rename runtest.py to runtests.py.
vstinner added a commit to vstinner/cpython that referenced this issue Sep 11, 2023
* Pass results, quiet and pgo to Logger constructor.
* Move display_progress() method from Regrtest to Logger.
* No longer pass Regrtest to RunWorkers, but logger and results.
vstinner added a commit that referenced this issue Sep 11, 2023
* Pass results, quiet and pgo to Logger constructor.
* Move display_progress() method from Regrtest to Logger.
* No longer pass Regrtest to RunWorkers, but logger and results.
vstinner added a commit to vstinner/cpython that referenced this issue Sep 11, 2023
* Rename runtest_mp.py to run_workers.py
* Move exit_timeout() and temp_cwd() context managers from
  Regrtest.main() to Regrtest.run_tests(). Actions like --list-tests
  or --list-cases don't need these protections.
* Regrtest: remove selected and tests attributes. Pass 'selected' to
  list_tests(), list_cases() and run_tests(). display_result() now
  expects a TestTuple, instead of TestList.
* Rename setup_tests() to setup_process() and rename setup_support()
  to setup_tests().
* Move _adjust_resource_limits() to utils and rename it to
  adjust_rlimit_nofile().
* Move replace_stdout() to utils.
* Fix RunTests.verbose type: it's an int.
vstinner added a commit to vstinner/cpython that referenced this issue Sep 11, 2023
* Rename runtest_mp.py to run_workers.py
* Move exit_timeout() and temp_cwd() context managers from
  Regrtest.main() to Regrtest.run_tests(). Actions like --list-tests
  or --list-cases don't need these protections.
* Regrtest: remove selected and tests attributes. Pass 'selected' to
  list_tests(), list_cases() and run_tests(). display_result() now
  expects a TestTuple, instead of TestList.
* Rename setup_tests() to setup_process() and rename setup_support()
  to setup_tests().
* Move _adjust_resource_limits() to utils and rename it to
  adjust_rlimit_nofile().
* Move replace_stdout() to utils.
* Fix RunTests.verbose type: it's an int.
vstinner added a commit that referenced this issue Sep 11, 2023
* Rename runtest_mp.py to run_workers.py
* Move exit_timeout() and temp_cwd() context managers from
  Regrtest.main() to Regrtest.run_tests(). Actions like --list-tests
  or --list-cases don't need these protections.
* Regrtest: remove selected and tests attributes. Pass 'selected' to
  list_tests(), list_cases() and run_tests(). display_result() now
  expects a TestTuple, instead of TestList.
* Rename setup_tests() to setup_process() and rename setup_support()
  to setup_tests().
* Move _adjust_resource_limits() to utils and rename it to
  adjust_rlimit_nofile().
* Move replace_stdout() to utils.
* Fix RunTests.verbose type: it's an int.
vstinner added a commit to vstinner/cpython that referenced this issue Sep 11, 2023
libregrtest.__init__ no longer exposes any symbol, so
"python -m test.libregrtest.worker" imports less modules.
vstinner added a commit to vstinner/cpython that referenced this issue Sep 11, 2023
libregrtest.__init__ no longer exposes any symbol, so
"python -m test.libregrtest.worker" imports less modules.
vstinner added a commit that referenced this issue Sep 11, 2023
libregrtest.__init__ no longer exposes any symbol, so
"python -m test.libregrtest.worker" imports less modules.
vstinner added a commit to vstinner/cpython that referenced this issue Sep 11, 2023
* Move Regrtest.display_header() to utils.py.
* Move cleanup_temp_dir() to utils.py.
* Move list_cases() to findtests.py.
vstinner added a commit to vstinner/cpython that referenced this issue Sep 11, 2023
* Move Regrtest.display_header() to utils.py.
* Move cleanup_temp_dir() to utils.py.
* Move list_cases() to findtests.py.
vstinner added a commit to vstinner/cpython that referenced this issue Sep 11, 2023
* Move Regrtest.display_header() to utils.py.
* Move cleanup_temp_dir() to utils.py.
* Move list_cases() to findtests.py.
vstinner added a commit to vstinner/cpython that referenced this issue Sep 11, 2023
* Move Regrtest.display_header() to utils.py.
* Move cleanup_temp_dir() to utils.py.
* Move list_cases() to findtests.py.
vstinner added a commit that referenced this issue Sep 11, 2023
* Move Regrtest.display_header() to utils.py.
* Move cleanup_temp_dir() to utils.py.
* Move list_cases() to findtests.py.
@vstinner
Copy link
Member Author

@serhiy-storchaka @AlexWaygood: Ok, with the last change of this serie "libregrtest: move code around", I close the issue.

@AlexWaygood: If you want to give a try to type annotation, you can now go ahead :-) The code should now be cleaner to make it possible. For example, Regrtest is no longer passed to RunWorkers, only results and logger. The "ns" namespace (argparse result) is only used to fill Regrtest attributes, and then is no longer used.

I converted multiple lists to tuples, to make sure that in the API, they cannot be modified. I also added some better defined types, like HuntRefleak class which replaces a flag tuple.

I added some types for type hints: StrPath, TestName, StrJSON. It gives the intent to the reader, compared to str type (the 3 types are in fact aliases to str). I also added other types: TestTuple, TestList, FilterTuple, FilterDict.

@vstinner
Copy link
Member Author

Worker processes don't run test.libregrtest.main anymore, but run python -m test.libregrtest.worker which is separated. The goal is to minimize code imported by libregrtest to run a test. The number of imports should be as small as possible.

Moreover, worker processes are now run in a different worker directory: the temporary directory created by the main regrtest process. (It broke wasm32 buildbots, I have to repair that.)

Well, it's hard to summarize 18 changes, you can look at each commit message if you're curious ;-)

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
Projects
None yet
Development

No branches or pull requests

2 participants