From 75120d2205af086140e5e4e2dc620eb19cdf9078 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Fri, 26 Apr 2019 09:28:53 +0200 Subject: [PATCH] bpo-36719: regrtest always detect uncollectable objects (GH-12951) regrtest now always detects uncollectable objects. Previously, the check was only enabled by --findleaks. The check now also works with -jN/--multiprocess N. --findleaks becomes a deprecated alias to --fail-env-changed. --- Lib/test/libregrtest/cmdline.py | 12 +++--- Lib/test/libregrtest/main.py | 43 +++---------------- Lib/test/libregrtest/runtest.py | 35 +++++++-------- Lib/test/test_regrtest.py | 41 +++++++++++++++--- .../2019-04-26-09-02-49.bpo-36719.ys2uqH.rst | 4 ++ 5 files changed, 66 insertions(+), 69 deletions(-) create mode 100644 Misc/NEWS.d/next/Tests/2019-04-26-09-02-49.bpo-36719.ys2uqH.rst diff --git a/Lib/test/libregrtest/cmdline.py b/Lib/test/libregrtest/cmdline.py index 7cd85bf2803..cb09ee0e03b 100644 --- a/Lib/test/libregrtest/cmdline.py +++ b/Lib/test/libregrtest/cmdline.py @@ -226,8 +226,9 @@ def _create_parser(): '(instead of the Python stdlib test suite)') group = parser.add_argument_group('Special runs') - group.add_argument('-l', '--findleaks', action='store_true', - help='if GC is available detect tests that leak memory') + group.add_argument('-l', '--findleaks', action='store_const', const=2, + default=1, + help='deprecated alias to --fail-env-changed') group.add_argument('-L', '--runleaks', action='store_true', help='run the leaks(1) command just before exit.' + more_details) @@ -309,7 +310,7 @@ def _parse_args(args, **kwargs): # Defaults ns = argparse.Namespace(testdir=None, verbose=0, quiet=False, exclude=False, single=False, randomize=False, fromfile=None, - findleaks=False, use_resources=None, trace=False, coverdir='coverage', + findleaks=1, 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, failfast=False, match_tests=None, pgo=False) @@ -330,12 +331,13 @@ def _parse_args(args, **kwargs): parser.error("unrecognized arguments: %s" % arg) sys.exit(1) + if ns.findleaks > 1: + # --findleaks implies --fail-env-changed + ns.fail_env_changed = True if ns.single and ns.fromfile: parser.error("-s and -f don't go together!") if ns.use_mp is not None and ns.trace: parser.error("-T and -j don't go together!") - if ns.use_mp is not None and ns.findleaks: - parser.error("-l and -j don't go together!") if ns.failfast and not (ns.verbose or ns.verbose3): parser.error("-G/--failfast needs either -v or -W") if ns.pgo and (ns.verbose or ns.verbose2 or ns.verbose3): diff --git a/Lib/test/libregrtest/main.py b/Lib/test/libregrtest/main.py index 606dc268ae3..def6532b623 100644 --- a/Lib/test/libregrtest/main.py +++ b/Lib/test/libregrtest/main.py @@ -20,10 +20,6 @@ from test.libregrtest.runtest import ( from test.libregrtest.setup import setup_tests from test.libregrtest.utils import removepy, count, format_duration, printlist from test import support -try: - import gc -except ImportError: - gc = None # When tests are run from the Python build directory, it is best practice @@ -90,9 +86,6 @@ class Regrtest: # used by --coverage, trace.Trace instance self.tracer = None - # used by --findleaks, store for gc.garbage - self.found_garbage = [] - # used to display the progress bar "[ 3/100]" self.start_time = time.monotonic() self.test_count = '' @@ -173,22 +166,6 @@ class Regrtest: "faulthandler.dump_traceback_later", file=sys.stderr) ns.timeout = None - if ns.threshold is not None and gc is None: - print('No GC available, ignore --threshold.', file=sys.stderr) - ns.threshold = None - - if ns.findleaks: - if gc is not None: - # Uncomment the line below to report garbage that is not - # freeable by reference counting alone. By default only - # garbage that is not collectable by the GC is reported. - pass - #gc.set_debug(gc.DEBUG_SAVEALL) - else: - print('No GC available, disabling --findleaks', - file=sys.stderr) - ns.findleaks = False - if ns.xmlpath: support.junit_xml_list = self.testsuite_xml = [] @@ -308,7 +285,7 @@ class Regrtest: print("Re-running failed tests in verbose mode") self.rerun = self.bad[:] for test_name in self.rerun: - print("Re-running test %r in verbose mode" % test_name, flush=True) + print(f"Re-running {test_name} in verbose mode", flush=True) self.ns.verbose = True ok = runtest(self.ns, test_name) @@ -318,10 +295,10 @@ class Regrtest: if ok.result == INTERRUPTED: self.interrupted = True break - else: - if self.bad: - print(count(len(self.bad), 'test'), "failed again:") - printlist(self.bad) + + if self.bad: + print(count(len(self.bad), 'test'), "failed again:") + printlist(self.bad) self.display_result() @@ -426,16 +403,6 @@ class Regrtest: # be quiet: say nothing if the test passed shortly previous_test = None - if self.ns.findleaks: - gc.collect() - if gc.garbage: - print("Warning: test created", len(gc.garbage), end=' ') - print("uncollectable object(s).") - # move the uncollectable objects somewhere so we don't see - # them again - self.found_garbage.extend(gc.garbage) - del gc.garbage[:] - # Unload the newly imported modules (best effort finalization) for module in sys.modules.keys(): if module not in save_modules and module.startswith("test."): diff --git a/Lib/test/libregrtest/runtest.py b/Lib/test/libregrtest/runtest.py index c0cfa5312f7..a9574929a4c 100644 --- a/Lib/test/libregrtest/runtest.py +++ b/Lib/test/libregrtest/runtest.py @@ -1,6 +1,7 @@ import collections import faulthandler import functools +import gc import importlib import io import os @@ -8,6 +9,7 @@ import sys import time import traceback import unittest + from test import support from test.libregrtest.refleak import dash_R, clear_caches from test.libregrtest.save_env import saved_test_environment @@ -59,7 +61,7 @@ NOTTESTS = set() # used by --findleaks, store for gc.garbage -found_garbage = [] +FOUND_GARBAGE = [] def format_test_result(result): @@ -182,11 +184,6 @@ def runtest(ns, test_name): return TestResult(test_name, FAILED, 0.0, None) -def post_test_cleanup(): - support.gc_collect() - support.reap_children() - - def _test_module(the_module): loader = unittest.TestLoader() tests = loader.loadTestsFromModule(the_module) @@ -224,21 +221,19 @@ def _runtest_inner2(ns, test_name): finally: cleanup_test_droppings(test_name, ns.verbose) - if ns.findleaks: - import gc - support.gc_collect() - if gc.garbage: - import gc - gc.garbage = [1] - print_warning(f"{test_name} created {len(gc.garbage)} " - f"uncollectable object(s).") - # move the uncollectable objects somewhere, - # so we don't see them again - found_garbage.extend(gc.garbage) - gc.garbage.clear() - support.environment_altered = True + support.gc_collect() - post_test_cleanup() + if gc.garbage: + support.environment_altered = True + print_warning(f"{test_name} created {len(gc.garbage)} " + f"uncollectable object(s).") + + # move the uncollectable objects somewhere, + # so we don't see them again + FOUND_GARBAGE.extend(gc.garbage) + gc.garbage.clear() + + support.reap_children() return refleak diff --git a/Lib/test/test_regrtest.py b/Lib/test/test_regrtest.py index e0d1d3cec7c..7ff2dde5aa1 100644 --- a/Lib/test/test_regrtest.py +++ b/Lib/test/test_regrtest.py @@ -26,9 +26,8 @@ ROOT_DIR = os.path.join(os.path.dirname(__file__), '..', '..') ROOT_DIR = os.path.abspath(os.path.normpath(ROOT_DIR)) TEST_INTERRUPTED = textwrap.dedent(""" - from signal import SIGINT + from signal import SIGINT, raise_signal try: - from signal import raise_signal raise_signal(SIGINT) except ImportError: import os @@ -255,9 +254,7 @@ class ParseArgsTestCase(unittest.TestCase): self.checkError([opt], 'expected one argument') self.checkError([opt, 'foo'], 'invalid int value') self.checkError([opt, '2', '-T'], "don't go together") - self.checkError([opt, '2', '-l'], "don't go together") self.checkError([opt, '0', '-T'], "don't go together") - self.checkError([opt, '0', '-l'], "don't go together") def test_coverage(self): for opt in '-T', '--coverage': @@ -454,8 +451,8 @@ class BaseTestCase(unittest.TestCase): regex = list_regex('%s re-run test%s', rerun) self.check_line(output, regex) self.check_line(output, "Re-running failed tests in verbose mode") - for name in rerun: - regex = "Re-running test %r in verbose mode" % name + for test_name in rerun: + regex = f"Re-running {test_name} in verbose mode" self.check_line(output, regex) if no_test_ran: @@ -1070,6 +1067,38 @@ class ArgsTestCase(BaseTestCase): self.check_executed_tests(output, [testname, testname2], no_test_ran=[testname]) + @support.cpython_only + def test_findleaks(self): + code = textwrap.dedent(r""" + import _testcapi + import gc + import unittest + + @_testcapi.with_tp_del + class Garbage: + def __tp_del__(self): + pass + + class Tests(unittest.TestCase): + def test_garbage(self): + # create an uncollectable object + obj = Garbage() + obj.ref_cycle = obj + obj = None + """) + testname = self.create_test(code=code) + + output = self.run_tests("--fail-env-changed", testname, exitcode=3) + self.check_executed_tests(output, [testname], + env_changed=[testname], + fail_env_changed=True) + + # --findleaks is now basically an alias to --fail-env-changed + output = self.run_tests("--findleaks", testname, exitcode=3) + self.check_executed_tests(output, [testname], + env_changed=[testname], + fail_env_changed=True) + class TestUtils(unittest.TestCase): def test_format_duration(self): diff --git a/Misc/NEWS.d/next/Tests/2019-04-26-09-02-49.bpo-36719.ys2uqH.rst b/Misc/NEWS.d/next/Tests/2019-04-26-09-02-49.bpo-36719.ys2uqH.rst new file mode 100644 index 00000000000..4b6ef76bc6d --- /dev/null +++ b/Misc/NEWS.d/next/Tests/2019-04-26-09-02-49.bpo-36719.ys2uqH.rst @@ -0,0 +1,4 @@ +regrtest now always detects uncollectable objects. Previously, the check was +only enabled by ``--findleaks``. The check now also works with +``-jN/--multiprocess N``. ``--findleaks`` becomes a deprecated alias to +``--fail-env-changed``.