From 0d3df1ce5c045d0455248673bbfff3a15b6c1517 Mon Sep 17 00:00:00 2001 From: Andrei Daraschenka Date: Thu, 14 Nov 2019 13:26:31 +0300 Subject: [PATCH 1/8] Implement subprocess.Popen.__repr__ --- Lib/subprocess.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Lib/subprocess.py b/Lib/subprocess.py index b5a45d9fea23c5..37f0b698629e49 100644 --- a/Lib/subprocess.py +++ b/Lib/subprocess.py @@ -978,6 +978,10 @@ def __init__(self, args, bufsize=-1, executable=None, raise + def __repr__(self): + return '<%s.%s: pid-%s>' % ( + self.__module__, self.__class__.__name__, self.pid) + @property def universal_newlines(self): # universal_newlines as retained as an alias of text_mode for API From 081c1c3d3b55e05876d87476ef872cd64fd2f538 Mon Sep 17 00:00:00 2001 From: Andrei Daraschenka Date: Thu, 14 Nov 2019 15:11:06 +0300 Subject: [PATCH 2/8] Update after review --- Lib/subprocess.py | 15 +++++++++++-- Lib/test/test_subprocess.py | 21 +++++++++++++++++++ .../2019-11-14-14-13-29.bpo-38724.T5ySfR.rst | 1 + 3 files changed, 35 insertions(+), 2 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2019-11-14-14-13-29.bpo-38724.T5ySfR.rst diff --git a/Lib/subprocess.py b/Lib/subprocess.py index 37f0b698629e49..f45166a53cac62 100644 --- a/Lib/subprocess.py +++ b/Lib/subprocess.py @@ -979,8 +979,19 @@ def __init__(self, args, bufsize=-1, executable=None, raise def __repr__(self): - return '<%s.%s: pid-%s>' % ( - self.__module__, self.__class__.__name__, self.pid) + max_args_length = 30 + args = ' '.join(self.args) + + if len(args) > max_args_length: + args = ('%s...' % args[:max_args_length]).strip() + + format_str = "%s.%s: pid:'%s' args:'%s'" % ( + self.__module__, self.__class__.__name__, self.pid, args) + + if self.returncode: + format_str += " retcode:'%d'" % self.returncode + + return '<%s>' % format_str @property def universal_newlines(self): diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py index 9e96a6d9a87474..bdfc0ced5dc0a9 100644 --- a/Lib/test/test_subprocess.py +++ b/Lib/test/test_subprocess.py @@ -1360,6 +1360,27 @@ def test_communicate_epipe(self): self.addCleanup(p.stdin.close) p.communicate(b"x" * 2**20) + def test_string_representation(self): + """Test stirng representation Popen object without returncode.""" + code = 'import sys; sys.exit(1)' + cmd = [sys.executable, '-c', code] + with subprocess.Popen(cmd) as proc: + args = ('%s...' % ' '.join(cmd)[:30]).strip() + result = "" % ( + proc.pid, args) + self.assertEqual(proc.__repr__(), result) + + def test_string_representation_with_retcode(self): + """Test stirng representation Popen object with returncode.""" + code = 'import sys; sys.exit(1)' + cmd = [sys.executable, '-c', code] + proc = subprocess.Popen(cmd) + proc.wait() + args = ('%s...' % ' '.join(cmd)[:30]).strip() + result = "" % ( + proc.pid, args, proc.returncode) + self.assertEqual(proc.__repr__(), result) + def test_communicate_epipe_only_stdin(self): # Issue 10963: communicate() should hide EPIPE p = subprocess.Popen(ZERO_RETURN_CMD, diff --git a/Misc/NEWS.d/next/Library/2019-11-14-14-13-29.bpo-38724.T5ySfR.rst b/Misc/NEWS.d/next/Library/2019-11-14-14-13-29.bpo-38724.T5ySfR.rst new file mode 100644 index 00000000000000..e5a7fa272516be --- /dev/null +++ b/Misc/NEWS.d/next/Library/2019-11-14-14-13-29.bpo-38724.T5ySfR.rst @@ -0,0 +1 @@ +String representation of an object ``subprocess.Popen.__repr__`` \ No newline at end of file From bd0bccadf93bb282d85f2715fd34ae161334ed06 Mon Sep 17 00:00:00 2001 From: Andrei Daraschenka Date: Fri, 15 Nov 2019 08:54:56 +0300 Subject: [PATCH 3/8] Fix tests and change on format string --- Lib/subprocess.py | 17 ++++++++--------- Lib/test/test_subprocess.py | 31 +++++++++++++++---------------- 2 files changed, 23 insertions(+), 25 deletions(-) diff --git a/Lib/subprocess.py b/Lib/subprocess.py index f45166a53cac62..af3eb1318f6444 100644 --- a/Lib/subprocess.py +++ b/Lib/subprocess.py @@ -47,6 +47,7 @@ import os import time import signal +import shlex import sys import threading import warnings @@ -980,18 +981,16 @@ def __init__(self, args, bufsize=-1, executable=None, def __repr__(self): max_args_length = 30 - args = ' '.join(self.args) + returncode = self.returncode or 'unfinished' + args = ' '.join(map(shlex.quote, self.args)) if len(args) > max_args_length: - args = ('%s...' % args[:max_args_length]).strip() + args = f"{args[:max_args_length]}...".rstrip() - format_str = "%s.%s: pid:'%s' args:'%s'" % ( - self.__module__, self.__class__.__name__, self.pid, args) - - if self.returncode: - format_str += " retcode:'%d'" % self.returncode - - return '<%s>' % format_str + return ( + f"<{self.__module__}.{self.__class__.__name__}: " + f"pid:{self.pid} args:{args} returncode:{returncode}>" + ) @property def universal_newlines(self): diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py index bdfc0ced5dc0a9..1564059dc69c60 100644 --- a/Lib/test/test_subprocess.py +++ b/Lib/test/test_subprocess.py @@ -15,6 +15,7 @@ import sysconfig import select import shutil +import shlex import threading import gc import textwrap @@ -1361,25 +1362,23 @@ def test_communicate_epipe(self): p.communicate(b"x" * 2**20) def test_string_representation(self): - """Test stirng representation Popen object without returncode.""" - code = 'import sys; sys.exit(1)' - cmd = [sys.executable, '-c', code] - with subprocess.Popen(cmd) as proc: - args = ('%s...' % ' '.join(cmd)[:30]).strip() - result = "" % ( - proc.pid, args) - self.assertEqual(proc.__repr__(), result) - - def test_string_representation_with_retcode(self): - """Test stirng representation Popen object with returncode.""" - code = 'import sys; sys.exit(1)' + """Test string representation Popen object.""" + code = 'import sys; sys.exit(57)' cmd = [sys.executable, '-c', code] proc = subprocess.Popen(cmd) + + args = ' '.join(map(shlex.quote, cmd)) + if len(args) > 30: + args = f"{args[:30]}...".rstrip() + + unfinished = ( + f"" + ) + self.assertEqual(repr(proc), unfinished) proc.wait() - args = ('%s...' % ' '.join(cmd)[:30]).strip() - result = "" % ( - proc.pid, args, proc.returncode) - self.assertEqual(proc.__repr__(), result) + finished = unfinished.replace('unfinished', str(proc.returncode)) + self.assertEqual(repr(proc), finished) def test_communicate_epipe_only_stdin(self): # Issue 10963: communicate() should hide EPIPE From 25d79568c05c13700d2e8dfc11041b67398e2c3d Mon Sep 17 00:00:00 2001 From: Andrei Daraschenka Date: Fri, 15 Nov 2019 12:25:44 +0300 Subject: [PATCH 4/8] Update after review --- Lib/subprocess.py | 9 +++---- Lib/test/test_subprocess.py | 27 ++++++++++--------- .../2019-11-14-14-13-29.bpo-38724.T5ySfR.rst | 2 +- 3 files changed, 19 insertions(+), 19 deletions(-) diff --git a/Lib/subprocess.py b/Lib/subprocess.py index af3eb1318f6444..a7a15a565b79a1 100644 --- a/Lib/subprocess.py +++ b/Lib/subprocess.py @@ -980,16 +980,15 @@ def __init__(self, args, bufsize=-1, executable=None, raise def __repr__(self): - max_args_length = 30 - returncode = self.returncode or 'unfinished' + max_args_length = 80 args = ' '.join(map(shlex.quote, self.args)) if len(args) > max_args_length: - args = f"{args[:max_args_length]}...".rstrip() + args = f"{args[:max_args_length-3]}..." return ( - f"<{self.__module__}.{self.__class__.__name__}: " - f"pid:{self.pid} args:{args} returncode:{returncode}>" + f"<{self.__class__.__name__}: " + f"returncode:'{self.returncode}' args:'{args}'>" ) @property diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py index 1564059dc69c60..b01880c011ae9a 100644 --- a/Lib/test/test_subprocess.py +++ b/Lib/test/test_subprocess.py @@ -1363,22 +1363,23 @@ def test_communicate_epipe(self): def test_string_representation(self): """Test string representation Popen object.""" - code = 'import sys; sys.exit(57)' + code = 'import sys; input(); sys.exit(57)' cmd = [sys.executable, '-c', code] - proc = subprocess.Popen(cmd) + max_args_length = 80 args = ' '.join(map(shlex.quote, cmd)) - if len(args) > 30: - args = f"{args[:30]}...".rstrip() - - unfinished = ( - f"" - ) - self.assertEqual(repr(proc), unfinished) - proc.wait() - finished = unfinished.replace('unfinished', str(proc.returncode)) - self.assertEqual(repr(proc), finished) + if len(args) > max_args_length: + args = f"{args[:max_args_length-3]}..." + result = "" + + with subprocess.Popen( + cmd, stdin=subprocess.PIPE, universal_newlines=True) as proc: + self.assertIsNone(proc.returncode) + proc.communicate(input='exit...\n') + self.assertEqual(repr(proc), result.format(proc.returncode, args)) + proc.wait() + self.assertIsNotNone(proc.returncode) + self.assertEqual(repr(proc), result.format(proc.returncode, args)) def test_communicate_epipe_only_stdin(self): # Issue 10963: communicate() should hide EPIPE diff --git a/Misc/NEWS.d/next/Library/2019-11-14-14-13-29.bpo-38724.T5ySfR.rst b/Misc/NEWS.d/next/Library/2019-11-14-14-13-29.bpo-38724.T5ySfR.rst index e5a7fa272516be..a5ebfb33229994 100644 --- a/Misc/NEWS.d/next/Library/2019-11-14-14-13-29.bpo-38724.T5ySfR.rst +++ b/Misc/NEWS.d/next/Library/2019-11-14-14-13-29.bpo-38724.T5ySfR.rst @@ -1 +1 @@ -String representation of an object ``subprocess.Popen.__repr__`` \ No newline at end of file +Add a repr for ``subprocess.Popen`` objects. Patch by Andrey Doroschenko. \ No newline at end of file From ff9d3da694b596127913c0a27856e59f32e8e357 Mon Sep 17 00:00:00 2001 From: Andrei Daraschenka Date: Sat, 16 Nov 2019 12:10:19 +0300 Subject: [PATCH 5/8] Update after review --- Lib/subprocess.py | 13 +++++-------- Lib/test/test_subprocess.py | 13 +++++-------- 2 files changed, 10 insertions(+), 16 deletions(-) diff --git a/Lib/subprocess.py b/Lib/subprocess.py index a7a15a565b79a1..ce2ae9c1b18fc5 100644 --- a/Lib/subprocess.py +++ b/Lib/subprocess.py @@ -980,16 +980,13 @@ def __init__(self, args, bufsize=-1, executable=None, raise def __repr__(self): - max_args_length = 80 - args = ' '.join(map(shlex.quote, self.args)) - - if len(args) > max_args_length: - args = f"{args[:max_args_length-3]}..." - - return ( + obj_repr = ( f"<{self.__class__.__name__}: " - f"returncode:'{self.returncode}' args:'{args}'>" + f"returncode: {self.returncode} args: {list(self.args)!r}>" ) + if len(obj_repr) > 80: + obj_repr = obj_repr[:76] + "...>" + return obj_repr @property def universal_newlines(self): diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py index b01880c011ae9a..590a5f6d345c50 100644 --- a/Lib/test/test_subprocess.py +++ b/Lib/test/test_subprocess.py @@ -1365,21 +1365,18 @@ def test_string_representation(self): """Test string representation Popen object.""" code = 'import sys; input(); sys.exit(57)' cmd = [sys.executable, '-c', code] - - max_args_length = 80 - args = ' '.join(map(shlex.quote, cmd)) - if len(args) > max_args_length: - args = f"{args[:max_args_length-3]}..." - result = "" + result = " Date: Sun, 17 Nov 2019 09:22:15 +0300 Subject: [PATCH 6/8] Remove unused import --- Lib/subprocess.py | 1 - Lib/test/test_subprocess.py | 1 - 2 files changed, 2 deletions(-) diff --git a/Lib/subprocess.py b/Lib/subprocess.py index ce2ae9c1b18fc5..ba6f1983a5a227 100644 --- a/Lib/subprocess.py +++ b/Lib/subprocess.py @@ -47,7 +47,6 @@ import os import time import signal -import shlex import sys import threading import warnings diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py index 590a5f6d345c50..402ebc65ca01f7 100644 --- a/Lib/test/test_subprocess.py +++ b/Lib/test/test_subprocess.py @@ -15,7 +15,6 @@ import sysconfig import select import shutil -import shlex import threading import gc import textwrap From 0464fa8944dddee4a9e1e540681942c1039cc37a Mon Sep 17 00:00:00 2001 From: Tal Einat Date: Sun, 17 Nov 2019 09:13:32 +0200 Subject: [PATCH 7/8] cleanup test a bit --- Lib/test/test_subprocess.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py index 402ebc65ca01f7..30598cce3de946 100644 --- a/Lib/test/test_subprocess.py +++ b/Lib/test/test_subprocess.py @@ -1360,8 +1360,9 @@ def test_communicate_epipe(self): self.addCleanup(p.stdin.close) p.communicate(b"x" * 2**20) - def test_string_representation(self): - """Test string representation Popen object.""" + 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 = "') and + 'import sys' in repr(proc) + ) + + proc.communicate(input='exit...\n') proc.wait() + self.assertIsNotNone(proc.returncode) self.assertTrue( repr(proc).startswith(result.format(proc.returncode))) From efc0c55aed7a4bf8ad37aff1ace6d4d9dd5a788e Mon Sep 17 00:00:00 2001 From: Tal Einat Date: Sun, 17 Nov 2019 11:31:03 +0200 Subject: [PATCH 8/8] fix failing test --- Lib/test/test_subprocess.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py index 30598cce3de946..97dc09c564965a 100644 --- a/Lib/test/test_subprocess.py +++ b/Lib/test/test_subprocess.py @@ -1372,8 +1372,7 @@ def test_repr(self): self.assertIsNone(proc.returncode) self.assertTrue( repr(proc).startswith(result.format(proc.returncode)) and - repr(proc).endswith('>') and - 'import sys' in repr(proc) + repr(proc).endswith('>') ) proc.communicate(input='exit...\n') @@ -1381,7 +1380,9 @@ def test_repr(self): self.assertIsNotNone(proc.returncode) self.assertTrue( - repr(proc).startswith(result.format(proc.returncode))) + repr(proc).startswith(result.format(proc.returncode)) and + repr(proc).endswith('>') + ) def test_communicate_epipe_only_stdin(self): # Issue 10963: communicate() should hide EPIPE