bpo-36719: regrtest always detect uncollectable objects (GH-12951)

vstinner · web-flow · commit 75120d2205af · 2019-04-26T09:28:53.000+02:00
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.
diff --git 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
@@ -20,10 +20,6 @@
 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 @@ def __init__(self):
         # 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 @@ def parse_args(self, kwargs):
                   "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 @@ def rerun_failed_tests(self):
         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 @@ def rerun_failed_tests(self):
             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 @@ def run_tests_sequential(self):
                 # 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
@@ -1,13 +1,15 @@
 import collections
 import faulthandler
 import functools
+import gc
 import importlib
 import io
 import os
 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 @@
 
 
 # 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()
+
+    if gc.garbage:
+        support.environment_altered = True
+        print_warning(f"{test_name} created {len(gc.garbage)} "
+                      f"uncollectable object(s).")
 
-    post_test_cleanup()
+        # 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
@@ -26,9 +26,8 @@
 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 @@ def test_multiprocess(self):
                 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 @@ def list_regex(line_format, tests):
             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 @@ def test_other_bug(self):
         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
@@ -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``.
