[PATCH 6 of 9] run-tests: extract a `process_cmd_line` from the main function

Joerg Sonnenberger joerg at bec.de
Sat Sep 7 09:03:25 EDT 2019


On Sat, Sep 07, 2019 at 02:16:45PM +0200, Pierre-Yves David wrote:
> diff --git a/tests/run-tests.py b/tests/run-tests.py
> --- a/tests/run-tests.py
> +++ b/tests/run-tests.py
> @@ -1619,6 +1619,7 @@ class TTest(Test):
>              if salt in out_rawline:
>                  out_line, cmd_line = out_rawline.split(salt, 1)
>  
> +
>              while out_line:
>                  if not out_line.endswith(b'\n'):
>                      out_line += b' (no-eol)\n'

Extra newline?

> @@ -1699,15 +1700,8 @@ class TTest(Test):
>                                  continue
>                      postout.append(b'  ' + el)
>  
> -            if cmd_line:
> -                # Add on last return code.
> -                ret = int(cmd_line.split()[1])
> -                if ret != 0:
> -                    postout.append(b'  [%d]\n' % ret)
> -                if pos in after:
> -                    # Merge in non-active test bits.
> -                    postout += after.pop(pos)
> -                pos = int(cmd_line.split()[0])
> +            pos, postout = self._process_cmd_line(cmd_line, pos, postout, after)
> +

I find the behavior with postout here a bit surprising. It is modified
in place, but also returned as value. IMO it should be either one or the
other? Also extra newline.

> @@ -1717,6 +1711,19 @@ class TTest(Test):
>  
>          return exitcode, postout
>  
> +    def _process_cmd_line(self, cmd_line, pos, postout, after):
> +        """process a "command" part of a line from unified test output"""
> +        if cmd_line:
> +            # Add on last return code.
> +            ret = int(cmd_line.split()[1])
> +            if ret != 0:
> +                postout.append(b'  [%d]\n' % ret)
> +            if pos in after:
> +                # Merge in non-active test bits.
> +                postout += after.pop(pos)
> +            pos = int(cmd_line.split()[0])
> +        return pos, postout
> +

Does it make sense to mark it as staticmethod?

Joerg


More information about the Mercurial-devel mailing list