Skip to content

Conversation

WaVEV
Copy link
Collaborator

@WaVEV WaVEV commented Sep 28, 2024

generic_relations_regress.tests.GenericRelationTests.test_annotate assumes an integer pk.

@WaVEV WaVEV requested a review from timgraham September 28, 2024 02:34
@timgraham
Copy link
Collaborator

Or we could edit the test to ensure the functionality works. What do you think?

diff --git a/tests/generic_relations_regress/tests.py b/tests/generic_relations_regress/tests.py
index 20c59f839f..f95d8b39ab 100644
--- a/tests/generic_relations_regress/tests.py
+++ b/tests/generic_relations_regress/tests.py
@@ -251,7 +251,8 @@ class GenericRelationTests(TestCase):
         HasLinkThing.objects.create()
         b = Board.objects.create(name=str(hs1.pk))
         Link.objects.create(content_object=hs2)
-        link = Link.objects.create(content_object=hs1)
+        # An integer PK is required for the Sum() queryset that follows.
+        link = Link.objects.create(content_object=hs1, pk=10)
         Link.objects.create(content_object=b)
         qs = HasLinkThing.objects.annotate(Sum("links")).filter(pk=hs1.pk)
         # If content_type restriction isn't in the query's join condition,
@@ -265,11 +266,11 @@ class GenericRelationTests(TestCase):
         # clear cached results
         qs = qs.all()
         self.assertEqual(qs.count(), 1)
-        # Note - 0 here would be a nicer result...
-        self.assertIs(qs[0].links__sum, None)
+        # Unlike other databases, MongoDB returns 0 instead of null (None).
+        self.assertIs(qs[0].links__sum, 0)
         # Finally test that filtering works.
-        self.assertEqual(qs.filter(links__sum__isnull=True).count(), 1)
-        self.assertEqual(qs.filter(links__sum__isnull=False).count(), 0)
+        self.assertEqual(qs.filter(links__sum__isnull=True).count(), 0)
+        self.assertEqual(qs.filter(links__sum__isnull=False).count(), 1)
 
     def test_filter_targets_related_pk(self):
         # Use hardcoded PKs to ensure different PKs for "link" and "hs2"

@WaVEV
Copy link
Collaborator Author

WaVEV commented Sep 30, 2024

Or we could edit the test to ensure the functionality works. What do you think?

diff --git a/tests/generic_relations_regress/tests.py b/tests/generic_relations_regress/tests.py
index 20c59f839f..f95d8b39ab 100644
--- a/tests/generic_relations_regress/tests.py
+++ b/tests/generic_relations_regress/tests.py
@@ -251,7 +251,8 @@ class GenericRelationTests(TestCase):
         HasLinkThing.objects.create()
         b = Board.objects.create(name=str(hs1.pk))
         Link.objects.create(content_object=hs2)
-        link = Link.objects.create(content_object=hs1)
+        # An integer PK is required for the Sum() queryset that follows.
+        link = Link.objects.create(content_object=hs1, pk=10)
         Link.objects.create(content_object=b)
         qs = HasLinkThing.objects.annotate(Sum("links")).filter(pk=hs1.pk)
         # If content_type restriction isn't in the query's join condition,
@@ -265,11 +266,11 @@ class GenericRelationTests(TestCase):
         # clear cached results
         qs = qs.all()
         self.assertEqual(qs.count(), 1)
-        # Note - 0 here would be a nicer result...
-        self.assertIs(qs[0].links__sum, None)
+        # Unlike other databases, MongoDB returns 0 instead of null (None).
+        self.assertIs(qs[0].links__sum, 0)
         # Finally test that filtering works.
-        self.assertEqual(qs.filter(links__sum__isnull=True).count(), 1)
-        self.assertEqual(qs.filter(links__sum__isnull=False).count(), 0)
+        self.assertEqual(qs.filter(links__sum__isnull=True).count(), 0)
+        self.assertEqual(qs.filter(links__sum__isnull=False).count(), 1)
 
     def test_filter_targets_related_pk(self):
         # Use hardcoded PKs to ensure different PKs for "link" and "hs2"

Ok good. I will create the Django branch with those changes.

@timgraham
Copy link
Collaborator

No problem, I'll just push it directly.

@timgraham timgraham changed the title Reclassify test. remove an expected failure fixed on the Django fork Sep 30, 2024
@timgraham timgraham merged commit a904789 into mongodb:main Sep 30, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants