Merge lp:~jml/pkgme/index-error into lp:pkgme

[Jonathan Lange]

Properly handle no eligible backends.

Fixes a bug where we get IndexErrors when there's no eligible backend.

There are now some tests. Yay.

[James Westby (james-w)]

review: Approve

=== modified file 'NEWS.txt'
--- NEWS.txt	2012-09-06 16:24:31 +0000
+++ NEWS.txt	2012-09-12 09:06:20 +0000
@@ -9,6 +9,9 @@
 Changes
 -------
 
+ * ``query_backends`` raises a ``NoEligibleBackend`` error if there are no
+   eligible backends.  (Jonathan Lange).
+
 Improvements
 ------------
 

=== modified file 'pkgme/backend.py'
--- pkgme/backend.py	2012-08-31 11:42:33 +0000
+++ pkgme/backend.py	2012-09-12 09:06:20 +0000
@@ -308,7 +308,12 @@
 
 
 def choose_backend(eligible_backends):
-    return eligible_backends[0]['backend']
+    """Choose the most eligible backend.
+
+    If ``eligible_backends`` is empty, then raises an IndexError.
+    """
+    bs = sorted(eligible_backends, key=itemgetter('score'), reverse=True)
+    return bs[0]['backend']
 
 
 def query_backends(path, backends, allowed_backend_names=None):
@@ -321,6 +326,10 @@
             lambda b: b.name in allowed_backend_names, backends)
     results = _query_backends(path, allowed)
     ineligible, eligible = dichotomy(_is_eligible, results)
+    if not eligible:
+        raise NoEligibleBackend(
+            path, ineligible, disallowed_backends=disallowed)
+    # XXX: Can maybe get rid of this.
     eligible.sort(key=itemgetter('score'), reverse=True)
     return disallowed, ineligible, eligible
 
@@ -350,9 +359,6 @@
         """
         disallowed, ineligible, eligible = query_backends(
             path, self.backends, self.allowed_backend_names)
-        if not eligible:
-            raise NoEligibleBackend(
-                path, ineligible, disallowed_backends=disallowed)
         return eligible
 
     def get_info(self, path):

=== modified file 'pkgme/tests/test_backend.py'
--- pkgme/tests/test_backend.py	2012-09-03 16:14:08 +0000
+++ pkgme/tests/test_backend.py	2012-09-12 09:06:20 +0000
@@ -11,6 +11,7 @@
 from ..backend import (
     Backend,
     BackendSelector,
+    choose_backend,
     EXTERNAL_BACKEND_PATHS,
     ExternalHelpersBackend,
     ExternalHelpersBackendLoader,
@@ -19,6 +20,7 @@
     get_info_for,
     NoBackend,
     NoEligibleBackend,
+    query_backends,
     StaticBackend,
     StaticLoader,
     WantError,
@@ -489,6 +491,60 @@
         self.assertEqual(EXTERNAL_BACKEND_PATHS, loader.paths)
 
 
+class ChooseBackendTests(TestCase):
+
+    def test_first_backend(self):
+        b1 = object()
+        b2 = object()
+        backends = [{'score': 10, 'backend': b1}, {'score': 20, 'backend': b2}]
+        self.assertIs(b2, choose_backend(backends))
+
+    def test_no_backends(self):
+        self.assertRaises(IndexError, choose_backend, [])
+
+
+class QueryBackendsTests(TestCase):
+
+    def make_backend(self, name=None, score=0, info=None, reason=None):
+        if name is None:
+            name = self.getUniqueString('name')
+        if info is None:
+            info = {}
+        if reason is None:
+            reason = self.getUniqueString('reason')
+        return StaticBackend(name, score, info, reason=reason)
+
+    def to_query(self, backends):
+        return sorted(
+            [{'score': b.score, 'reason': b.reason, 'backend': b}
+             for b in backends])
+
+    def test_no_backends(self):
+        self.assertRaises(NoBackend, query_backends, '.', [])
+
+    def test_no_eligible_backends(self):
+        backends = [self.make_backend(score=0), self.make_backend(score=0)]
+        self.assertRaises(NoEligibleBackend, query_backends, '.', backends)
+
+    def test_eligible_backends_only(self):
+        eligible = [self.make_backend(score=10), self.make_backend(score=20)]
+        expected_eligible = self.to_query(eligible)
+        result = query_backends('.', eligible)
+        self.assertEqual(
+            ([], [], expected_eligible),
+            (result[0], result[1], sorted(result[2])))
+
+    def test_ineligible_backends(self):
+        eligible = [self.make_backend(score=10), self.make_backend(score=20)]
+        expected_eligible = self.to_query(eligible)
+        ineligible = [self.make_backend() for i in range(3)]
+        expected_ineligible = self.to_query(ineligible)
+        result = query_backends('.', eligible + ineligible)
+        self.assertEqual(
+            ([], expected_ineligible, expected_eligible),
+            (result[0], sorted(result[1]), sorted(result[2])))
+
+
 class TestSelector(BackendSelector):
 
     def get_info(self, path):