Re: [PATCH] print debug information when subprocess.Popen subprocess.Popen

Please send patches as text/plain (*.txt extension often does this).

Masaru Tsuchiyama wrote on Sun, Jul 28, 2013 at 20:00:27 +0900:
> +++ subversion/tests/cmdline/svntest/main.py (working copy)
> @@ -412,12 +412,19 @@
> if not stderr:
> stderr = subprocess.PIPE
>
> - p = subprocess.Popen(command,
> + try:
> + p = subprocess.Popen(command,
> bufsize,
> stdin=stdin,
> stdout=stdout,
> stderr=stderr,
> close_fds=not windows)
> + except:

Don't catch everything. Here you can/should catch only CalledProcessError.

> + # catch expeption, print information, and reraise
> + print "current dir:", os.path.abspath(os.getcwd())
> + print command

Makes me twitchy. svntest is a library and it shouldn't print, since
this assumes no caller catches the exception. If possible I'd rather
annotate the exception object and re-raise it.

Also you should use a print() when possible for Python 3 compatibility.

I haven't looked at the context, but I assume this is to address the
failure mode of running tests before the svn* binaries are built?

> + raise
> +
> return p.stdin, p.stdout, p.stderr, (p, command_string)
>
> def wait_on_pipe(waiter, binary_mode, stdin=None):

Cheers,

Daniel
Received on 2013-08-01 13:51:56 CEST