[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: Ben Reser <ben_at_reser.org>
Date: Fri, 25 Jan 2013 00:00:36 -0800

On Thu, Jan 24, 2013 at 10:35 PM, Prabhu Gnana Sundar
<prabhugs_at_collab.net> wrote:
> But how do we find if the list contains just strings or regexes. I am not
> sure of any concrete idea for this.
>
> We can set is_regex_list = True only if we are sure that the list contains
> regexes.
>
>
> So far, we supported three things
> 1. a list of strings
> 2. a string (regex)
> 3. integer (regex)

We handle it the same way we already do. The expected output, error,
etc values don't have to be just lists, they can also be an instance
of a class. What we're doing now is if what you pass isn't an
instance of ExpectedOutput (which includes all the sub classes
thereof) we assume it is just a list of strings and make an
ExpectedOutput class for you.

In fact we support a lot more than your list above using these
techniques (though I really don't know what you mean by integer
(regex)):

1. A list (autoconverted to ExpectedOutput for you)
2. ExpectedOutput instance which is a list of strings.
match_all=False allows some lines in the actual output, not in the
expected output, ordering still matters though. Otherwise all lines
must match exactly.
2. AnyOutput instance matches as long as there is some output.
3. RegexOutput instance matches a single regex, If match_all=False,
the first regex that matches results in the output being considered
matched. Otherwise every line in the output must match the regex.
4. UnorderedOutput instance matches a list of strings not caring about
the order between the actual and the expected. If match_all=False
then as long as one line in the expected output matches the actual
output it matches. If match_all=True then every line regardless of
order must match.
5. UnorderedRegexOutput instance, subclass of UnorderedOutput, that
accepts multiple regexes but doesn't enforce an order. If
match_all=True every regex provided must match, though not necessarily
in the same order. If match_all=False then as long any regex matches
it is considered successful.

These classes don't just hold the data that it should match but the
implementation of the classes hold the methods to validate the data
against the actual output.

For instance here are two cases from the svnauthz_tests.py which I
recently wrote:

Case #1 I'm verifying a list of strings that only had the explicit
data (this is #1 in the list above).
  expected_data = ['Exit 0\n']
  verify_logfile(logfilepath, expected_data)

vs

Case #2 I'm verifying a regex that doesn't need to match every line:
  expected_data = svntest.verify.RegexOutput("Exit 1", match_all=False)
  verify_logfile(logfilepath, expected_data)

In fact now that I'm looking at that, ExpectedOutput with
match_all=False still is probably what I really wanted instead of
RegexOutput.

So we're talking about wanting a way to want to use a list of regexes
that are still ordered. The logical thing to do would be to extend
ExpectedOutput with another class that does just that. If you take a
look at UnorderedOutput UnorderedRegexOuput all they do is use the
other classes as superclasses and set some flags that change the
behavior.

> I am attaching a patch with this mail which would support both list of
> regexes as well as list of strings. I have checked that all tests pass.
> In the new implementation, we always check for "exact match 'or' re.match"
> which would solve our problem easily. Please let me know if this approach is
> good to go with, since we do not need a new flag nor any new logic.

See above I don't think this is the way to do what you want.

The bigger concern here is not just that the tests pass, but that the
tests are able to detect failures. When you're writing a test, the
work of testing the test is not testing that it passes, but testing
that it fails when the condition it tests is not true.

> Index: tests/cmdline/svntest/verify.py
> ===================================================================
> --- tests/cmdline/svntest/verify.py (revision 1437863)
> +++ tests/cmdline/svntest/verify.py (working copy)
> @@ -165,17 +165,18 @@ class ExpectedOutput:
> def is_equivalent_list(self, expected, actual):
> "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 self.match_all and not actual:
> + # If MATCH_ALL is True and there are no ACTUAL lines, then we are good
> + # to go.
> + return True

This makes no sense to me. Why would we want to accept no input when
given a literal set of strings that we want to match every line for?

> # 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:
> + if (expected[i_expected] == actual_line or
> + re.match(expected[i_expected], actual_line)):
> i_expected += 1
> if i_expected == len(expected):
> return True

This is wrong. The flag says this is not a regex, we shouldn't be
testing it as if it was a regex.
Received on 2013-01-25 09:01:19 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.