[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] Support regex in EXPECTED ERR list

From: Julian Foad <julianfoad_at_btopenworld.com>
Date: Wed, 23 Jan 2013 18:08:23 +0000 (GMT)

----- Original Message -----

> From: Ben Reser <ben_at_reser.org>
> To: Prabhu Gnana Sundar <prabhugs_at_collab.net>
> Cc: "dev_at_subversion.apache.org" <dev_at_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
> <prabhugs_at_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
Received on 2013-01-23 19:08:57 CET

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.