[PATCH 2 of 3] run-tests: defer leftover (?) cleanup until after all output is exhausted

[Matt Harbison]

On Tue, 01 Mar 2016 10:49:39 -0500, Yuya Nishihara <yuya at tcha.org> wrote:

> On Mon, 29 Feb 2016 01:14:28 -0500, Matt Harbison wrote:
>> # HG changeset patch
>> # User Matt Harbison <matt_harbison at yahoo.com>
>> # Date 1456719688 18000
>> #      Sun Feb 28 23:21:28 2016 -0500
>> # Node ID 7afd6d67701e7057883895324647facd50ff8db8
>> # Parent  ceb7ce2df958755ffc5c9f0c98d13de991f6b192
>> run-tests: defer leftover (?) cleanup until after all output is  
>> exhausted
>>
>> Previously, after matching a single line, any contiguous subsequent  
>> lines ending
>> with (?) would be added to the output and removed from the expected  
>> output.
>> This is a problem if the subsequent test output would have matched the  
>> consumed
>> (?) line, because it kept the optional line and then added a duplicate  
>> without
>> the (?) [1].  Instead, wait until there is nothing more to match before  
>> handling
>> the leftovers.
>>
>> [1]  
>> https://www.mercurial-scm.org/pipermail/mercurial-devel/2016-February/080197.html
>>
>> diff --git a/tests/run-tests.py b/tests/run-tests.py
>> --- a/tests/run-tests.py
>> +++ b/tests/run-tests.py
>> @@ -1137,14 +1137,13 @@
>>                      elif warnonly == 1: # Is "not yet" and line is  
>> warn only.
>>                          warnonly = 2 # Yes do warn.
>>                  break
>> -
>> -            # clean up any optional leftovers
>> -            while expected.get(pos, None):
>> -                el = expected[pos].pop(0)
>> -                if not el.endswith(b" (?)\n"):
>> -                    expected[pos].insert(0, el)
>> -                    break
>> -                postout.append(b'  ' + el)
>> +            else:
>> +                # clean up any optional leftovers
>> +                while expected.get(pos, None):
>> +                    el = expected[pos].pop(0)
>> +                    if el and not el.endswith(b" (?)\n"):
>> +                        break
>
> Noob question: can we drop unmatched "el" here? Previously we did push  
> back it.

I think it is OK, but I don't have a good understanding of this myself.  I  
can't get it to fail with this code + push back, or the old code without  
it.  So it's not clear what purpose it served.

It appears that 'expected' is a dict of lists, and each list is lines of  
expected output for one command in the test.  After this loop, the 'pos'  
value is modified, so the current list is not examined again, unless lcmd  
is None somehow.

I can put it back if you want, since it doesn't seem to affect anything  
one way or the other.

>> +  + printf 'abc\ndef\nxyz\n'
>
> With Debian dash, quotes are removed.
>
>    *SALT* 6 0 (glob)
> -  + printf 'abc\ndef\nxyz\n'
> +  + printf abc\ndef\nxyz\n
>    abc

Is dash happy with it globbed away?  It doesn't print correctly (on  
Windows anyway) without quoting of some kind.

> Other than that, the series looks good to me.