STINNER Victor added the comment:
> Even if there turn out to be a few uses where having bytes is required, it
> would probably be worth catering to the common case and making those
> exceptional cases use Popen directly. (Or alternatively provide
> assert_python_xxx_binary helpers.)
I would prefer the opposite: *always* use script_helper rather than Popen()
directly in tests. We have to check why some tests use directly Popen?
I know that in test_faulthandler for example, I chose to use directly Popen
because script_helper always enable faulthandler, there is no option to disable
this behaviour, and as I wrote in my previous comment, it's a pain to extend
the API (I don't want to use yet another __xxx custom keyword).
Maybe we need differently API levels in script helper, the lowest level would
return a Popen object but add -I, -E and/or -X faulthandler.
By the way, I'm using more and more functions like the one I added to
Lib/test/eintrdata/eintr_tester.py:
@contextlib.contextmanager
def kill_on_error(proc):
"""Context manager killing the subprocess if a Python exception is
raised."""
with proc:
try:
yield proc
except:
proc.kill()
raise
Such helper should also be moved to script_helper.
For examle, in my perf project I have these two helper functions:
@contextlib.contextmanager
def popen_killer(proc):
try:
yield
except:
# Close pipes
if proc.stdin:
proc.stdin.close()
if proc.stdout:
proc.stdout.close()
if proc.stderr:
proc.stderr.close()
try:
proc.kill()
except OSError:
# process already terminated
pass
proc.wait()
raise
def popen_communicate(proc):
with popen_killer(proc):
return proc.communicate()
Or maybe we should add back these features directly into the subprocess module?
----------
_______________________________________
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue18299>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe:
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
