----- Original Message -----
> From: Ben Reser <b...@reser.org>
> To: Prabhu Gnana Sundar <prabh...@collab.net>
> Cc: "dev@subversion.apache.org" <dev@subversion.apache.org>
> Sent: Wednesday, 23 January 2013, 11:19
> Subject: Re: [PATCH] Support regex in EXPECTED ERR list
>
> On the whole I'm going to say that this patch is not the right
> approach to doing what you want. Given the other email I sent I think
> what you want to do is reasonable. But should be handled by adjusting
> the is_regex path in ExpectedOutput to be able to handle lists of
> regex.
>
> On Tue, Jan 22, 2013 at 11:38 PM, Prabhu Gnana Sundar
> <prabh...@collab.net> wrote:
>> Index: tests/cmdline/svntest/verify.py
>> ===================================================================
>> --- tests/cmdline/svntest/verify.py (revision 1427745)
>> +++ tests/cmdline/svntest/verify.py (working copy)
>> @@ -166,16 +166,20 @@ class ExpectedOutput:
>> "Return whether EXPECTED and ACTUAL are equivalent."
>> if not self.is_regex:
>> if self.match_all:
>> - # The EXPECTED lines must match the ACTUAL lines, one-to-one, in
>> - # the same order.
>> - return expected == actual
>> + if not actual:
>> + return True
>
> You really don't want to mess with the existing 1:1 matching like
> you've done here.
>
>> # The EXPECTED lines must match a subset of the ACTUAL lines,
>> # one-to-one, in the same order, with zero or more other ACTUAL
>> # lines interspersed among the matching ACTUAL lines.
>> i_expected = 0
>> for actual_line in actual:
>> - if expected[i_expected] == actual_line:
>> + # As soon an actual_line matches something, then we're good.
>> + # Also check if the regex in the EXPECTED line matches the
>> + # corresponding ACTUAL line.
>> + if ((expected[i_expected] == actual_line) or
>> + (expected[i_expected].startswith(".*") and
>> + re.match(expected[i_expected], actual_line))):
>> i_expected += 1
>> if i_expected == len(expected):
>> return True
>
> Looking for .* is not a valid way of determining if something is a
> regex. If you want to match the first character then you'll end up
> having to put a bogus .* in your pattern to deal with this.
>
> If you look just below the code you changed you'll see:
> expected_re = expected[0]
>
> I'd look at that and adjust it.
>
> Now the question is how. I can see two ways:
>
> 1) list of regular expressions, as long as one matches any single line
> with match_all=False then it's good and if match_all=True then every
> line must match at least one of the expressions.
That's no better than the current typical usage of
RegexOutput('regex1|regex2|regex3|...', match_all=...), is it?
> 2) An ordered list of regular expressions where one is consumed for each line.
>
> The first way is easier to maintain consistency with the current
> implementation. If you really want the second way then I'd suggest
> subclassing RegexOutput with another class like say RegexListOutput,
> add another flag to the implementation e.g. is_regex_list = True, and
> create a 3rd code path in is_equivalent_list.
Lots of our tests contain code that tries to work around the limitations or
simply doesn't test for as strict a match as it ought to, because the available
Output classes don't provide a way to match a sequence of lines in a specific
order.
So, implementing (2) would be useful.
- Julian
