Issue #18948: improve SuppressCoreFiles to include Windows crash popup suppression, and use it in more tests.

Patch by Valerie Lambert and Zachary Ware.

8 files changed, 87 insertions, 94 deletions

diff --git a/Doc/library/test.rst b/Doc/library/test.rst
index c1270f4..2c51549 100644
--- a/Doc/library/test.rst
+++ b/Doc/library/test.rst
@@ -442,13 +442,6 @@ The :mod:`test.support` module defines the following functions:
    A decorator for running tests that require support for symbolic links.
 
 
-.. function:: suppress_crash_popup()
-
-   A context manager that disables Windows Error Reporting dialogs using
-   `SetErrorMode <http://msdn.microsoft.com/en-us/library/windows/desktop/ms680621%28v=vs.85%29.aspx>`_.
-   On other platforms it's a no-op.
-
-
 .. decorator:: anticipate_failure(condition)
 
    A decorator to conditionally mark tests with
@@ -593,6 +586,21 @@ The :mod:`test.support` module defines the following classes:
    Temporarily unset the environment variable ``envvar``.
 
 
+.. class:: SuppressCrashReport()
+
+   A context manager used to try to prevent crash dialog popups on tests that
+   are expected to crash a subprocess.
+
+   On Windows, it disables Windows Error Reporting dialogs using
+   `SetErrorMode <http://msdn.microsoft.com/en-us/library/windows/desktop/ms680621.aspx>`_.
+
+   On UNIX, :func:`resource.setrlimit` is used to set
+   :attr:`resource.RLIMIT_CORE`'s soft limit to 0 to prevent coredump file
+   creation.
+
+   On both platforms, the old value is restored by :meth:`__exit__`.
+
+
 .. class:: WarningsRecorder()
 
    Class used to record warnings for unit tests. See documentation of
diff --git a/Lib/test/support/__init__.py b/Lib/test/support/__init__.py
index 5687ef9..d4f7566 100644
--- a/Lib/test/support/__init__.py
+++ b/Lib/test/support/__init__.py
@@ -81,8 +81,7 @@ __all__ = [
     "TestHandler", "Matcher", "can_symlink", "skip_unless_symlink",
     "skip_unless_xattr", "import_fresh_module", "requires_zlib",
     "PIPE_MAX_SIZE", "failfast", "anticipate_failure", "run_with_tz",
-    "requires_gzip", "requires_bz2", "requires_lzma", "suppress_crash_popup",
-    "SuppressCoreFiles",
+    "requires_gzip", "requires_bz2", "requires_lzma", "SuppressCrashReport"
     ]
 
 class Error(Exception):
@@ -2013,27 +2012,67 @@ def skip_unless_xattr(test):
     return test if ok else unittest.skip(msg)(test)
 
 
-if sys.platform.startswith('win'):
-    @contextlib.contextmanager
-    def suppress_crash_popup():
-        """Disable Windows Error Reporting dialogs using SetErrorMode."""
-        # see http://msdn.microsoft.com/en-us/library/windows/desktop/ms680621%28v=vs.85%29.aspx
-        # GetErrorMode is not available on Windows XP and Windows Server 2003,
-        # but SetErrorMode returns the previous value, so we can use that
-        import ctypes
-        k32 = ctypes.windll.kernel32
-        SEM_NOGPFAULTERRORBOX = 0x02
-        old_error_mode = k32.SetErrorMode(SEM_NOGPFAULTERRORBOX)
-        k32.SetErrorMode(old_error_mode | SEM_NOGPFAULTERRORBOX)
-        try:
-            yield
-        finally:
-            k32.SetErrorMode(old_error_mode)
-else:
-    # this is a no-op for other platforms
-    @contextlib.contextmanager
-    def suppress_crash_popup():
-        yield
+class SuppressCrashReport:
+    """Try to prevent a crash report from popping up.
+
+    On Windows, don't display the Windows Error Reporting dialog.  On UNIX,
+    disable the creation of coredump file.
+    """
+    old_value = None
+
+    def __enter__(self):
+        """On Windows, disable Windows Error Reporting dialogs using
+        SetErrorMode.
+
+        On UNIX, try to save the previous core file size limit, then set
+        soft limit to 0.
+        """
+        if sys.platform.startswith('win'):
+            # see http://msdn.microsoft.com/en-us/library/windows/desktop/ms680621.aspx
+            # GetErrorMode is not available on Windows XP and Windows Server 2003,
+            # but SetErrorMode returns the previous value, so we can use that
+            import ctypes
+            self._k32 = ctypes.windll.kernel32
+            SEM_NOGPFAULTERRORBOX = 0x02
+            self.old_value = self._k32.SetErrorMode(SEM_NOGPFAULTERRORBOX)
+            self._k32.SetErrorMode(self.old_value | SEM_NOGPFAULTERRORBOX)
+        else:
+            if resource is not None:
+                try:
+                    self.old_value = resource.getrlimit(resource.RLIMIT_CORE)
+                    resource.setrlimit(resource.RLIMIT_CORE,
+                                       (0, self.old_value[1]))
+                except (ValueError, OSError):
+                    pass
+            if sys.platform == 'darwin':
+                # Check if the 'Crash Reporter' on OSX was configured
+                # in 'Developer' mode and warn that it will get triggered
+                # when it is.
+                #
+                # This assumes that this context manager is used in tests
+                # that might trigger the next manager.
+                value = subprocess.Popen(['/usr/bin/defaults', 'read',
+                        'com.apple.CrashReporter', 'DialogType'],
+                        stdout=subprocess.PIPE).communicate()[0]
+                if value.strip() == b'developer':
+                    print("this test triggers the Crash Reporter, "
+                          "that is intentional", end='', flush=True)
+
+        return self
+
+    def __exit__(self, *ignore_exc):
+        """Restore Windows ErrorMode or core file behavior to initial value."""
+        if self.old_value is None:
+            return
+
+        if sys.platform.startswith('win'):
+            self._k32.SetErrorMode(self.old_value)
+        else:
+            if resource is not None:
+                try:
+                    resource.setrlimit(resource.RLIMIT_CORE, self.old_value)
+                except (ValueError, OSError):
+                    pass
 
 
 def patch(test_instance, object_to_patch, attr_name, new_value):
@@ -2068,42 +2107,3 @@ def patch(test_instance, object_to_patch, attr_name, new_value):
 
     # actually override the attribute
     setattr(object_to_patch, attr_name, new_value)
-
-
-class SuppressCoreFiles:
-
-    """Try to prevent core files from being created."""
-    old_limit = None
-
-    def __enter__(self):
-        """Try to save previous ulimit, then set the soft limit to 0."""
-        if resource is not None:
-            try:
-                self.old_limit = resource.getrlimit(resource.RLIMIT_CORE)
-                resource.setrlimit(resource.RLIMIT_CORE, (0, self.old_limit[1]))
-            except (ValueError, OSError):
-                pass
-        if sys.platform == 'darwin':
-            # Check if the 'Crash Reporter' on OSX was configured
-            # in 'Developer' mode and warn that it will get triggered
-            # when it is.
-            #
-            # This assumes that this context manager is used in tests
-            # that might trigger the next manager.
-            value = subprocess.Popen(['/usr/bin/defaults', 'read',
-                    'com.apple.CrashReporter', 'DialogType'],
-                    stdout=subprocess.PIPE).communicate()[0]
-            if value.strip() == b'developer':
-                print("this test triggers the Crash Reporter, "
-                      "that is intentional", end='')
-                sys.stdout.flush()
-
-    def __exit__(self, *ignore_exc):
-        """Return core file behavior to default."""
-        if self.old_limit is None:
-            return
-        if resource is not None:
-            try:
-                resource.setrlimit(resource.RLIMIT_CORE, self.old_limit)
-            except (ValueError, OSError):
-                pass
diff --git a/Lib/test/test_capi.py b/Lib/test/test_capi.py
index 62827e5..bbbacc2 100644
--- a/Lib/test/test_capi.py
+++ b/Lib/test/test_capi.py
@@ -44,7 +44,7 @@ class CAPITest(unittest.TestCase):
 
     @unittest.skipUnless(threading, 'Threading required for this test.')
     def test_no_FatalError_infinite_loop(self):
-        with support.suppress_crash_popup():
+        with support.SuppressCrashReport():
             p = subprocess.Popen([sys.executable, "-c",
                                   'import _testcapi;'
                                   '_testcapi.crash_no_current_thread()'],
diff --git a/Lib/test/test_faulthandler.py b/Lib/test/test_faulthandler.py
index d78bcb0..2d374b9 100644
--- a/Lib/test/test_faulthandler.py
+++ b/Lib/test/test_faulthandler.py
@@ -19,18 +19,6 @@ except ImportError:
 
 TIMEOUT = 0.5
 
-try:
-    from resource import setrlimit, RLIMIT_CORE, error as resource_error
-except ImportError:
-    prepare_subprocess = None
-else:
-    def prepare_subprocess():
-        # don't create core file
-        try:
-            setrlimit(RLIMIT_CORE, (0, 0))
-        except (ValueError, resource_error):
-            pass
-
 def expected_traceback(lineno1, lineno2, header, min_count=1):
     regex = header
     regex += '  File "<string>", line %s in func\n' % lineno1
@@ -59,10 +47,8 @@ class FaultHandlerTests(unittest.TestCase):
         build, and replace "Current thread 0x00007f8d8fbd9700" by "Current
         thread XXX".
         """
-        options = {}
-        if prepare_subprocess:
-            options['preexec_fn'] = prepare_subprocess
-        process = script_helper.spawn_python('-c', code, **options)
+        with support.SuppressCrashReport():
+            process = script_helper.spawn_python('-c', code)
         stdout, stderr = process.communicate()
         exitcode = process.wait()
         output = support.strip_python_stderr(stdout)
@@ -101,8 +87,7 @@ class FaultHandlerTests(unittest.TestCase):
             header=re.escape(header))
         if other_regex:
             regex += '|' + other_regex
-        with support.suppress_crash_popup():
-            output, exitcode = self.get_output(code, filename)
+        output, exitcode = self.get_output(code, filename)
         output = '\n'.join(output)
         self.assertRegex(output, regex)
         self.assertNotEqual(exitcode, 0)
@@ -232,8 +217,7 @@ faulthandler.disable()
 faulthandler._sigsegv()
 """.strip()
         not_expected = 'Fatal Python error'
-        with support.suppress_crash_popup():
-            stderr, exitcode = self.get_output(code)
+        stderr, exitcode = self.get_output(code)
         stder = '\n'.join(stderr)
         self.assertTrue(not_expected not in stderr,
                      "%r is present in %r" % (not_expected, stderr))
diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py
index 4c15ee3..637b1bf 100644
--- a/Lib/test/test_subprocess.py
+++ b/Lib/test/test_subprocess.py
@@ -1231,7 +1231,7 @@ class POSIXProcessTestCase(BaseTestCase):
 
     def test_run_abort(self):
         # returncode handles signal termination
-        with support.SuppressCoreFiles():
+        with support.SuppressCrashReport():
             p = subprocess.Popen([sys.executable, "-c",
                                   'import os; os.abort()'])
             p.wait()
diff --git a/Lib/test/test_support.py b/Lib/test/test_support.py
index 9afc1d0..16b660b 100644
--- a/Lib/test/test_support.py
+++ b/Lib/test/test_support.py
@@ -306,7 +306,7 @@ class TestSupport(unittest.TestCase):
     # args_from_interpreter_flags
     # can_symlink
     # skip_unless_symlink
-    # SuppressCoreFiles
+    # SuppressCrashReport
 
 
 def test_main():
diff --git a/Lib/test/test_sys.py b/Lib/test/test_sys.py
index 70b55b2..8437745 100644
--- a/Lib/test/test_sys.py
+++ b/Lib/test/test_sys.py
@@ -250,7 +250,7 @@ class SysModuleTest(unittest.TestCase):
 
             sys.setrecursionlimit(%d)
             f()""")
-        with test.support.suppress_crash_popup():
+        with test.support.SuppressCrashReport():
             for i in (50, 1000):
                 sub = subprocess.Popen([sys.executable, '-c', code % i],
                     stderr=subprocess.PIPE)
diff --git a/Lib/test/test_threading.py b/Lib/test/test_threading.py
index c39d5e2..826acbb 100644
--- a/Lib/test/test_threading.py
+++ b/Lib/test/test_threading.py
@@ -839,7 +839,8 @@ class SubinterpThreadingTests(BaseTestCase):
 
             _testcapi.run_in_subinterp(%r)
             """ % (subinterp_code,)
-        rc, out, err = assert_python_failure("-c", script)
+        with test.support.SuppressCrashReport():
+            rc, out, err = assert_python_failure("-c", script)
         self.assertIn("Fatal Python error: Py_EndInterpreter: "
                       "not the last thread", err.decode())