Improve exception message for invalid filter result (carltongibson#943)

* Improve exception message for invalid filter calls

The current exception is an AttributeError that lacks any context about
the filter call. e.g., "NoneType has no attribute filter". This message
is improved by including which filterset and which filter are incorrect.

* Fix tests broken by new exception

django_filters/filterset.py

@@ -222,6 +222,9 @@ def filter_queryset(self, queryset):
         """
         for name, value in self.form.cleaned_data.items():
             queryset = self.filters[name].filter(queryset, value)
+            assert isinstance(queryset, models.QuerySet), \
+                "Expected '%s.%s' to return a QuerySet, but got a %s instead." \
+                % (type(self).__name__, name, type(queryset).__name__)
         return queryset
 
     @property

tests/test_filtering.py

@@ -45,6 +45,7 @@
     SpacewalkRecord,
     User
 )
+from .utils import MockQuerySet
 
 
 class CharFilterTests(TestCase):
@@ -1894,10 +1895,8 @@ class Meta:
                 model = User
                 fields = ['account']
 
-        qs = mock.NonCallableMagicMock()
-        f = F({'account': 'jdoe'}, queryset=qs)
-        result = f.qs
-        self.assertNotEqual(qs, result)
+        qs = MockQuerySet()
+        F({'account': 'jdoe'}, queryset=qs).qs
         qs.all.return_value.filter.assert_called_with(username__exact='jdoe')
 
     def test_filtering_without_meta(self):

tests/test_filterset.py

@@ -40,6 +40,7 @@
     UUIDTestModel,
     Worker
 )
+from .utils import MockQuerySet
 
 
 def checkItemsEqual(L1, L2):
@@ -655,12 +656,14 @@ def test_creating_with_request(self):
 class FilterSetQuerysetTests(TestCase):
 
     class F(FilterSet):
+        invalid = CharFilter(method=lambda *args: None)
+
         class Meta:
             model = User
-            fields = ['username']
+            fields = ['username', 'invalid']
 
     def test_filter_queryset_called_once(self):
-        m = mock.Mock()
+        m = MockQuerySet()
         f = self.F({'username': 'bob'}, queryset=m)
 
         with mock.patch.object(f, 'filter_queryset',
@@ -693,7 +696,7 @@ def test_form_caching(self):
         self.assertIs(f.form, f.form)
 
     def test_qs_triggers_form_validation(self):
-        m = mock.Mock()
+        m = MockQuerySet()
         f = self.F({'username': 'bob'}, queryset=m)
 
         with mock.patch.object(f.form, 'full_clean',
@@ -702,6 +705,14 @@ def test_qs_triggers_form_validation(self):
             f.qs
             fn.assert_called()
 
+    def test_filters_must_return_queryset(self):
+        m = MockQuerySet()
+        f = self.F({'invalid': 'result'}, queryset=m)
+
+        msg = "Expected 'F.invalid' to return a QuerySet, but got a NoneType instead."
+        with self.assertRaisesMessage(AssertionError, msg):
+            f.qs
+
 
 # test filter.method here, as it depends on its parent FilterSet
 class FilterMethodTests(TestCase):
@@ -780,7 +791,7 @@ def test_parent_unresolvable(self):
     def test_method_self_is_parent(self):
         # Ensure the method isn't 're-parented' on the `FilterMethod` helper class.
         # Filter methods should have access to the filterset's properties.
-        request = mock.Mock()
+        request = MockQuerySet()
 
         class F(FilterSet):
             f = CharFilter(method='filter_f')
@@ -792,6 +803,7 @@ class Meta:
             def filter_f(inner_self, qs, name, value):
                 self.assertIsInstance(inner_self, F)
                 self.assertIs(inner_self.request, request)
+                return qs
 
         F({'f': 'foo'}, request=request, queryset=User.objects.all()).qs
 

tests/utils.py

@@ -0,0 +1,15 @@
+from unittest import mock
+
+from django.db import models
+
+
+class MockQuerySet:
+    """
+    Generate a mock that is suitably similar to a QuerySet
+    """
+
+    def __new__(self):
+        m = mock.Mock(spec_set=models.QuerySet())
+        m.filter.return_value = m
+        m.all.return_value = m
+        return m