Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(4440)

Unified Diff: Lib/test/test_subprocess.py

Issue 26741: subprocess.Popen should emit a ResourceWarning in destructor if the process is still running
Patch Set: Created 3 years, 6 months ago
Use n/p to move between diff chunks; N/P to move between comments. Please Sign in to add in-line comments.
Jump to:
View side-by-side diff with in-line comments
Download patch
« Lib/subprocess.py ('K') | « Lib/subprocess.py ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
--- a/Lib/test/test_subprocess.py Tue Apr 12 22:38:22 2016 +0200
+++ b/Lib/test/test_subprocess.py Wed Apr 13 00:41:49 2016 +0200
@@ -447,8 +447,8 @@ class ProcessTestCase(BaseTestCase):
p = subprocess.Popen([sys.executable, "-c",
'import sys; sys.stdout.write("orange")'],
stdout=subprocess.PIPE)
- self.addCleanup(p.stdout.close)
- self.assertEqual(p.stdout.read(), b"orange")
+ with p:
+ self.assertEqual(p.stdout.read(), b"orange")
def test_stdout_filedes(self):
# stdout is set to open file descriptor
@@ -478,8 +478,8 @@ class ProcessTestCase(BaseTestCase):
p = subprocess.Popen([sys.executable, "-c",
'import sys; sys.stderr.write("strawberry")'],
stderr=subprocess.PIPE)
- self.addCleanup(p.stderr.close)
- self.assertStderrEqual(p.stderr.read(), b"strawberry")
+ with p:
+ self.assertStderrEqual(p.stderr.read(), b"strawberry")
def test_stderr_filedes(self):
# stderr is set to open file descriptor
@@ -513,8 +513,8 @@ class ProcessTestCase(BaseTestCase):
'sys.stderr.write("orange")'],
stdout=subprocess.PIPE,
stderr=subprocess.STDOUT)
- self.addCleanup(p.stdout.close)
- self.assertStderrEqual(p.stdout.read(), b"appleorange")
+ with p:
+ self.assertStderrEqual(p.stdout.read(), b"appleorange")
def test_stdout_stderr_file(self):
# capture stdout and stderr to the same open file
@@ -772,18 +772,19 @@ class ProcessTestCase(BaseTestCase):
stdin=subprocess.PIPE,
stdout=subprocess.PIPE,
universal_newlines=1)
- p.stdin.write("line1\n")
- p.stdin.flush()
- self.assertEqual(p.stdout.readline(), "line1\n")
- p.stdin.write("line3\n")
- p.stdin.close()
- self.addCleanup(p.stdout.close)
- self.assertEqual(p.stdout.readline(),
- "line2\n")
- self.assertEqual(p.stdout.read(6),
- "line3\n")
- self.assertEqual(p.stdout.read(),
- "line4\nline5\nline6\nline7\nline8")
+ with p:
+ p.stdin.write("line1\n")
+ p.stdin.flush()
+ self.assertEqual(p.stdout.readline(), "line1\n")
+ p.stdin.write("line3\n")
+ p.stdin.close()
+ self.addCleanup(p.stdout.close)
Martin Panter 2016/04/13 04:31:30 Can we drop this cleanup now?
+ self.assertEqual(p.stdout.readline(),
+ "line2\n")
+ self.assertEqual(p.stdout.read(6),
+ "line3\n")
+ self.assertEqual(p.stdout.read(),
+ "line4\nline5\nline6\nline7\nline8")
def test_universal_newlines_communicate(self):
# universal newlines through communicate()
@@ -1417,8 +1418,8 @@ class POSIXProcessTestCase(BaseTestCase)
'sys.stdout.write(os.getenv("FRUIT"))'],
stdout=subprocess.PIPE,
preexec_fn=lambda: os.putenv("FRUIT", "apple"))
- self.addCleanup(p.stdout.close)
- self.assertEqual(p.stdout.read(), b"apple")
+ with p:
+ self.assertEqual(p.stdout.read(), b"apple")
def test_preexec_exception(self):
def raise_it():
@@ -1566,8 +1567,8 @@ class POSIXProcessTestCase(BaseTestCase)
p = subprocess.Popen(["echo $FRUIT"], shell=1,
stdout=subprocess.PIPE,
env=newenv)
- self.addCleanup(p.stdout.close)
- self.assertEqual(p.stdout.read().strip(b" \t\r\n\f"), b"apple")
+ with p:
+ self.assertEqual(p.stdout.read().strip(b" \t\r\n\f"), b"apple")
def test_shell_string(self):
# Run command through the shell (string)
@@ -1576,8 +1577,8 @@ class POSIXProcessTestCase(BaseTestCase)
p = subprocess.Popen("echo $FRUIT", shell=1,
stdout=subprocess.PIPE,
env=newenv)
- self.addCleanup(p.stdout.close)
- self.assertEqual(p.stdout.read().strip(b" \t\r\n\f"), b"apple")
+ with p:
+ self.assertEqual(p.stdout.read().strip(b" \t\r\n\f"), b"apple")
def test_call_string(self):
# call() function with string argument on UNIX
@@ -1609,8 +1610,8 @@ class POSIXProcessTestCase(BaseTestCase)
for sh in shells:
p = subprocess.Popen("echo $0", executable=sh, shell=True,
stdout=subprocess.PIPE)
- self.addCleanup(p.stdout.close)
- self.assertEqual(p.stdout.read().strip(), bytes(sh, 'ascii'))
+ with p:
+ self.assertEqual(p.stdout.read().strip(), bytes(sh, 'ascii'))
def _kill_process(self, method, *args):
# Do not inherit file handles from the parent.
@@ -2268,7 +2269,9 @@ class POSIXProcessTestCase(BaseTestCase)
self.addCleanup(p.stderr.close)
ident = id(p)
pid = p.pid
- del p
+ with support.check_warnings(('', ResourceWarning)):
+ p = None
Martin Panter 2016/04/13 04:31:30 I’m a bit curious why you changed “del p” to “p =
+
# check that p is in the active processes list
self.assertIn(ident, [id(o) for o in subprocess._active])
@@ -2287,7 +2290,9 @@ class POSIXProcessTestCase(BaseTestCase)
self.addCleanup(p.stderr.close)
ident = id(p)
pid = p.pid
- del p
+ with support.check_warnings(('', ResourceWarning)):
+ p = None
+
os.kill(pid, signal.SIGKILL)
# check that p is in the active processes list
self.assertIn(ident, [id(o) for o in subprocess._active])
« Lib/subprocess.py ('K') | « Lib/subprocess.py ('k') | no next file » | no next file with comments »

RSS Feeds Recent Issues | This issue
This is Rietveld 894c83f36cb7+