[PATCH 5 of 5] run-tests: use subprocess.Popen on all systems
Simon Heimberg
simohe at besonet.ch
Wed Feb 9 14:18:05 CST 2011
Am Mittwoch, den 09.02.2011, 18:32 +0100 schrieb Martin Geisler:
> Simon Heimberg <simohe at besonet.ch> writes:
>
> > # HG changeset patch
> > # User Simon Heimberg <simohe at besonet.ch>
> > # Date 1296593286 -3600
> > # Node ID 41b708fd0aa6167ac264846ff2141f61a1cf5904
> > # Parent 797b0e0c0d239e78439b373e5e388ec32cddb039
> > run-tests: use subprocess.Popen on all systems
>
> Great idea, but see below :)
>
> > diff -r 797b0e0c0d23 -r 41b708fd0aa6 tests/run-tests.py
> > --- a/tests/run-tests.py Die Feb 08 07:20:34 2011 +0100
> > +++ b/tests/run-tests.py Die Feb 01 21:48:06 2011 +0100
> > @@ -584,19 +584,11 @@
> > def run(cmd, options, replacements):
> > """Run command in a sub-process, capturing the output (stdout and stderr).
> > Return a tuple (exitcode, output). output is None in debug mode."""
> > - # TODO: Use subprocess.Popen if we're running on Python 2.4
> > if options.debug:
> > proc = subprocess.Popen(cmd, shell=True)
> > ret = proc.wait()
> > return (ret, None)
> >
> > - if os.name == 'nt' or sys.platform.startswith('java'):
> > - tochild, fromchild = os.popen4(cmd)
> > - tochild.close()
> > - output = fromchild.read()
> > - ret = fromchild.close()
> > - if ret is None:
> > - ret = 0
> > else:
> > proc = Popen4(cmd)
> > def cleanup():
>
> This looks wrong -- I would expect the code in the else-branch to be
> dedented one level now that the if-statement disappears?
Agree, it looks unclear.
Because the if statement i merge the else with contains a return, it
works. The deleted if could also be written as elif without changing the
functionality.
Like this, the patch is much smaller. :-)
(And personally i prefer elif even when if contains a return. But this
is a matter of taste.)
Greetings,
Simon
More information about the Mercurial-devel
mailing list