GeezerVison is showing github.com — Refactor test.libregrtest · Issue #109162 · python/cpython
Refactor test.libregrtest #109162DescriptionI 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
Activityvstinner commented on 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 commented on Sep 11, 2023 @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 commented on Sep 11, 2023 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 ;-)
MetadataMetadataDevelopmentNo branches or pull requestsIssue actions
|