Submitter | Pierre-Yves David |
---|---|
Date | Sept. 7, 2019, 12:16 p.m. |
Message ID | <e9d8154ab03d353fcc52.1567858605@nodosa.octopoid.net> |
Download | mbox | patch |
Permalink | /patch/41537/ |
State | Accepted |
Headers | show |
Comments
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
On 9/7/19 3:03 PM, Joerg Sonnenberger wrote: > 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. This is mostly how the current code flow. I am mostly slicing it up, not doing a deep refactoring.
Patch
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' @@ -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) + if pos in after: postout += after.pop(pos) @@ -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 + @staticmethod def rematch(el, l): try: