[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