Merge pull request #80 from clokep/distinct-tests

Improve the tests for distinct.

Author: clokep

tests/models.py

@@ -51,7 +51,7 @@ class BlogPost(models.Model):
 class Book(models.Model):
     title = models.CharField(max_length=50)
     author = models.ForeignKey(Author, on_delete=models.CASCADE)
-    publisher = models.ForeignKey(Publisher, related_name='published', on_delete=models.CASCADE)
+    publishers = models.ManyToManyField(Publisher, related_name='published')
     release = models.DateField()
     pages = models.PositiveSmallIntegerField()
 

tests/test_pagination.py

@@ -46,11 +46,13 @@ def setUp(self):
                                                   address="123 Publishing Street")
 
         for d in range(1, 15):
-            Book.objects.create(title='Book %s' % (d % 2),
-                                author=self.author,
-                                publisher=self.publisher,
-                                pages=d,
-                                release=date(2018, 10, 5))
+            book = Book.objects.create(
+                title='Book %s' % (d % 2),
+                author=self.author,
+                pages=d,
+                release=date(2018, 10, 5),
+            )
+            book.publishers.set([self.publisher])
 
         self.pagination = _TestPagination()
         self.queryset = QuerySetSequence(Book.objects.filter(pages__lte=7),
@@ -225,11 +227,13 @@ def test_duplicates(self):
 
         # Create a bunch of books that are the same.
         for i in range(15):
-            Book.objects.create(title=str(i),
-                                author=self.author,
-                                publisher=self.publisher,
-                                pages=PAGES,
-                                release=date(2018, 10, 5))
+            book = Book.objects.create(
+                title=str(i),
+                author=self.author,
+                pages=PAGES,
+                release=date(2018, 10, 5),
+            )
+            book.publishers.set([self.publisher])
 
         # And use only those duplicate books.
         self.queryset = QuerySetSequence(Book.objects.filter(pages=PAGES))

tests/test_querysetsequence.py

@@ -50,10 +50,12 @@ def setUp(self):
                                publisher=mad_magazine, release=date(1990, 8, 14))
 
         # Bob wrote a couple of books, an article, and a blog post.
-        Book.objects.create(title="Fiction", author=bob, publisher=big_books,
-                            pages=10, release=date(2001, 6, 12))
-        Book.objects.create(title="Biography", author=bob, publisher=big_books,
-                            pages=20, release=date(2002, 12, 24))
+        book = Book.objects.create(title="Fiction", author=bob,
+                                   pages=10, release=date(2001, 6, 12))
+        book.publishers.set([big_books])
+        book = Book.objects.create(title="Biography", author=bob,
+                                   pages=20, release=date(2002, 12, 24))
+        book.publishers.set([big_books])
         Article.objects.create(title="Some Article", author=bob,
                                publisher=mad_magazine, release=date(1979, 1, 1))
         BlogPost.objects.create(title="Post", author=bob,
@@ -387,7 +389,7 @@ def test_empty_defer(self):
     def test_only(self):
         """Only causes other fields to load on access (opposite of defer)."""
         with self.assertNumQueries(2):
-            books = list(self.all.only('publisher'))
+            books = list(self.all.only('author'))
         with self.assertNumQueries(5):
             titles = [b.title for b in books]
         self.assertEqual(titles, self.TITLES_BY_PK)
@@ -399,7 +401,7 @@ def test_order_by(self):
         # Additional queries are due to the field needing to be pulled
         # individually during sorting.
         with self.assertNumQueries(7):
-            books = list(self.all.only('publisher').order_by('title'))
+            books = list(self.all.only('author').order_by('title'))
         # No additional queries are necessary since the titles were used in
         # sorting.
         with self.assertNumQueries(0):
@@ -421,14 +423,34 @@ def test_using(self):
 
 class TestDistinct(TestBase):
     def test_distinct(self):
-        for qs in self.all._querysets:
-            assert not qs.query.distinct
+        """
+        Distinct gets applied to each QuerySet, but not the overall QuerySetSequence.
+        """
+        # Create another publisher and add an existing book to it.
+        publisher = Publisher.objects.create(name="Bigger Books")
+        book = Book.objects.get(title="Biography")
+        book.publishers.add(publisher)
+
+        # Make a QuerySetSequence that would include non-distinct items.
+        big_books = Book.objects.filter(publishers__name__startswith="Big")
+        qss = QuerySetSequence(big_books, Article.objects.filter(publisher__name__startswith="Big"))
+
+        # There should be more items than unique items.
+        titles = [b.title for b in qss]
+        self.assertGreater(len(titles), len(set(titles)))
+
         with self.assertNumQueries(0):
-            distinct = self.all.distinct()
-        for qs in distinct._querysets:
-            assert qs.query.distinct
+            qss = qss.distinct()
+
+        # After calling distinct there should not be duplicated entries.
+        titles = [b.title for b in qss]
+        self.assertEqual(len(titles), len(set(titles)))
 
     def test_multiple_querysets_same_model(self):
+        """
+        Calling distinct on a QuerySetSequence made up of multiple QuerySet instances
+        of the same model is not supported.
+        """
         qss = QuerySetSequence(Book.objects.all(), Book.objects.all())
         with self.assertRaises(NotImplementedError):
             qss.distinct()
@@ -877,8 +899,7 @@ def test_order_by_multi(self):
         """Test ordering by multiple fields."""
         # Add another object with the same title, but a later release date.
         Book.objects.create(title="Fiction", author=self.alice,
-                            publisher=self.big_books, pages=1,
-                            release=date(2018, 10, 3))
+                            pages=1, release=date(2018, 10, 3))
 
         with self.assertNumQueries(0):
             qss = self.all.order_by('title', '-release')
@@ -937,20 +958,22 @@ def test_order_by_relation_pk(self):
         Apply order_by() with a field that returns a model without a default
         ordering (i.e. using the pk).
         """
-        # Order by publisher and ensure it takes.
+        # Order by author and ensure it takes.
         with self.assertNumQueries(0):
-            qss = self.all.order_by('publisher')
+            qss = self.all.order_by('author')
 
         # Ensure that the test has any hope of passing.
-        self.assertLess(self.mad_magazine.pk, self.big_books.pk)
+        self.assertLess(self.alice.pk, self.bob.pk)
 
         # The first three should be from Mad Magazine, followed by three from
         # Big Books.
         # Note that the QuerySetSequence itself needs the publisher objects to
         # compare them, so they all get pulled in.
         with self.assertNumQueries(2 + 5):
-            for expected, element in zip([self.mad_magazine.id] * 3 + [self.big_books.id] * 2, qss):
-                self.assertEqual(element.publisher.id, expected)
+            self.assertEqual(
+                [self.alice.id] * 2 + [self.bob.id] * 3,
+                [b.author.id for b in qss],
+            )
 
     def test_order_by_relation_with_ordering(self):
         """
@@ -997,14 +1020,6 @@ def test_order_by_relation_field(self):
             for expected, element in zip([self.alice.id] * 2 + [self.bob.id] * 3, qss):
                 self.assertEqual(element.author.id, expected)
 
-    def test_order_by_relation_no_existent_field(self):
-        """Apply order_by() with a field through a model relationship that doesn't exist."""
-        with self.assertNumQueries(0):
-            qss = self.all.order_by('publisher__address')
-
-        with self.assertRaises(FieldError):
-            list(qss)
-
     def test_order_by_queryset(self):
         """Ensure we can order by QuerySet and then other fields."""
         # Order by title, but don't interleave each QuerySet.
@@ -1470,10 +1485,12 @@ def test_empty(self):
 class TestDelete(TestBase):
     def test_delete_all(self):
         """Ensure that delete() works properly."""
-        with self.assertNumQueries(2):
+        # 4 queries is due to the models themselves and then the many-to-many
+        # relationships.
+        with self.assertNumQueries(4):
             result = self.all.delete()
-        self.assertEqual(result[0], 5)
-        self.assertEqual(result[1], {'tests.Article': 3, 'tests.Book': 2})
+        self.assertEqual(result[0], 7)
+        self.assertEqual(result[1], {'tests.Article': 3, 'tests.Book': 2, 'tests.Book_publishers': 2})
 
         with self.assertNumQueries(2):
             self.assertEqual(self.all.count(), 0)
@@ -1483,10 +1500,7 @@ def test_delete_filter(self):
         with self.assertNumQueries(2):
             result = self.all.filter(author=self.alice).delete()
         self.assertEqual(result[0], 2)
-        # Django 3.1 no longer returns 0 as the number of objects deleted.
         expected = {'tests.Article': 2}
-        if django.VERSION < (3, 1):
-            expected['tests.Book'] = 0
         self.assertEqual(result[1], expected)
 
         with self.assertNumQueries(2):

tests/test_values.py

@@ -13,7 +13,7 @@ def test_values(self):
         authors = [it['author_id'] for it in values]
         self.assertEqual(titles, self.TITLES_BY_PK)
         self.assertEqual(authors, [2, 2, 1, 1, 2])
-        self.assertCountEqual(values[0].keys(), ['#', 'id', 'author_id', 'pages', 'publisher_id', 'release', 'title'])
+        self.assertCountEqual(values[0].keys(), ['#', 'id', 'author_id', 'pages', 'release', 'title'])
 
     def test_fields(self):
         """Ensure the proper fields are returned."""
@@ -93,7 +93,7 @@ def test_values_list(self):
         """Ensure the values conversion works as expected."""
         with self.assertNumQueries(2):
             values = list(self.all.values_list())
-        self.assertEqual(values[0], (1, 'Fiction', 2, 2, datetime.date(2001, 6, 12), 10))
+        self.assertEqual(values[0], (1, 'Fiction', 2, datetime.date(2001, 6, 12), 10))
 
     def test_fields(self):
         """Ensure the proper fields are returned."""
@@ -240,8 +240,8 @@ def test_values_list(self):
         """Ensure the values conversion works as expected."""
         with self.assertNumQueries(2):
             values = list(self.all.values_list(named=True))
-        self.assertEqual(values[0], (1, 'Fiction', 2, 2, datetime.date(2001, 6, 12), 10))
-        self.assertEqual(values[0]._fields, ('id', 'title', 'author_id', 'publisher_id', 'release', 'pages'))
+        self.assertEqual(values[0], (1, 'Fiction', 2, datetime.date(2001, 6, 12), 10))
+        self.assertEqual(values[0]._fields, ('id', 'title', 'author_id', 'release', 'pages'))
         # Also check one of the other types.
         self.assertEqual(values[2]._fields, ('id', 'title', 'author_id', 'publisher_id', 'release'))