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

Choose a reason for hiding this comment

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

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

_match_test_func = match_function


def _update_match_function(patterns):

Choose a reason for hiding this comment

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

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


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

Choose a reason for hiding this comment

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

Maybe do that in _update_match_function()?

Choose a reason for hiding this comment

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

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

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

Choose a reason for hiding this comment

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

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

Choose a reason for hiding this comment

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

I can remove it :)

Choose a reason for hiding this comment

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

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

Choose a reason for hiding this comment

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

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)

Choose a reason for hiding this comment

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

I have removed them :)

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

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)

Choose a reason for hiding this comment

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

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

Choose a reason for hiding this comment

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

Indeed, good catch!

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

Choose a reason for hiding this comment

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

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

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

Choose a reason for hiding this comment

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

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

Choose a reason for hiding this comment

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

LGTM, thanks for the update.

jacobneiltaylor pushed a commit to jacobneiltaylor/cpython that referenced this pull request

Dec 5, 2019

…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.

shihai1991 pushed a commit to shihai1991/cpython that referenced this pull request

Jan 31, 2020

…to ignore (pythonGH-16989)

For these reasons, add a new option to allow to ignore a list of test and subtests for these situations.

vstinner added a commit that referenced this pull request

Apr 14, 2020

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)

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

vstinner added a commit that referenced this pull request

Apr 14, 2020

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)

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