From 307e7f0d70dd42d4184e9cac96d244ca3b7efdc3 Mon Sep 17 00:00:00 2001
From: Doug Hellmann <doug@doughellmann.com>
Date: Fri, 25 Aug 2017 16:26:43 -0400
Subject: [PATCH] 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              | 40 +++++++++++++++++++------------------
 whereto/rules.py            | 11 +++++++---
 whereto/tests/test_app.py   |  6 ++----
 whereto/tests/test_rules.py |  9 ++++-----
 4 files changed, 35 insertions(+), 31 deletions(-)

diff --git a/whereto/app.py b/whereto/app.py
index 4c46964..daf71b4 100644
--- a/whereto/app.py
+++ b/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
-    (line, input, expected), and the second element is the list of any
-    rules that did match the input pattern.
+    The mismatched tuples contain the test values (line, input,
+    expected).
 
     :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:
-        matches = list(ruleset.match(input))
-        if len(matches) == 1:
-            match = matches[0]
+    for test in tests:
+        try:
+            linenum, input, code, expected = test
+        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]]))
+        print('No rule matched test on line {}: {}'.format(
+            test[0], ' '.join(test[1:]))
+        )
 
     if untested:
         if not args.quiet:
diff --git a/whereto/rules.py b/whereto/rules.py
index 7a9ac16..764f0af 100644
--- a/whereto/rules.py
+++ b/whereto/rules.py
@@ -102,6 +102,11 @@ class RuleSet(object):
 
     def match(self, path):
         for rule in self:
-            m = rule.match(path)
-            if m is not None:
-                yield (rule.linenum,) + m
+            try:
+                m = rule.match(path)
+            except Exception as e:
+                print('Failed to evaluate {} against {}: {}'.format(
+                    rule, path, e))
+            else:
+                if m is not None:
+                    return (rule.linenum,) + m
diff --git a/whereto/tests/test_app.py b/whereto/tests/test_app.py
index cf0bb87..39fe28c 100644
--- a/whereto/tests/test_app.py
+++ b/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, '301', '/duplicate/redirect')])],
-            {1, 2},
+            [],
+            {2},
         )
         self.assertEqual(expected, actual)
diff --git a/whereto/tests/test_rules.py b/whereto/tests/test_rules.py
index 6c17fce..72fd569 100644
--- a/whereto/tests/test_rules.py
+++ b/whereto/tests/test_rules.py
@@ -174,8 +174,8 @@ class TestRuleSet(base.TestCase):
             'redirect', '301', '/path', '/new/path',
         )
         self.assertEqual(
-            [(1, '301', '/new/path')],
-            list(self.ruleset.match('/path')),
+            (1, '301', '/new/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'),
-             (2, '301', '/other/path')],
-            list(self.ruleset.match('/path')),
+            (1, '301', '/new/path'),
+            self.ruleset.match('/path'),
         )