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

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 <vstinner@python.org>

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

Lib/test/support/__init__.py

    if not patterns:
        func = None
        # set_match_tests(None) behaves as set_match_tests(())
        patterns = ()
    elif all(map(_is_full_match_test, patterns)):
        # 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 <Pablogsal@gmail.com>

* 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 <Pablogsal@gmail.com> (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 <steve.dower@python.org>

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 <vstinner@python.org>

Update Lib/test/support/__init__.py

f0c1da4

Co-Authored-By: Victor Stinner <vstinner@python.org>

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

pablogsal commented Oct 30, 2019 •

edited by bedevere-bot

Loading

vstinner left a comment

vstinner Nov 19, 2019

vstinner Nov 19, 2019

pablogsal Nov 19, 2019

vstinner Nov 19, 2019

pablogsal Nov 19, 2019

pablogsal Nov 19, 2019

vstinner Nov 19, 2019

pablogsal Nov 19, 2019

vstinner Nov 19, 2019

pablogsal Nov 19, 2019

vstinner Nov 19, 2019

vstinner Nov 19, 2019

vstinner left a comment

pablogsal commented Nov 19, 2019

		_match_test_func = match_function


		def _update_match_function(patterns):

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

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

Conversation

pablogsal commented Oct 30, 2019 • edited by bedevere-bot Loading

vstinner left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vstinner left a comment

Choose a reason for hiding this comment

pablogsal commented Nov 19, 2019

pablogsal commented Oct 30, 2019 •

edited by bedevere-bot

Loading