launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #07183
[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(