accept the first match as the result

Rather than reporting multiple matches, stop evaluating rules when we
get the first match. This will still report redirect failures, and
avoids a false-negative when a more specific rule appears before a
more general rule and the more specific rule is valid.

Change-Id: Ie909301a367659a0ce199000342bcdc9422ce801
Signed-off-by: Doug Hellmann <doug@doughellmann.com>

whereto/app.py

@@ -31,9 +31,8 @@ def process_tests(ruleset, tests):
     inputs that did not match the expected value, and a set containing
     the line numbers of the rules that never matched an input test.
 
-    The first element of the mismatched tuples is the test tuple
+    The mismatched tuples contain the test values (line, input,
-    (line, input, expected), and the second element is the list of any
+    expected).
-    rules that did match the input pattern.
 
     :param ruleset: The redirect rules.
     :type ruleset: RuleSet
@@ -41,15 +40,25 @@ def process_tests(ruleset, tests):
     """
     used = set()
     mismatches = []
-    for linenum, input, code, expected in tests:
+    for test in tests:
-        matches = list(ruleset.match(input))
+        try:
-        if len(matches) == 1:
+            linenum, input, code, expected = test
-            match = matches[0]
+        except ValueError as e:
+            linenum = test[0]
+            if len(test) != 4:
+                raise ValueError(
+                    'Wrong number of arguments in test on line {}: {}'.format(
+                        linenum, ' '.join(test[1:]))
+                )
+            raise RuntimeError('Unable to process test {}: {}'.format(
+                test, e))
+        match = ruleset.match(input)
+        if match is not None:
             if (code, expected) == match[1:]:
                 used.add(match[0])
                 continue
         mismatches.append(
-            ((linenum, input, code, expected), matches)
+            (linenum, input, code, expected)
         )
     untested = set(ruleset.all_ids) - used
     return (mismatches, untested)
@@ -104,18 +113,11 @@ def main():
 
     failures = 0
     mismatches, untested = process_tests(ruleset, tests)
-    for test, matches in mismatches:
+    for test in mismatches:
         failures += 1
-        if not matches:
         print('No rule matched test on line {}: {}'.format(
             test[0], ' '.join(test[1:]))
         )
-        else:
-            print('Test on line {} did not produce expected result: {}'.format(
-                test[0], ' '.join(test[1:]))
-            )
-            for match in matches:
-                print('  {}'.format(ruleset[match[0]]))
 
     if untested:
         if not args.quiet:

whereto/rules.py

@@ -102,6 +102,11 @@ class RuleSet(object):
 
     def match(self, path):
         for rule in self:
+            try:
                 m = rule.match(path)
+            except Exception as e:
+                print('Failed to evaluate {} against {}: {}'.format(
+                    rule, path, e))
+            else:
                 if m is not None:
-                yield (rule.linenum,) + m
+                    return (rule.linenum,) + m

whereto/tests/test_app.py

@@ -45,9 +45,7 @@ class TestProcessTests(base.TestCase):
             [(1, '/path', '301', '/new/path')],
         )
         expected = (
-            [((1, '/path', '301', '/new/path'),
+            [],
-              [(1, '301', '/new/path'),
+            {2},
-               (2, '301', '/duplicate/redirect')])],
-            {1, 2},
         )
         self.assertEqual(expected, actual)

whereto/tests/test_rules.py

@@ -174,8 +174,8 @@ class TestRuleSet(base.TestCase):
             'redirect', '301', '/path', '/new/path',
         )
         self.assertEqual(
-            [(1, '301', '/new/path')],
+            (1, '301', '/new/path'),
-            list(self.ruleset.match('/path')),
+            self.ruleset.match('/path'),
         )
 
     def test_match_multiple(self):
@@ -188,7 +188,6 @@ class TestRuleSet(base.TestCase):
             'redirect', '301', '/path', '/other/path',
         )
         self.assertEqual(
-            [(1, '301', '/new/path'),
+            (1, '301', '/new/path'),
-             (2, '301', '/other/path')],
+            self.ruleset.match('/path'),
-            list(self.ruleset.match('/path')),
         )