bpo-37957: Allow regrtest to receive a file with test (and subtests) to ignore by pablogsal · Pull Request #16989 · python/cpython

pablogsal · 2019-10-30T00:02:33Z

https://bugs.python.org/issue37957

…to ignore

vstinner

I love this feature :-) Here is my review.

Lib/test/support/__init__.py

vstinner · 2019-11-19T21:14:41Z

Lib/test/support/__init__.py

+        _match_test_func = match_function
+
+
+def _update_match_function(patterns):


I'm fine with "_update_match_function" name. Another propositions: "_compile_match_function" or "_create_match_function".

vstinner · 2019-11-19T21:15:53Z

Lib/test/support/__init__.py

+
+    # Create a copy since patterns can be mutable and so modified later
+    _accept_test_patterns = tuple(accept_patterns)
+    _ignore_test_patterns = tuple(reject_patterns)


Maybe do that in _update_match_function()?

Hummmm, I would suggest to have _update_match_function still be agnostic of the pattern is generating and be free of side effects

Lib/test/support/__init__.py

vstinner · 2019-11-19T21:18:42Z

Lib/test/test_regrtest.py

+        # by default, all methods should be run
+        output = self.run_tests("-v", testname)
+        methods = self.parse_methods(output)
+        self.assertEqual(methods, all_methods)


I don't think that this test is useful, but you can keep it if you prefer :-)

I can remove it :)

Do you mean the whole test_ignorefile test or just that assert?

My comment is on these 4 lines:

# by default, all methods should be run output = self.run_tests("-v", testname) methods = self.parse_methods(output) self.assertEqual(methods, all_methods)

I have removed them :)

Co-Authored-By: Victor Stinner <[email protected]>

vstinner · 2019-11-19T22:17:40Z

Lib/test/support/__init__.py

        # Simple case: all patterns are full test identifier.
        # The test.bisect_cmd utility only uses such full test identifiers.
-        func = set(patterns).__contains__
+        func = lambda elem: elem in set(patterns)


Hum, you create a new temporary set object at each call. set(patterns).__contains__ creates the set exactly once. No?

Indeed, good catch!

vstinner · 2019-11-19T22:18:08Z

Lib/test/test_support.py

        with support.swap_attr(support, '_match_test_func', None):
            # match all
-            support.set_match_tests([])
+            support.set_match_tests([], None)


You might omit None here now. It's up to you.

vstinner · 2019-11-19T22:18:46Z

Lib/test/test_support.py

+        # Test rejection
+        with support.swap_attr(support, '_match_test_func', None):
+            # match all
+            support.set_match_tests(None, [])


You might write support.set_match_tests(ignore_patterns=[]) instead. It's up to you.

vstinner

LGTM, thanks for the update.

pablogsal · 2019-11-19T23:47:05Z

Thanks for the review, @vstinner 🎉

…to ignore (pythonGH-16989) When building Python in some uncommon platforms there are some known tests that will fail. Right now, the test suite has the ability to ignore entire tests using the -x option and to receive a filter file using the --matchfile filter. The problem with the --matchfile option is that it receives a file with patterns to accept and when you want to ignore a couple of tests and subtests, is too cumbersome to lists ALL tests that are not the ones that you want to accept and he problem with -x is that is not easy to ignore just a subtests that fail and the whole test needs to be ignored. For these reasons, add a new option to allow to ignore a list of test and subtests for these situations.

* bpo-37531: regrtest now catchs ProcessLookupError (GH-16827) Fix a warning on a race condition on TestWorkerProcess.kill(): ignore silently ProcessLookupError rather than logging an useless warning. (cherry picked from commit a661392) * bpo-38502: regrtest uses process groups if available (GH-16829) test.regrtest now uses process groups in the multiprocessing mode (-jN command line option) if process groups are available: if os.setsid() and os.killpg() functions are available. (cherry picked from commit ecb035c) * bpo-37957: Allow regrtest to receive a file with test (and subtests) to ignore (GH-16989) When building Python in some uncommon platforms there are some known tests that will fail. Right now, the test suite has the ability to ignore entire tests using the -x option and to receive a filter file using the --matchfile filter. The problem with the --matchfile option is that it receives a file with patterns to accept and when you want to ignore a couple of tests and subtests, is too cumbersome to lists ALL tests that are not the ones that you want to accept and he problem with -x is that is not easy to ignore just a subtests that fail and the whole test needs to be ignored. For these reasons, add a new option to allow to ignore a list of test and subtests for these situations. (cherry picked from commit e0cd8aa) * regrtest: log timeout at startup (GH-19514) Reduce also worker timeout. (cherry picked from commit 4cf65a6) Co-authored-by: Pablo Galindo <[email protected]>

* bpo-36670: regrtest bug fixes (GH-16537) * Fix TestWorkerProcess.__repr__(): start_time is only valid if _popen is not None. * Fix _kill(): don't set _killed to True if _popen is None. * _run_process(): only set _killed to False after calling run_test_in_subprocess(). (cherry picked from commit 2ea71a0) * [3.8] Update libregrtest from master (GH-19516) * bpo-37531: regrtest now catchs ProcessLookupError (GH-16827) Fix a warning on a race condition on TestWorkerProcess.kill(): ignore silently ProcessLookupError rather than logging an useless warning. (cherry picked from commit a661392) * bpo-38502: regrtest uses process groups if available (GH-16829) test.regrtest now uses process groups in the multiprocessing mode (-jN command line option) if process groups are available: if os.setsid() and os.killpg() functions are available. (cherry picked from commit ecb035c) * bpo-37957: Allow regrtest to receive a file with test (and subtests) to ignore (GH-16989) When building Python in some uncommon platforms there are some known tests that will fail. Right now, the test suite has the ability to ignore entire tests using the -x option and to receive a filter file using the --matchfile filter. The problem with the --matchfile option is that it receives a file with patterns to accept and when you want to ignore a couple of tests and subtests, is too cumbersome to lists ALL tests that are not the ones that you want to accept and he problem with -x is that is not easy to ignore just a subtests that fail and the whole test needs to be ignored. For these reasons, add a new option to allow to ignore a list of test and subtests for these situations. (cherry picked from commit e0cd8aa) * regrtest: log timeout at startup (GH-19514) Reduce also worker timeout. (cherry picked from commit 4cf65a6) Co-authored-by: Pablo Galindo <[email protected]> (cherry picked from commit 67b8a1f) * bpo-36842: Fix reference leak in tests by running out-of-proc (GH-13556) (cherry picked from commit 9ddc416) * Backport libregrtest changes from master Co-authored-by: Steve Dower <[email protected]>

pablogsal requested a review from vstinner October 30, 2019 00:02

the-knights-who-say-ni added the CLA signed label Oct 30, 2019

bedevere-bot added the awaiting core review label Oct 30, 2019

pablogsal force-pushed the bpo-37957 branch from f779185 to 7e5ffd2 Compare October 30, 2019 00:04

bpo-37957: Allow regrtest to receive a file with test (and subtests) …

bd5ba6e

…to ignore

pablogsal force-pushed the bpo-37957 branch from 7e5ffd2 to bd5ba6e Compare November 19, 2019 01:02

vstinner reviewed Nov 19, 2019

View reviewed changes

pablogsal and others added 4 commits November 19, 2019 21:35

Update Lib/test/support/__init__.py

720ba4a

Co-Authored-By: Victor Stinner <[email protected]>

Update Lib/test/support/__init__.py

f0c1da4

Co-Authored-By: Victor Stinner <[email protected]>

fixup! Update Lib/test/support/__init__.py

16cec07

fixup! fixup! Update Lib/test/support/__init__.py

da9763c

vstinner reviewed Nov 19, 2019

View reviewed changes

pablogsal added 2 commits November 19, 2019 22:25

fixup! fixup! fixup! Update Lib/test/support/__init__.py

e9ee42f

fixup! fixup! fixup! fixup! Update Lib/test/support/__init__.py

6a27c69

vstinner approved these changes Nov 19, 2019

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting core review labels Nov 19, 2019

pablogsal merged commit e0cd8aa into python:master Nov 19, 2019

pablogsal deleted the bpo-37957 branch November 19, 2019 23:46

bedevere-bot removed the awaiting merge label Nov 19, 2019

pablogsal mentioned this pull request Apr 10, 2022

Allow regrtest to receive a file with test (and subtests) to ignore #82138

Closed

		_match_test_func = match_function


		def _update_match_function(patterns):

Uh oh!

Conversation

pablogsal commented Oct 30, 2019 • edited by bedevere-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vstinner left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vstinner left a comment

Choose a reason for hiding this comment

Uh oh!

pablogsal commented Nov 19, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

pablogsal commented Oct 30, 2019 •

edited by bedevere-bot

Loading