[3.9] bpo-43776: Remove list call from args in Popen repr (GH-25338) (GH-26510)

gpshead · mpkocher · web-flow · commit 5a8ddcc4524d · 2021-06-02T21:15:26.000-07:00
Removes the `list` call in the Popen `repr`. Current implementation: For cmd = `python --version`, with `shell=True`. ```bash <Popen: returncode: None args: ['p', 'y', 't', 'h', 'o', 'n', ' ', '-', '-',...> ``` For `shell=False` and args=`['python', '--version']`, the output is correct: ```bash <Popen: returncode: None args: ['python', '--version']> ``` With the new changes the `repr` yields: For cmd = `python --version`, with `shell=True`: ```bash <Popen: returncode: None args: 'python --version'> ``` For `shell=False` and args=`['python', '--version']`, the output: ```bash <Popen: returncode: None args: ['python', '--version']> ``` Automerge-Triggered-By: GH:gpshead. (cherry picked from commit db0c5b7) Co-authored-by: M. Kocher <michael.kocher@me.com> Co-authored-by: M. Kocher <michael.kocher@me.com>
diff --git a/Lib/subprocess.py b/Lib/subprocess.py
@@ -989,7 +989,7 @@ def __init__(self, args, bufsize=-1, executable=None,
     def __repr__(self):
         obj_repr = (
             f"<{self.__class__.__name__}: "
-            f"returncode: {self.returncode} args: {list(self.args)!r}>"
+            f"returncode: {self.returncode} args: {self.args!r}>"
         )
         if len(obj_repr) > 80:
             obj_repr = obj_repr[:76] + "...>"
diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py
@@ -20,6 +20,7 @@
 import gc
 import textwrap
 import json
+import pathlib
 from test.support import FakePath
 
 try:
@@ -1373,28 +1374,23 @@ def test_communicate_epipe(self):
         p.communicate(b"x" * 2**20)
 
     def test_repr(self):
-        # Run a command that waits for user input, to check the repr() of
-        # a Proc object while and after the sub-process runs.
-        code = 'import sys; input(); sys.exit(57)'
-        cmd = [sys.executable, '-c', code]
-        result = "<Popen: returncode: {}"
-
-        with subprocess.Popen(
-                cmd, stdin=subprocess.PIPE, universal_newlines=True) as proc:
-            self.assertIsNone(proc.returncode)
-            self.assertTrue(
-                repr(proc).startswith(result.format(proc.returncode)) and
-                repr(proc).endswith('>')
-            )
-
-            proc.communicate(input='exit...\n')
-            proc.wait()
-
-            self.assertIsNotNone(proc.returncode)
-            self.assertTrue(
-                repr(proc).startswith(result.format(proc.returncode)) and
-                repr(proc).endswith('>')
-            )
+        path_cmd = pathlib.Path("my-tool.py")
+        pathlib_cls = path_cmd.__class__.__name__
+
+        cases = [
+            ("ls", True, 123, "<Popen: returncode: 123 args: 'ls'>"),
+            ('a' * 100, True, 0,
+             "<Popen: returncode: 0 args: 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa...>"),
+            (["ls"], False, None, "<Popen: returncode: None args: ['ls']>"),
+            (["ls", '--my-opts', 'a' * 100], False, None,
+             "<Popen: returncode: None args: ['ls', '--my-opts', 'aaaaaaaaaaaaaaaaaaaaaaaa...>"),
+            (path_cmd, False, 7, f"<Popen: returncode: 7 args: {pathlib_cls}('my-tool.py')>")
+        ]
+        with unittest.mock.patch.object(subprocess.Popen, '_execute_child'):
+            for cmd, shell, code, sx in cases:
+                p = subprocess.Popen(cmd, shell=shell)
+                p.returncode = code
+                self.assertEqual(repr(p), sx)
 
     def test_communicate_epipe_only_stdin(self):
         # Issue 10963: communicate() should hide EPIPE
diff --git a/Misc/NEWS.d/next/Library/2021-04-12-00-00-00.bpo-43776.p14y7a.rst b/Misc/NEWS.d/next/Library/2021-04-12-00-00-00.bpo-43776.p14y7a.rst
@@ -0,0 +1 @@
+When :class:`subprocess.Popen` args are provided as a string or as :class:`pathlib.Path`, the Popen instance repr now shows the right thing.

Original file line number	Diff line number	Diff line change
`@@ -989,7 +989,7 @@ def __init__(self, args, bufsize=-1, executable=None,`
`989`	`989`	`def __repr__(self):`
`990`	`990`	`obj_repr = (`
`991`	`991`	`f"<{self.__class__.__name__}: "`
`992`		`- f"returncode: {self.returncode} args: {list(self.args)!r}>"`
	`992`	`+ f"returncode: {self.returncode} args: {self.args!r}>"`
`993`	`993`	`)`
`994`	`994`	`if len(obj_repr) > 80:`
`995`	`995`	`obj_repr = obj_repr[:76] + "...>"`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	+When :class:`subprocess.Popen` args are provided as a string or as :class:`pathlib.Path`, the Popen instance repr now shows the right thing.