Author haypo
Recipients BreamoreBoy, Pranav Deshpande, barry, haypo, r.david.murray
Date 2017-05-16.21:25:39
SpamBayes Score -1.0
Marked as misclassified Yes
Message-id <1494969939.54.0.0751433582497.issue18299@psf.upfronthosting.co.za>
In-reply-to
Content
> 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?
History
Date User Action Args
2017-05-16 21:25:39hayposetrecipients: + haypo, barry, r.david.murray, BreamoreBoy, Pranav Deshpande
2017-05-16 21:25:39hayposetmessageid: <1494969939.54.0.0751433582497.issue18299@psf.upfronthosting.co.za>
2017-05-16 21:25:39haypolinkissue18299 messages
2017-05-16 21:25:39haypocreate