Browse Source

#21361 Expand unit tests for ObjectChange and testing asserts (#21905)

* #21361 Expand unit tests for ObjectChange and testing asserts

* cleanup

* review feedback

* review feedback

* cleanup

* cleanup

* cleanup

* cleanup
Arthur Hanson 1 month ago
parent
commit
a6cc0b671e
3 changed files with 49 additions and 21 deletions
  1. 12 10
      netbox/utilities/testing/api.py
  2. 25 0
      netbox/utilities/testing/base.py
  3. 12 11
      netbox/utilities/testing/views.py

+ 12 - 10
netbox/utilities/testing/api.py

@@ -253,7 +253,8 @@ class APIViewTestCases:
                     changed_object_id=instance.pk,
                     action=ObjectChangeActionChoices.ACTION_CREATE,
                 )
-                self.assertEqual(objectchange.message, data['changelog_message'])
+                self.assertObjectChange(objectchange, action=ObjectChangeActionChoices.ACTION_CREATE,
+                    message=data['changelog_message'])
 
         def test_bulk_create_objects(self):
             """
@@ -306,7 +307,8 @@ class APIViewTestCases:
                 )
                 self.assertEqual(len(objectchanges), len(self.create_data))
                 for oc in objectchanges:
-                    self.assertEqual(oc.message, changelog_message)
+                    self.assertObjectChange(oc, action=ObjectChangeActionChoices.ACTION_CREATE,
+                        message=changelog_message)
 
     class UpdateObjectViewTestCase(APITestCase):
         update_data = {}
@@ -364,8 +366,8 @@ class APIViewTestCases:
                     changed_object_type=ContentType.objects.get_for_model(instance),
                     changed_object_id=instance.pk
                 )
-                self.assertEqual(objectchange.action, ObjectChangeActionChoices.ACTION_UPDATE)
-                self.assertEqual(objectchange.message, data['changelog_message'])
+                self.assertObjectChange(objectchange, action=ObjectChangeActionChoices.ACTION_UPDATE,
+                    message=data['changelog_message'])
 
         def test_bulk_update_objects(self):
             """
@@ -414,8 +416,8 @@ class APIViewTestCases:
                 )
                 self.assertEqual(len(objectchanges), len(data))
                 for oc in objectchanges:
-                    self.assertEqual(oc.action, ObjectChangeActionChoices.ACTION_UPDATE)
-                    self.assertEqual(oc.message, changelog_message)
+                    self.assertObjectChange(oc, action=ObjectChangeActionChoices.ACTION_UPDATE,
+                        message=changelog_message)
 
     class DeleteObjectViewTestCase(APITestCase):
 
@@ -462,8 +464,8 @@ class APIViewTestCases:
                     changed_object_type=ContentType.objects.get_for_model(instance),
                     changed_object_id=instance.pk
                 )
-                self.assertEqual(objectchange.action, ObjectChangeActionChoices.ACTION_DELETE)
-                self.assertEqual(objectchange.message, data['changelog_message'])
+                self.assertObjectChange(objectchange, action=ObjectChangeActionChoices.ACTION_DELETE,
+                    message=data['changelog_message'])
 
         def test_bulk_delete_objects(self):
             """
@@ -503,8 +505,8 @@ class APIViewTestCases:
                 )
                 self.assertEqual(len(objectchanges), len(data))
                 for oc in objectchanges:
-                    self.assertEqual(oc.action, ObjectChangeActionChoices.ACTION_DELETE)
-                    self.assertEqual(oc.message, changelog_message)
+                    self.assertObjectChange(oc, action=ObjectChangeActionChoices.ACTION_DELETE,
+                        message=changelog_message)
 
     class GraphQLTestCase(APITestCase):
 

+ 25 - 0
netbox/utilities/testing/base.py

@@ -13,6 +13,7 @@ from django.test import TestCase as _TestCase
 from netaddr import IPNetwork
 from taggit.managers import TaggableManager
 
+from core.choices import ObjectChangeActionChoices
 from core.models import ObjectType
 from users.models import ObjectPermission, User
 from utilities.data import ranges_to_string
@@ -83,6 +84,30 @@ class TestCase(_TestCase):
     # Custom assertions
     #
 
+    def assertObjectChange(self, objectchange, *, action, message=None):
+        """
+        Assert that an ObjectChange record has the expected attributes. If message is provided, it will be
+        compared against objectchange.message.
+        """
+        # Verify the change action (create, update, delete)
+        self.assertEqual(objectchange.action, action)
+
+        # Verify the changelog message if provided
+        if message is not None:
+            self.assertEqual(objectchange.message, message)
+
+        # Verify pre/postchange data presence and integrity based on action type
+        if action == ObjectChangeActionChoices.ACTION_CREATE:
+            self.assertIsNone(objectchange.prechange_data, "Expected prechange_data to be None for a create")
+            self.assertIsNotNone(objectchange.postchange_data, "Expected postchange_data to be populated for a create")
+        elif action == ObjectChangeActionChoices.ACTION_UPDATE:
+            self.assertIsNotNone(objectchange.prechange_data, "Expected prechange_data to be populated for an update")
+            self.assertIsNotNone(objectchange.postchange_data, "Expected postchange_data to be populated for an update")
+            self.assertNotEqual(objectchange.prechange_data, objectchange.postchange_data)
+        elif action == ObjectChangeActionChoices.ACTION_DELETE:
+            self.assertIsNotNone(objectchange.prechange_data, "Expected prechange_data to be populated for a delete")
+            self.assertIsNone(objectchange.postchange_data, "Expected postchange_data to be None for a delete")
+
     def assertHttpStatus(self, response, expected_status):
         """
         TestCase method. Provide more detail in the event of an unexpected HTTP response.

+ 12 - 11
netbox/utilities/testing/views.py

@@ -192,8 +192,8 @@ class ViewTestCases:
                     changed_object_id=instance.pk
                 )
                 self.assertEqual(len(objectchanges), 1)
-                self.assertEqual(objectchanges[0].action, ObjectChangeActionChoices.ACTION_CREATE)
-                self.assertEqual(objectchanges[0].message, self.form_data['changelog_message'])
+                self.assertObjectChange(objectchanges[0], action=ObjectChangeActionChoices.ACTION_CREATE,
+                    message=self.form_data['changelog_message'])
 
         @override_settings(EXEMPT_VIEW_PERMISSIONS=['*'], EXEMPT_EXCLUDE_MODELS=[])
         def test_create_object_with_constrained_permission(self):
@@ -299,8 +299,8 @@ class ViewTestCases:
                     changed_object_id=instance.pk
                 )
                 self.assertEqual(len(objectchanges), 1)
-                self.assertEqual(objectchanges[0].action, ObjectChangeActionChoices.ACTION_UPDATE)
-                self.assertEqual(objectchanges[0].message, self.form_data['changelog_message'])
+                self.assertObjectChange(objectchanges[0], action=ObjectChangeActionChoices.ACTION_UPDATE,
+                    message=self.form_data['changelog_message'])
 
         @override_settings(EXEMPT_VIEW_PERMISSIONS=['*'], EXEMPT_EXCLUDE_MODELS=[])
         def test_edit_object_with_constrained_permission(self):
@@ -394,8 +394,8 @@ class ViewTestCases:
                     changed_object_id=instance.pk
                 )
                 self.assertEqual(len(objectchanges), 1)
-                self.assertEqual(objectchanges[0].action, ObjectChangeActionChoices.ACTION_DELETE)
-                self.assertEqual(objectchanges[0].message, form_data['changelog_message'])
+                self.assertObjectChange(objectchanges[0], action=ObjectChangeActionChoices.ACTION_DELETE,
+                    message=form_data['changelog_message'])
 
         @override_settings(EXEMPT_VIEW_PERMISSIONS=['*'])
         def test_delete_object_with_constrained_permission(self):
@@ -716,7 +716,8 @@ class ViewTestCases:
                 self.assertEqual(len(objectchanges), expected_new_objects)
 
                 for oc in objectchanges:
-                    self.assertEqual(oc.message, data['changelog_message'])
+                    self.assertObjectChange(oc, action=ObjectChangeActionChoices.ACTION_CREATE,
+                        message=data['changelog_message'])
 
         @override_settings(EXEMPT_VIEW_PERMISSIONS=['*'])
         def test_bulk_update_objects_with_permission(self):
@@ -868,8 +869,8 @@ class ViewTestCases:
                 )
                 self.assertEqual(len(objectchanges), len(pk_list))
                 for oc in objectchanges:
-                    self.assertEqual(oc.action, ObjectChangeActionChoices.ACTION_UPDATE)
-                    self.assertEqual(oc.message, data['changelog_message'])
+                    self.assertObjectChange(oc, action=ObjectChangeActionChoices.ACTION_UPDATE,
+                        message=data['changelog_message'])
 
         @override_settings(EXEMPT_VIEW_PERMISSIONS=['*'], EXEMPT_EXCLUDE_MODELS=[])
         def test_bulk_edit_objects_with_constrained_permission(self):
@@ -964,8 +965,8 @@ class ViewTestCases:
                 )
                 self.assertEqual(len(objectchanges), len(pk_list))
                 for oc in objectchanges:
-                    self.assertEqual(oc.action, ObjectChangeActionChoices.ACTION_DELETE)
-                    self.assertEqual(oc.message, data['changelog_message'])
+                    self.assertObjectChange(oc, action=ObjectChangeActionChoices.ACTION_DELETE,
+                        message=data['changelog_message'])
 
         def test_bulk_delete_objects_with_constrained_permission(self):
             pk_list = self._get_queryset().values_list('pk', flat=True)