Add a functional test for restore_signals=True.

ChangeLog

@@ -1,3 +1,11 @@
+-----------------
+2018-06-04 3.5.2 (not yet released)
+-----------------
+* Explicitly include <signal.h> in _posixsubprocess_helpers.c; it already
+  gets configure checked and pulled in via Python's own <Python.h> but it
+  is better to be explicit.  #IWYU
+* Adds a functional test for restore_signals=True behavior.
+
 -----------------
 2018-05-21 3.5.1
 -----------------

_posixsubprocess_config.h.in

@@ -25,6 +25,9 @@
 /* Define to 1 if you have the `setsid' function. */
 #undef HAVE_SETSID
 
+/* Define to 1 if you have the <signal.h> header file. */
+#undef HAVE_SIGNAL_H
+
 /* Define to 1 if you have the <stdint.h> header file. */
 #undef HAVE_STDINT_H
 

_posixsubprocess_helpers.c

@@ -2,6 +2,12 @@
    This file is #included by _posixsubprocess.c and the functions
    are declared static to avoid exposing them outside this module. */
 
+/* _posixsubprocess_config.h was already included by _posixsubprocess.c
+ * which is #include'ing us despite the .c name.  HAVE_SIGNAL_H comes
+ * from there.  Yes, confusing! */
+#ifdef HAVE_SIGNAL_H
+#include <signal.h>
+#endif
 #include "unicodeobject.h"
 
 #if (PY_VERSION_HEX < 0x02050000)

configure

@@ -3477,7 +3477,7 @@ fi
 done
 
 
-for ac_header in unistd.h fcntl.h \
+for ac_header in unistd.h fcntl.h signal.h \
                  sys/cdefs.h sys/types.h sys/stat.h sys/syscall.h
 do :
   as_ac_Header=`$as_echo "ac_cv_header_$ac_header" | $as_tr_sh`

configure.ac

@@ -137,7 +137,7 @@ fi
 ######## The stuff we _actually_ want to care about. ########
 
 AC_HEADER_STDC
-AC_CHECK_HEADERS(unistd.h fcntl.h \
+AC_CHECK_HEADERS(unistd.h fcntl.h signal.h \
                  sys/cdefs.h sys/types.h sys/stat.h sys/syscall.h)
 AC_HEADER_DIRENT
 

test_subprocess32.py

@@ -1245,12 +1245,30 @@ def test_exception_bad_args_0(self):
         else:
             self.fail("Expected OSError: %s" % desired_exception)
 
+    #@unittest.skipIf(not os.path.exists('/proc/self/status'))
     def test_restore_signals(self):
-        # Code coverage for both values of restore_signals to make sure it
-        # at least does not blow up.
-        # A test for behavior would be complex.  Contributions welcome.
-        subprocess.call([sys.executable, "-c", ""], restore_signals=True)
-        subprocess.call([sys.executable, "-c", ""], restore_signals=False)
+        if not os.path.exists('/proc/self/status'):
+            print("SKIP - Functional test requires /proc/self/status.")
+            return
+        # Blindly assume that cat exists on systems with /proc/self/status...
+        default_proc_status = subprocess.check_output(
+                ['cat', '/proc/self/status'],
+                restore_signals=False)
+        for line in default_proc_status.splitlines():
+            if line.startswith(b'SigIgn'):
+                default_sig_ign_mask = line
+                break
+        else:
+            self.skipTest("SigIgn not found in /proc/self/status.")
+        restored_proc_status = subprocess.check_output(
+                ['cat', '/proc/self/status'],
+                restore_signals=True)
+        for line in restored_proc_status.splitlines():
+            if line.startswith(b'SigIgn'):
+                restored_sig_ign_mask = line
+                break
+        # restore_signals=True should've unblocked SIGPIPE and friends.
+        self.assertNotEqual(default_sig_ign_mask, restored_sig_ign_mask)
 
     def test_start_new_session(self):
         # For code coverage of calling setsid().  We don't care if we get an
@@ -2258,7 +2276,7 @@ def test_cloexec_pass_fds(self):
             finally:
                 fd_reader_proc.wait()
         finally:
-          null_reader_proc.wait()
+            null_reader_proc.wait()
 
 
 if not getattr(subprocess, '_posixsubprocess', False):