← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~jtv/maas/cobbler-fake-factory into lp:maas

 

Jeroen T. Vermeulen has proposed merging lp:~jtv/maas/cobbler-fake-factory into lp:maas.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #962801 in MAAS: "CobblerObjectTestScenario tests don't belong in test_fakecobbler."
  https://bugs.launchpad.net/maas/+bug/962801

For more details, see:
https://code.launchpad.net/~jtv/maas/cobbler-fake-factory/+merge/102863

This is really just part of an agreed-upon reorg, which consists of a lot of branches.  So I can't say that I've had a pre-imp for this particular branch; really I'm just splitting it off from another branch that has gotten out of hand.

I'm in the process of moving the test cases from test_fakecobbler that really test the cobbler client, rather than the fake cobbler, into test_cobblerclient.  But some helpers that create Cobbler-side objects are used both in tests that will move, and tests that will stay.  So I'm moving them out into a reusable spot.  My follow-up branch then has nothing to do except move the actual test cases over.

Note that the factory is shaped like a mixin for the test case.  Not ideal, but it's a way to keep track of temporary files that it has to create.  Those simply go into fixtures.


Jeroen
-- 
https://code.launchpad.net/~jtv/maas/cobbler-fake-factory/+merge/102863
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/maas/cobbler-fake-factory into lp:maas.
=== modified file 'src/provisioningserver/testing/factory.py'
--- src/provisioningserver/testing/factory.py	2012-04-16 10:00:51 +0000
+++ src/provisioningserver/testing/factory.py	2012-04-20 14:17:23 +0000
@@ -11,14 +11,20 @@
 
 __metaclass__ = type
 __all__ = [
+    'CobblerFakeFactory',
     'ProvisioningFakeFactory',
     ]
 
 from abc import ABCMeta
 from itertools import count
+from random import randint
 from time import time
 from xmlrpclib import Fault
 
+from provisioningserver.cobblerclient import (
+    CobblerDistro,
+    CobblerProfile,
+    )
 from provisioningserver.enum import POWER_TYPE
 from twisted.internet.defer import (
     inlineCallbacks,
@@ -126,3 +132,66 @@
             papi.delete_nodes_by_name,
             node_name)
         returnValue(node_name)
+
+
+class CobblerFakeFactory:
+    """Mixin for test cases: factory of fake objects in Cobbler.
+
+    Warning: there is no cleanup for these yet.  Don't just run this against
+    a real cobbler, or there will be trouble.
+    """
+
+    def default_to_file(self, attributes, attribute, required_attrs):
+        """If `attributes[attribute]` is missing, make it a file.
+
+        Sets the given attribute to a newly-created file, if it is a required
+        attribute and not already set in attributes.
+        """
+        if attribute in required_attrs and attribute not in attributes:
+            attributes[attribute] = self.make_file()
+
+    @inlineCallbacks
+    def default_to_object(self, attributes, attribute, required_attrs,
+                          session, cobbler_class):
+        """If `attributes[attribute]` is missing, make it an object.
+
+        Sets the given attribute to a newly-created Cobbler object, if it is
+        a required attribute and not already set in attributes.
+        """
+        if attribute in required_attrs and attribute not in attributes:
+            other_obj = yield self.fake_cobbler_object(session, cobbler_class)
+            attributes[attribute] = other_obj.name
+
+    @inlineCallbacks
+    def fake_cobbler_object(self, session, object_class, name=None,
+                            attributes=None):
+        """Create a fake Cobbler object.
+
+        :param session: `CobblerSession`.
+        :param object_class: concrete `CobblerObject` class to instantiate.
+        :param name: Option name for the object.
+        :param attributes: Optional dict of attribute values for the object.
+        """
+        if attributes is None:
+            attributes = {}
+        else:
+            attributes = attributes.copy()
+        unique_int = randint(1, 9999)
+        if name is None:
+            name = 'name-%s-%d' % (object_class.object_type, unique_int)
+        attributes['name'] = name
+        self.default_to_file(
+            attributes, 'kernel', object_class.required_attributes),
+        self.default_to_file(
+            attributes, 'initrd', object_class.required_attributes),
+        yield self.default_to_object(
+            attributes, 'profile', object_class.required_attributes, session,
+            CobblerProfile)
+        yield self.default_to_object(
+            attributes, 'distro', object_class.required_attributes, session,
+            CobblerDistro)
+        for attr in object_class.required_attributes:
+            if attr not in attributes:
+                attributes[attr] = '%s-%d' % (attr, unique_int)
+        new_object = yield object_class.new(session, name, attributes)
+        returnValue(new_object)

=== modified file 'src/provisioningserver/tests/test_fakecobbler.py'
--- src/provisioningserver/tests/test_fakecobbler.py	2012-04-19 11:42:03 +0000
+++ src/provisioningserver/tests/test_fakecobbler.py	2012-04-20 14:17:23 +0000
@@ -14,8 +14,8 @@
 
 from itertools import count
 from random import randint
-from tempfile import NamedTemporaryFile
 
+from maastesting.testcase import TestCase
 from provisioningserver.cobblercatcher import ProvisioningError
 from provisioningserver.cobblerclient import (
     CobblerDistro,
@@ -25,16 +25,14 @@
     CobblerRepo,
     CobblerSystem,
     )
+from provisioningserver.testing.factory import CobblerFakeFactory
 from provisioningserver.testing.fakecobbler import (
     FakeCobbler,
     log_in_to_fake_cobbler,
     )
 from testtools.content import text_content
 from testtools.deferredruntest import AsynchronousDeferredRunTest
-from testtools.testcase import (
-    ExpectedException,
-    TestCase,
-    )
+from testtools.testcase import ExpectedException
 from twisted.internet.defer import (
     inlineCallbacks,
     returnValue,
@@ -44,75 +42,7 @@
 unique_ints = count(randint(0, 9999))
 
 
-def make_file():
-    """Make a temporary file."""
-    temp_file = NamedTemporaryFile()
-    temp_file.write("Data here.")
-    temp_file.flush()
-    return temp_file
-
-
-def default_to_file(attributes, attribute, required_attrs):
-    """If `attributes[attribute]` is required but not set, make a file.
-
-    :return: A temporary file.  Keep this alive as long as you need the file.
-    """
-    if attribute in required_attrs and attribute not in attributes:
-        temp_file = make_file()
-        attributes[attribute] = temp_file.name
-        return temp_file
-    else:
-        return None
-
-
-@inlineCallbacks
-def default_to_object(attributes, attribute, required_attrs, session,
-                      cobbler_class):
-    """If `attributes[attribute]` is required but not set, make an object."""
-    if attribute in required_attrs and attribute not in attributes:
-        other_obj = yield fake_cobbler_object(session, cobbler_class)
-        attributes[attribute] = other_obj.name
-
-
-@inlineCallbacks
-def fake_cobbler_object(session, object_class, name=None, attributes=None):
-    """Create a fake Cobbler object.
-
-    :param session: `CobblerSession`.
-    :param object_class: concrete `CobblerObject` class to instantiate.
-    :param name: Option name for the object.
-    :param attributes: Optional dict of attribute values for the object.
-    """
-    if attributes is None:
-        attributes = {}
-    else:
-        attributes = attributes.copy()
-    unique_int = next(unique_ints)
-    if name is None:
-        name = 'name-%s-%d' % (object_class.object_type, unique_int)
-    attributes['name'] = name
-    temp_files = [
-        default_to_file(
-            attributes, 'kernel', object_class.required_attributes),
-        default_to_file(
-            attributes, 'initrd', object_class.required_attributes),
-        ]
-    yield default_to_object(
-        attributes, 'profile', object_class.required_attributes, session,
-        CobblerProfile)
-    yield default_to_object(
-        attributes, 'distro', object_class.required_attributes, session,
-        CobblerDistro)
-    for attr in object_class.required_attributes:
-        if attr not in attributes:
-            attributes[attr] = '%s-%d' % (attr, unique_int)
-    new_object = yield object_class.new(session, name, attributes)
-    # Keep the temporary files alive for the lifetime of the object.
-    new_object._hold_these_files_please = temp_files
-    returnValue(new_object)
-
-
-class TestFakeCobbler(TestCase):
+class TestFakeCobbler(TestCase, CobblerFakeFactory):
     """Test `FakeCobbler`.
 
     These tests should also pass if run against a real (clean) Cobbler.
@@ -139,7 +69,7 @@
         # Use of the token will now fail with an "invalid token"
         # error.  The Cobbler client notices this, re-authenticates, and
         # re-runs the method.
-        yield fake_cobbler_object(session, CobblerRepo)
+        yield self.fake_cobbler_object(session, CobblerRepo)
 
         # The re-authentication results in a fresh token.
         self.assertNotEqual(old_token, session.token)
@@ -148,11 +78,11 @@
     def test_valid_token_does_not_raise_auth_error(self):
         session = yield log_in_to_fake_cobbler()
         old_token = session.token
-        yield fake_cobbler_object(session, CobblerRepo)
+        yield self.fake_cobbler_object(session, CobblerRepo)
         self.assertEqual(old_token, session.token)
 
 
-class CobblerObjectTestScenario:
+class CobblerObjectTestScenario(CobblerFakeFactory):
     """Generic tests for the various `CobblerObject` classes.
 
     This will be run once for each of the classes in the hierarchy.
@@ -189,7 +119,8 @@
     def test_create_object(self):
         session = yield log_in_to_fake_cobbler()
         name = self.make_name()
-        obj = yield fake_cobbler_object(session, self.cobbler_class, name)
+        obj = yield self.fake_cobbler_object(
+            session, self.cobbler_class, name)
         self.assertEqual(name, obj.name)
 
     @inlineCallbacks
@@ -201,7 +132,7 @@
         session = yield log_in_to_fake_cobbler()
         session.fake_proxy.fake_cobbler.xapi_object_edit = return_false
         with ExpectedException(RuntimeError):
-            yield fake_cobbler_object(session, self.cobbler_class)
+            yield self.fake_cobbler_object(session, self.cobbler_class)
 
     @inlineCallbacks
     def test_find_returns_empty_list_if_no_match(self):
@@ -214,7 +145,7 @@
     def test_find_matches_name(self):
         session = yield log_in_to_fake_cobbler()
         name = self.make_name()
-        yield fake_cobbler_object(session, self.cobbler_class, name)
+        yield self.fake_cobbler_object(session, self.cobbler_class, name)
         by_name = yield self.cobbler_class.find(session, name=name)
         self.assertSequenceEqual([name], [obj.name for obj in by_name])
 
@@ -223,7 +154,7 @@
         session = yield log_in_to_fake_cobbler()
         name = self.make_name()
         comment = "This is comment #%d" % next(unique_ints)
-        yield fake_cobbler_object(
+        yield self.fake_cobbler_object(
             session, self.cobbler_class, name, {'comment': comment})
         by_comment = yield self.cobbler_class.find(session, comment=comment)
         self.assertSequenceEqual([name], [obj.name for obj in by_comment])
@@ -232,7 +163,7 @@
     def test_find_without_args_finds_everything(self):
         session = yield log_in_to_fake_cobbler()
         name = self.make_name()
-        yield fake_cobbler_object(session, self.cobbler_class, name)
+        yield self.fake_cobbler_object(session, self.cobbler_class, name)
         found_objects = yield self.cobbler_class.find(session)
         self.assertIn(name, [obj.name for obj in found_objects])
 
@@ -241,7 +172,7 @@
         session = yield log_in_to_fake_cobbler()
         comment = "This is comment #%d" % next(unique_ints)
         name = self.make_name()
-        obj = yield fake_cobbler_object(
+        obj = yield self.fake_cobbler_object(
             session, self.cobbler_class, name, {'comment': comment})
         attributes = yield obj.get_values()
         self.assertEqual(name, attributes['name'])
@@ -252,7 +183,7 @@
         session = yield log_in_to_fake_cobbler()
         comment = "This is comment #%d" % next(unique_ints)
         name = self.make_name()
-        yield fake_cobbler_object(
+        yield self.fake_cobbler_object(
             session, self.cobbler_class, name, {'comment': comment})
         all_objects = yield self.cobbler_class.get_all_values(session)
         found_obj = all_objects[name]
@@ -270,23 +201,24 @@
     def test_get_handle_finds_handle(self):
         session = yield log_in_to_fake_cobbler()
         name = self.make_name()
-        obj = yield fake_cobbler_object(session, self.cobbler_class, name)
+        obj = yield self.fake_cobbler_object(
+            session, self.cobbler_class, name)
         handle = yield obj._get_handle()
         self.assertNotEqual(None, handle)
 
     @inlineCallbacks
     def test_get_handle_distinguishes_objects(self):
         session = yield log_in_to_fake_cobbler()
-        obj1 = yield fake_cobbler_object(session, self.cobbler_class)
+        obj1 = yield self.fake_cobbler_object(session, self.cobbler_class)
         handle1 = yield obj1._get_handle()
-        obj2 = yield fake_cobbler_object(session, self.cobbler_class)
+        obj2 = yield self.fake_cobbler_object(session, self.cobbler_class)
         handle2 = yield obj2._get_handle()
         self.assertNotEqual(handle1, handle2)
 
     @inlineCallbacks
     def test_get_handle_is_consistent(self):
         session = yield log_in_to_fake_cobbler()
-        obj = yield fake_cobbler_object(session, self.cobbler_class)
+        obj = yield self.fake_cobbler_object(session, self.cobbler_class)
         handle1 = yield obj._get_handle()
         handle2 = yield obj._get_handle()
         self.assertEqual(handle1, handle2)
@@ -295,7 +227,8 @@
     def test_delete_removes_object(self):
         session = yield log_in_to_fake_cobbler()
         name = self.make_name()
-        obj = yield fake_cobbler_object(session, self.cobbler_class, name)
+        obj = yield self.fake_cobbler_object(
+            session, self.cobbler_class, name)
         yield obj.delete()
         matches = yield self.cobbler_class.find(session, name=name)
         self.assertSequenceEqual([], matches)
@@ -395,7 +328,7 @@
         names = [self.make_name() for counter in range(num_systems)]
         systems = []
         for name in names:
-            new_system = yield fake_cobbler_object(
+            new_system = yield self.fake_cobbler_object(
                 session, self.cobbler_class, name)
             systems.append(new_system)
         returnValue((names, systems))
@@ -412,7 +345,8 @@
     def test_interface_set_mac_address(self):
         session = yield log_in_to_fake_cobbler()
         name = self.make_name()
-        obj = yield fake_cobbler_object(session, self.cobbler_class, name)
+        obj = yield self.fake_cobbler_object(
+            session, self.cobbler_class, name)
         yield obj.modify(
             {"interface": "eth0", "mac_address": "12:34:56:78:90:12"})
         state = yield obj.get_values()
@@ -425,7 +359,8 @@
     def test_interface_set_dns_name(self):
         session = yield log_in_to_fake_cobbler()
         name = self.make_name()
-        obj = yield fake_cobbler_object(session, self.cobbler_class, name)
+        obj = yield self.fake_cobbler_object(
+            session, self.cobbler_class, name)
         yield obj.modify(
             {"interface": "eth0", "dns_name": "epitaph"})
         state = yield obj.get_values()
@@ -438,7 +373,8 @@
     def test_interface_delete_interface(self):
         session = yield log_in_to_fake_cobbler()
         name = self.make_name()
-        obj = yield fake_cobbler_object(session, self.cobbler_class, name)
+        obj = yield self.fake_cobbler_object(
+            session, self.cobbler_class, name)
         yield obj.modify(
             {"interface": "eth0", "mac_address": "12:34:56:78:90:12"})
         yield obj.modify(