Better Python spawning primitives in test.script_helper, for

easier writing of unit tests and better error reporting.

4 files changed, 68 insertions, 85 deletions

diff --git a/Lib/test/script_helper.py b/Lib/test/script_helper.py
index 7a29ec6..f40a676 100644
--- a/Lib/test/script_helper.py
+++ b/Lib/test/script_helper.py
@@ -15,12 +15,27 @@ from imp import source_from_cache
 from test.support import make_legacy_pyc
 
 # Executing the interpreter in a subprocess
-def python_exit_code(*args):
+def _assert_python(expected_success, *args):
     cmd_line = [sys.executable, '-E']
     cmd_line.extend(args)
-    with open(os.devnull, 'w') as devnull:
-        return subprocess.call(cmd_line, stdout=devnull,
-                                stderr=subprocess.STDOUT)
+    p = subprocess.Popen(cmd_line, stdin=subprocess.PIPE,
+                         stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+    try:
+        out, err = p.communicate()
+    finally:
+        subprocess._cleanup()
+    rc = p.returncode
+    if (rc and expected_success) or (not rc and not expected_success):
+        raise AssertionError(
+            "Process return code is %d, "
+            "stderr follows:\n%s" % (rc, err.decode('ascii', 'ignore')))
+    return rc, out, err
+
+def assert_python_ok(*args):
+    return _assert_python(True, *args)
+
+def assert_python_failure(*args):
+    return _assert_python(False, *args)
 
 def spawn_python(*args):
     cmd_line = [sys.executable, '-E']
@@ -38,14 +53,6 @@ def kill_python(p):
     subprocess._cleanup()
     return data
 
-def run_python(*args):
-    if __debug__:
-        p = spawn_python(*args)
-    else:
-        p = spawn_python('-O', *args)
-    stdout_data = kill_python(p)
-    return p.wait(), stdout_data
-
 # Script creation utilities
 @contextlib.contextmanager
 def temp_dir():
diff --git a/Lib/test/test_cmd_line.py b/Lib/test/test_cmd_line.py
index 46d2395..ae252b2 100644
--- a/Lib/test/test_cmd_line.py
+++ b/Lib/test/test_cmd_line.py
@@ -5,7 +5,7 @@
 import test.support, unittest
 import os
 import sys
-from test.script_helper import spawn_python, kill_python, python_exit_code
+from test.script_helper import spawn_python, kill_python, assert_python_ok, assert_python_failure
 
 # spawn_python normally enforces use of -E to avoid environmental effects
 # but one test checks PYTHONPATH behaviour explicitly
@@ -26,25 +26,15 @@ def _kill_python_and_exit_code(p):
     return data, returncode
 
 class CmdLineTest(unittest.TestCase):
-    def start_python(self, *args):
-        p = spawn_python(*args)
-        return kill_python(p)
-
-    def start_python_and_exit_code(self, *args):
-        p = spawn_python(*args)
-        return _kill_python_and_exit_code(p)
-
-    def exit_code(self, *args):
-        return python_exit_code(*args)
-
     def test_directories(self):
-        self.assertNotEqual(self.exit_code('.'), 0)
-        self.assertNotEqual(self.exit_code('< .'), 0)
+        assert_python_failure('.')
+        assert_python_failure('< .')
 
     def verify_valid_flag(self, cmd_line):
-        data = self.start_python(cmd_line)
-        self.assertTrue(data == b'' or data.endswith(b'\n'))
-        self.assertNotIn(b'Traceback', data)
+        rc, out, err = assert_python_ok(*cmd_line)
+        self.assertTrue(out == b'' or out.endswith(b'\n'))
+        self.assertNotIn(b'Traceback', out)
+        self.assertNotIn(b'Traceback', err)
 
     def test_optimize(self):
         self.verify_valid_flag('-O')
@@ -60,40 +50,34 @@ class CmdLineTest(unittest.TestCase):
         self.verify_valid_flag('-S')
 
     def test_usage(self):
-        self.assertIn(b'usage', self.start_python('-h'))
+        rc, out, err = assert_python_ok('-h')
+        self.assertIn(b'usage', out)
 
     def test_version(self):
         version = ('Python %d.%d' % sys.version_info[:2]).encode("ascii")
-        self.assertTrue(self.start_python('-V').startswith(version))
+        rc, out, err = assert_python_ok('-V')
+        self.assertTrue(err.startswith(version))
 
     def test_verbose(self):
         # -v causes imports to write to stderr.  If the write to
         # stderr itself causes an import to happen (for the output
         # codec), a recursion loop can occur.
-        data, rc = self.start_python_and_exit_code('-v')
-        self.assertEqual(rc, 0)
-        self.assertNotIn(b'stack overflow', data)
-        data, rc = self.start_python_and_exit_code('-vv')
-        self.assertEqual(rc, 0)
-        self.assertNotIn(b'stack overflow', data)
+        rc, out, err = assert_python_ok('-v')
+        self.assertNotIn(b'stack overflow', err)
+        rc, out, err = assert_python_ok('-vv')
+        self.assertNotIn(b'stack overflow', err)
 
     def test_run_module(self):
         # Test expected operation of the '-m' switch
         # Switch needs an argument
-        self.assertNotEqual(self.exit_code('-m'), 0)
+        assert_python_failure('-m')
         # Check we get an error for a nonexistent module
-        self.assertNotEqual(
-            self.exit_code('-m', 'fnord43520xyz'),
-            0)
+        assert_python_failure('-m', 'fnord43520xyz')
         # Check the runpy module also gives an error for
         # a nonexistent module
-        self.assertNotEqual(
-            self.exit_code('-m', 'runpy', 'fnord43520xyz'),
-            0)
+        assert_python_failure('-m', 'runpy', 'fnord43520xyz'),
         # All good if module is located and run successfully
-        self.assertEqual(
-            self.exit_code('-m', 'timeit', '-n', '1'),
-            0)
+        assert_python_ok('-m', 'timeit', '-n', '1'),
 
     def test_run_module_bug1764407(self):
         # -m and -i need to play well together
@@ -109,22 +93,16 @@ class CmdLineTest(unittest.TestCase):
     def test_run_code(self):
         # Test expected operation of the '-c' switch
         # Switch needs an argument
-        self.assertNotEqual(self.exit_code('-c'), 0)
+        assert_python_failure('-c')
         # Check we get an error for an uncaught exception
-        self.assertNotEqual(
-            self.exit_code('-c', 'raise Exception'),
-            0)
+        assert_python_failure('-c', 'raise Exception')
         # All good if execution is successful
-        self.assertEqual(
-            self.exit_code('-c', 'pass'),
-            0)
+        assert_python_ok('-c', 'pass')
 
         # Test handling of non-ascii data
         if sys.getfilesystemencoding() != 'ascii':
             command = "assert(ord('\xe9') == 0xe9)"
-            self.assertEqual(
-                self.exit_code('-c', command),
-                0)
+            assert_python_ok('-c', command)
 
     def test_unbuffered_output(self):
         # Test expected operation of the '-u' switch
@@ -132,14 +110,14 @@ class CmdLineTest(unittest.TestCase):
             # Binary is unbuffered
             code = ("import os, sys; sys.%s.buffer.write(b'x'); os._exit(0)"
                 % stream)
-            data, rc = self.start_python_and_exit_code('-u', '-c', code)
-            self.assertEqual(rc, 0)
+            rc, out, err = assert_python_ok('-u', '-c', code)
+            data = err if stream == 'stderr' else out
             self.assertEqual(data, b'x', "binary %s not unbuffered" % stream)
             # Text is line-buffered
             code = ("import os, sys; sys.%s.write('x\\n'); os._exit(0)"
                 % stream)
-            data, rc = self.start_python_and_exit_code('-u', '-c', code)
-            self.assertEqual(rc, 0)
+            rc, out, err = assert_python_ok('-u', '-c', code)
+            data = err if stream == 'stderr' else out
             self.assertEqual(data.strip(), b'x',
                 "text %s not line-buffered" % stream)
 
diff --git a/Lib/test/test_cmd_line_script.py b/Lib/test/test_cmd_line_script.py
index d19316b..75e9f8d 100644
--- a/Lib/test/test_cmd_line_script.py
+++ b/Lib/test/test_cmd_line_script.py
@@ -7,8 +7,8 @@ import py_compile
 
 from test import support
 from test.script_helper import (
-    make_pkg, make_script, make_zip_pkg, make_zip_script, run_python,
-    temp_dir)
+    make_pkg, make_script, make_zip_pkg, make_zip_script,
+    assert_python_ok, assert_python_failure, temp_dir)
 
 verbose = support.verbose
 
@@ -98,19 +98,19 @@ class CmdLineTest(unittest.TestCase):
                             expected_package,
                             *cmd_line_switches):
         run_args = cmd_line_switches + (script_name,)
-        exit_code, data = run_python(*run_args)
-        self._check_output(script_name, exit_code, data, expected_file,
+        rc, out, err = assert_python_ok(*run_args)
+        self._check_output(script_name, rc, out + err, expected_file,
                            expected_argv0, expected_path0, expected_package)
 
     def _check_import_error(self, script_name, expected_msg,
                             *cmd_line_switches):
         run_args = cmd_line_switches + (script_name,)
-        exit_code, data = run_python(*run_args)
+        rc, out, err = assert_python_failure(*run_args)
         if verbose > 1:
             print('Output from test script %r:' % script_name)
-            print(data)
+            print(err)
             print('Expected output: %r' % expected_msg)
-        self.assertIn(expected_msg.encode('utf-8'), data)
+        self.assertIn(expected_msg.encode('utf-8'), err)
 
     def test_basic_script(self):
         with temp_dir() as script_dir:
@@ -237,13 +237,12 @@ class CmdLineTest(unittest.TestCase):
                 pkg_dir = os.path.join(script_dir, 'test_pkg')
                 make_pkg(pkg_dir, "import sys; print('init_argv0==%r' % sys.argv[0])")
                 script_name = _make_test_script(pkg_dir, 'script')
-                exit_code, data = run_python('-m', 'test_pkg.script')
+                rc, out, err = assert_python_ok('-m', 'test_pkg.script')
                 if verbose > 1:
                     print(data)
-                self.assertEqual(exit_code, 0)
                 expected = "init_argv0==%r" % '-m'
-                self.assertIn(expected.encode('utf-8'), data)
-                self._check_output(script_name, exit_code, data,
+                self.assertIn(expected.encode('utf-8'), out)
+                self._check_output(script_name, rc, out,
                                    script_name, script_name, '', 'test_pkg')
 
     def test_issue8202_dash_c_file_ignored(self):
@@ -253,13 +252,12 @@ class CmdLineTest(unittest.TestCase):
             with support.temp_cwd(path=script_dir):
                 with open("-c", "w") as f:
                     f.write("data")
-                    exit_code, data = run_python('-c',
+                    rc, out, err = assert_python_ok('-c',
                         'import sys; print("sys.path[0]==%r" % sys.path[0])')
                     if verbose > 1:
-                        print(data)
-                    self.assertEqual(exit_code, 0)
+                        print(out)
                     expected = "sys.path[0]==%r" % ''
-                    self.assertIn(expected.encode('utf-8'), data)
+                    self.assertIn(expected.encode('utf-8'), out)
 
     def test_issue8202_dash_m_file_ignored(self):
         # Make sure a "-m" file in the current directory
@@ -269,8 +267,8 @@ class CmdLineTest(unittest.TestCase):
             with support.temp_cwd(path=script_dir):
                 with open("-m", "w") as f:
                     f.write("data")
-                    exit_code, data = run_python('-m', 'other')
-                    self._check_output(script_name, exit_code, data,
+                    rc, out, err = assert_python_ok('-m', 'other')
+                    self._check_output(script_name, rc, out,
                                       script_name, script_name, '', '')
 
 def test_main():
diff --git a/Lib/test/test_zipimport_support.py b/Lib/test/test_zipimport_support.py
index 27e0bf8..aed7f19 100644
--- a/Lib/test/test_zipimport_support.py
+++ b/Lib/test/test_zipimport_support.py
@@ -13,7 +13,7 @@ import doctest
 import inspect
 import linecache
 import pdb
-from test.script_helper import (spawn_python, kill_python, run_python,
+from test.script_helper import (spawn_python, kill_python, assert_python_ok,
                                 temp_dir, make_script, make_zip_script)
 
 verbose = test.support.verbose
@@ -177,22 +177,22 @@ class ZipSupportTests(ImportHooksBaseTestCase):
         pattern = 'File "%s", line 2, in %s'
         with temp_dir() as d:
             script_name = make_script(d, 'script', test_src)
-            exit_code, data = run_python(script_name)
+            rc, out, err = assert_python_ok(script_name)
             expected = pattern % (script_name, "__main__.Test")
             if verbose:
                 print ("Expected line", expected)
                 print ("Got stdout:")
-                print (data)
-            self.assertIn(expected.encode('utf-8'), data)
+                print (out)
+            self.assertIn(expected.encode('utf-8'), out)
             zip_name, run_name = make_zip_script(d, "test_zip",
                                                 script_name, '__main__.py')
-            exit_code, data = run_python(zip_name)
+            rc, out, err = assert_python_ok(zip_name)
             expected = pattern % (run_name, "__main__.Test")
             if verbose:
                 print ("Expected line", expected)
                 print ("Got stdout:")
-                print (data)
-            self.assertIn(expected.encode('utf-8'), data)
+                print (out)
+            self.assertIn(expected.encode('utf-8'), out)
 
     def test_pdb_issue4201(self):
         test_src = textwrap.dedent("""\