From 98e573a768aaccb9532a1849b9267ea86a7a8adf Mon Sep 17 00:00:00 2001 From: Michael Date: Sat, 10 Apr 2021 16:47:04 -0700 Subject: [PATCH 1/6] Remove list call from args in Popen repr --- Lib/subprocess.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/subprocess.py b/Lib/subprocess.py index 2b785496e4f5f31..8203aded7d55290 100644 --- a/Lib/subprocess.py +++ b/Lib/subprocess.py @@ -1003,7 +1003,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] + "...>" From 9075d67f1c96704408863d23dc6ba6c1c52e2e66 Mon Sep 17 00:00:00 2001 From: Michael Date: Mon, 12 Apr 2021 22:59:21 -0700 Subject: [PATCH 2/6] Add repr test for args as str to Popen --- Lib/test/test_subprocess.py | 46 ++++++++++++++++++++++--------------- 1 file changed, 28 insertions(+), 18 deletions(-) diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py index 70b54f4155a9a5c..374fc90a3d4ad95 100644 --- a/Lib/test/test_subprocess.py +++ b/Lib/test/test_subprocess.py @@ -1444,26 +1444,36 @@ def test_communicate_epipe(self): 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 = "') - ) + # args can be Seq[str] or a str, hence two test cases + expected_exit_code = 57 + code = f'import sys; input(); sys.exit({expected_exit_code})' + seq_cmd = [sys.executable, '-c', code] + arg_cmd = f"\"{sys.executable}\" -c \"{code}\"" + + cmd_shell = [(seq_cmd, False), (arg_cmd, True)] + + def to_result(exit_code, cmd): + sx = f" 80: + sx = sx[:76] + "...>" + return sx + + for cmd, shell in cmd_shell: + with subprocess.Popen( + cmd, stdin=subprocess.PIPE, + universal_newlines=True, shell=shell) as proc: + self.assertIsNone(proc.returncode) + self.assertEqual( + repr(proc), to_result(proc.returncode, cmd) + ) - proc.communicate(input='exit...\n') - proc.wait() + proc.communicate(input='exit...\n') + proc.wait() - self.assertIsNotNone(proc.returncode) - self.assertTrue( - repr(proc).startswith(result.format(proc.returncode)) and - repr(proc).endswith('>') - ) + self.assertEqual(proc.returncode, expected_exit_code) + self.assertEqual( + repr(proc), to_result(proc.returncode, cmd) + ) def test_communicate_epipe_only_stdin(self): # Issue 10963: communicate() should hide EPIPE From 3a97627828b8cb716593c5291584137bdfcab583 Mon Sep 17 00:00:00 2001 From: Michael Date: Mon, 12 Apr 2021 23:42:27 -0700 Subject: [PATCH 3/6] Add blurb for Popen repr fixes --- .../NEWS.d/next/Library/2021-04-12-00-00-00.bpo-43776.p14y7a.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Library/2021-04-12-00-00-00.bpo-43776.p14y7a.rst 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 new file mode 100644 index 000000000000000..a932f45ae9b05d6 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2021-04-12-00-00-00.bpo-43776.p14y7a.rst @@ -0,0 +1 @@ +Fix issue with :class:`subprocess.Popen` repr when args are provided as a string. \ No newline at end of file From e0867c070ef95e768c5906232f275a70a64c8ef0 Mon Sep 17 00:00:00 2001 From: Michael Date: Sat, 24 Apr 2021 20:47:30 -0700 Subject: [PATCH 4/6] Convert Popen repr test to use mock --- Lib/test/test_subprocess.py | 47 ++++++++++++------------------------- 1 file changed, 15 insertions(+), 32 deletions(-) diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py index 374fc90a3d4ad95..ee3ef5fa9fa0d0e 100644 --- a/Lib/test/test_subprocess.py +++ b/Lib/test/test_subprocess.py @@ -23,6 +23,7 @@ import gc import textwrap import json +import pathlib from test.support.os_helper import FakePath try: @@ -1442,38 +1443,20 @@ 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. - # args can be Seq[str] or a str, hence two test cases - expected_exit_code = 57 - code = f'import sys; input(); sys.exit({expected_exit_code})' - seq_cmd = [sys.executable, '-c', code] - arg_cmd = f"\"{sys.executable}\" -c \"{code}\"" - - cmd_shell = [(seq_cmd, False), (arg_cmd, True)] - - def to_result(exit_code, cmd): - sx = f" 80: - sx = sx[:76] + "...>" - return sx - - for cmd, shell in cmd_shell: - with subprocess.Popen( - cmd, stdin=subprocess.PIPE, - universal_newlines=True, shell=shell) as proc: - self.assertIsNone(proc.returncode) - self.assertEqual( - repr(proc), to_result(proc.returncode, cmd) - ) - - proc.communicate(input='exit...\n') - proc.wait() - - self.assertEqual(proc.returncode, expected_exit_code) - self.assertEqual( - repr(proc), to_result(proc.returncode, cmd) - ) + cases = [ + ("ls", True, 123, ""), + ('a' * 100, True, 0, + ""), + (["ls"], False, None, ""), + (["ls", '--my-opts', 'a' * 100], False, None, + ""), + (pathlib.Path("my-tool.py"), False, 7, "") + ] + 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 From 10401baa7a1b802548f84422673222069fbd2f22 Mon Sep 17 00:00:00 2001 From: Michael Date: Sat, 24 Apr 2021 20:47:54 -0700 Subject: [PATCH 5/6] Improve blurb --- .../next/Library/2021-04-12-00-00-00.bpo-43776.p14y7a.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 index a932f45ae9b05d6..51bc791f10d31a4 100644 --- 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 @@ -1 +1 @@ -Fix issue with :class:`subprocess.Popen` repr when args are provided as a string. \ No newline at end of file +When :class:`subprocess.Popen` args are provided as a string or as :class:`pathlib.Path`, the Popen instance repr now shows the right thing. \ No newline at end of file From 793ab392c9a384bdfbad2721cb154e8b3b2f48c8 Mon Sep 17 00:00:00 2001 From: Michael Date: Sat, 24 Apr 2021 21:44:25 -0700 Subject: [PATCH 6/6] Fix pathlib test case --- Lib/test/test_subprocess.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py index ee3ef5fa9fa0d0e..7c79365f4119141 100644 --- a/Lib/test/test_subprocess.py +++ b/Lib/test/test_subprocess.py @@ -1443,6 +1443,9 @@ def test_communicate_epipe(self): p.communicate(b"x" * 2**20) def test_repr(self): + path_cmd = pathlib.Path("my-tool.py") + pathlib_cls = path_cmd.__class__.__name__ + cases = [ ("ls", True, 123, ""), ('a' * 100, True, 0, @@ -1450,7 +1453,7 @@ def test_repr(self): (["ls"], False, None, ""), (["ls", '--my-opts', 'a' * 100], False, None, ""), - (pathlib.Path("my-tool.py"), False, 7, "") + (path_cmd, False, 7, f"") ] with unittest.mock.patch.object(subprocess.Popen, '_execute_child'): for cmd, shell, code, sx in cases: