launchpad-reviewers team mailing list archive

["Jeroen T. Vermeulen" <jtv@xxxxxxxxxxxxx>]

Jeroen T. Vermeulen has proposed merging lp:~jtv/maas/reload into lp:maas.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~jtv/maas/reload/+merge/95888

Gah.  This took much too long.  Move the load_object[s] helpers into the testing module.

Why did it take so long?  Because, as it turned out, of a weird Nose bug.  See the comments in the diff.
-- 
https://code.launchpad.net/~jtv/maas/reload/+merge/95888
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/maas/reload into lp:maas.

=== modified file 'src/maasserver/testing/__init__.py'
--- src/maasserver/testing/__init__.py	2012-02-21 11:52:58 +0000
+++ src/maasserver/testing/__init__.py	2012-03-05 12:41:17 +0000
@@ -11,6 +11,8 @@
 __metaclass__ = type
 __all__ = [
     "get_fake_provisioning_api_proxy",
+    "reload_object",
+    "reload_objects",
     "LoggedInTestCase",
     "TestCase",
     ]
@@ -56,3 +58,45 @@
         self.logged_in_user = factory.make_user(password='test')
         self.client.login(
             username=self.logged_in_user.username, password='test')
+
+
+def reload_object(model_object):
+    """Reload `obj` from the database.
+
+    Use this when a test needs to inspect changes to model objects made by
+    the API.
+
+    If the object has been deleted, this will raise the `DoesNotExist`
+    exception for its model class.
+
+    :param model_object: Model object to reload.
+    :type model_object: Concrete `Model` subtype.
+    :return: Freshly-loaded instance of `model_object`.
+    :rtype: Same as `model_object`.
+    """
+    model_class = model_object.__class__
+    try:
+        return model_class.objects.get(id=model_object.id)
+    except model_class.DoesNotExist:
+        return None
+
+
+def reload_objects(model_class, model_objects):
+    """Reload `model_objects` of type `model_class` from the database.
+
+    Use this when a test needs to inspect changes to model objects made by
+    the API.
+
+    If any of the objects have been deleted, they will not be included in
+    the result.
+
+    :param model_class: `Model` class to reload from.
+    :type model_class: Class.
+    :param model_objects: Objects to reload from the database.
+    :type model_objects: Sequence of `model_class` objects.
+    :return: Reloaded objects, in no particular order.
+    :rtype: Sequence of `model_class` objects.
+    """
+    assert all(isinstance(obj, model_class) for obj in model_objects)
+    return model_class.objects.filter(
+        id__in=[obj.id for obj in model_objects])

=== added file 'src/maasserver/testing/models.py'
--- src/maasserver/testing/models.py	1970-01-01 00:00:00 +0000
+++ src/maasserver/testing/models.py	2012-03-05 12:41:17 +0000
@@ -0,0 +1,25 @@
+# Copyright 2012 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Test model for tests of testing module."""
+
+from __future__ import (
+    print_function,
+    unicode_literals,
+    )
+
+__metaclass__ = type
+__all__ = [
+    'TestModel',
+    ]
+
+from django.db.models import (
+    CharField,
+    Model,
+    )
+
+
+class TestModel(Model):
+    """A trivial model class for testing."""
+
+    text = CharField(max_length=100)

=== modified file 'src/maasserver/testing/tests/test_module.py'
--- src/maasserver/testing/tests/test_module.py	2012-02-29 11:40:46 +0000
+++ src/maasserver/testing/tests/test_module.py	2012-03-05 12:41:17 +0000
@@ -12,9 +12,24 @@
 __all__ = []
 
 from maasserver import provisioning
-from maasserver.testing import TestCase
+from maasserver.testing import (
+    reload_object,
+    reload_objects,
+    TestCase,
+    TestModelTestCase,
+    )
+from maasserver.testing.models import TestModel
 from provisioningserver.testing import fakeapi
 
+# Horrible kludge.  Works around a bug where delete() does not work on
+# test models when using nose.  Without this, running the tests in this
+# module fails at the delete() calls, saying a table node_c does not
+# exist.  (Running just the test case passes, but running the entire
+# module's tests fails even if the failing test case is the only one).
+#
+# https://github.com/jbalogh/django-nose/issues/15
+TestModel._meta.get_all_related_objects()
+
 
 class TestTestCase(TestCase):
     """Tests for `TestCase`."""
@@ -50,3 +65,39 @@
         self.assertEqual(expected_profiles, papi_fake.profiles)
         # There are no nodes.
         self.assertEqual({}, papi_fake.nodes)
+
+
+class TestHelpers(TestModelTestCase):
+    """Test helper functions."""
+
+    app = 'maasserver.testing'
+
+    def test_reload_object_reloads_object(self):
+        test_obj = TestModel(text="old text")
+        test_obj.save()
+        TestModel.objects.filter(id=test_obj.id).update(text="new text")
+        self.assertEqual("new text", reload_object(test_obj).text)
+
+    def test_reload_object_returns_None_for_deleted_object(self):
+        test_obj = TestModel()
+        test_obj.save()
+        TestModel.objects.filter(id=test_obj.id).delete()
+        self.assertIsNone(reload_object(test_obj))
+
+    def test_reload_objects_reloads_objects(self):
+        texts = list(map(repr, range(3)))
+        objs = [TestModel(text=text) for text in texts]
+        for obj in objs:
+            obj.save()
+        texts[0] = "different text"
+        TestModel.objects.filter(id=objs[0].id).update(text=texts[0])
+        self.assertItemsEqual(
+            texts, [obj.text for obj in reload_objects(TestModel, objs)])
+
+    def test_reload_objects_omits_deleted_objects(self):
+        objs = [TestModel() for counter in range(3)]
+        for obj in objs:
+            obj.save()
+        dead_obj = objs.pop(0)
+        TestModel.objects.filter(id=dead_obj.id).delete()
+        self.assertItemsEqual(objs, reload_objects(TestModel, objs))

=== modified file 'src/maasserver/tests/test_api.py'
--- src/maasserver/tests/test_api.py	2012-03-05 10:04:43 +0000
+++ src/maasserver/tests/test_api.py	2012-03-05 12:41:17 +0000
@@ -26,6 +26,8 @@
     )
 from maasserver.testing import (
     LoggedInTestCase,
+    reload_object,
+    reload_objects,
     TestCase,
     )
 from maasserver.testing.enum import map_enum
@@ -38,44 +40,6 @@
 from metadataserver.nodeinituser import get_node_init_user
 
 
-def reload_object(model_object):
-    """Reload `obj` from the database.
-
-    Use this when a test needs to inspect changes to model objects made by
-    the API.
-
-    If the object has been deleted, this will raise the `DoesNotExist`
-    exception for its model class.
-
-    :param model_object: Model object to reload.
-    :type model_object: Concrete `Model` subtype.
-    :return: Freshly-loaded instance of `model_object`.
-    :rtype: Same as `model_object`.
-    """
-    return model_object.__class__.objects.get(id=model_object.id)
-
-
-def reload_objects(model_class, model_objects):
-    """Reload `model_objects` of type `model_class` from the database.
-
-    Use this when a test needs to inspect changes to model objects made by
-    the API.
-
-    If any of the objects have been deleted, they will not be included in
-    the result.
-
-    :param model_class: `Model` class to reload from.
-    :type model_class: Class.
-    :param model_objects: Objects to reload from the database.
-    :type model_objects: Sequence of `model_class` objects.
-    :return: Reloaded objects, in no particular order.
-    :rtype: Sequence of `model_class` objects.
-    """
-    assert all(isinstance(obj, model_class) for obj in model_objects)
-    return model_class.objects.filter(
-        id__in=[obj.id for obj in model_objects])
-
-
 class APIv10TestMixin:
 
     def get_uri(self, path):