launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #08583
[Merge] lp:~rvb/maas/celery-rabbit-fixture into lp:maas
Raphaël Badin has proposed merging lp:~rvb/maas/celery-rabbit-fixture into lp:maas.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~rvb/maas/celery-rabbit-fixture/+merge/109118
This branch fixes the fact that we need to have the system's rabbitmq server running to run the tests.
When using CeleryFixture we don't need rabbitmq at all because the test configuration of rabbitmq (cf. /src/maastesting/celery.py) configures celery to run the tasks directly within the caller's thread (as opposed to in a real worker). Since we will be using celery tasks more and more I decided to hook up the CeleryFixture in our Django Test Case. This saves us the trouble of having to configure it manually in each test that uses celery tasks but most importantly it will starting tasks which are not executed (see error 1 and error 2 below). Right now, if one creates a test which executes a task without setting up the Celery fixture, the task is simply not executed.
In case you wonder, there is no performance penalty for doing that (that's normal because there is no worker being started so setting up the Celery fixture is a very lightweight operation).
That change triggered 2 errors which where hidden previously because when the system's rabbitmq was used (i.e. when a task was started without using the Celery fixture), the task's message was simply lost (no worker was listening to that message). Now the task gets executed.
Error 1: maasserver.tests.test_models.NodeTest.test_get_effective_power_parameters_provides_usable_defaults
http://paste.ubuntu.com/1028442/
Looks like the 'virsh_url' parameter was not provided. From what I understand from the test, the goal is to have get_effective_power_parameters provide all the default parameters so that it works out of the box so I added the missing 'virsh_url' parameter.
Error 2: maasserver.tests.test_api.EnlistmentAPITest.test_POST_new_sets_power_type
http://paste.ubuntu.com/1028436/
Here I simply avoided the error by changing s/POWER_TYPE.VIRSH/POWER_TYPE.WAKE_ON_LAN/ in src/maasserver/tests/test_api.py because the problem is linked to the execution of the virsh template script. This avoids the error and is still ok as far as the test is concerned because the test is meant to only test that power_type gets set correctly.
But it's very probable that the error is revealing a problem somewhere so I think I could use the advice from someone who has experience with this part of the code here.
--
https://code.launchpad.net/~rvb/maas/celery-rabbit-fixture/+merge/109118
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/maas/celery-rabbit-fixture into lp:maas.
=== modified file 'src/maasserver/models/__init__.py'
--- src/maasserver/models/__init__.py 2012-06-07 09:30:56 +0000
+++ src/maasserver/models/__init__.py 2012-06-07 11:59:19 +0000
@@ -601,6 +601,7 @@
power_params.setdefault('system_id', self.system_id)
power_params.setdefault('virsh', '/usr/bin/virsh')
+ power_params.setdefault('virsh_url', 'qemu://localhost/')
# The "mac" parameter defaults to the node's primary MAC
# address, but only if no power parameters were set at all.
=== modified file 'src/maasserver/testing/testcase.py'
--- src/maasserver/testing/testcase.py 2012-04-19 15:48:46 +0000
+++ src/maasserver/testing/testcase.py 2012-06-07 11:59:19 +0000
@@ -20,6 +20,7 @@
from django.core.cache import cache
from maasserver.testing import reset_fake_provisioning_api_proxy
from maasserver.testing.factory import factory
+from maastesting.celery import CeleryFixture
import maastesting.djangotestcase
@@ -29,6 +30,7 @@
super(TestCase, self).setUp()
self.addCleanup(cache.clear)
self.addCleanup(reset_fake_provisioning_api_proxy)
+ self.celery = self.useFixture(CeleryFixture())
class TestModelTestCase(TestCase,
=== modified file 'src/maasserver/tests/test_api.py'
--- src/maasserver/tests/test_api.py 2012-05-29 10:46:26 +0000
+++ src/maasserver/tests/test_api.py 2012-06-07 11:59:19 +0000
@@ -257,12 +257,12 @@
self.get_uri('nodes/'), {
'op': 'new',
'architecture': architecture,
- 'power_type': POWER_TYPE.VIRSH,
+ 'power_type': POWER_TYPE.WAKE_ON_LAN,
'mac_addresses': ['00:11:22:33:44:55'],
})
node = Node.objects.get(
system_id=json.loads(response.content)['system_id'])
- self.assertEqual(POWER_TYPE.VIRSH, node.power_type)
+ self.assertEqual(POWER_TYPE.WAKE_ON_LAN, node.power_type)
def test_POST_new_associates_mac_addresses(self):
# The API allows a Node to be created and associated with MAC
=== modified file 'src/maasserver/tests/test_models.py'
--- src/maasserver/tests/test_models.py 2012-06-04 23:20:26 +0000
+++ src/maasserver/tests/test_models.py 2012-06-07 11:59:19 +0000
@@ -49,7 +49,6 @@
)
from maasserver.tests.models import TimestampedModelTestModel
from maasserver.utils import map_enum
-from maastesting.celery import CeleryFixture
from maastesting.djangotestcase import (
TestModelTransactionalTestCase,
TransactionTestCase,
@@ -409,7 +408,6 @@
{status: node.status for status, node in nodes.items()})
def test_start_commissioning_changes_status_and_starts_node(self):
- fixture = self.useFixture(CeleryFixture())
user = factory.make_user()
node = factory.make_node(
status=NODE_STATUS.DECLARED, power_type=POWER_TYPE.WAKE_ON_LAN)
@@ -423,7 +421,7 @@
self.assertAttributes(node, expected_attrs)
self.assertEqual(
(1, 'provisioningserver.tasks.power_on'),
- (len(fixture.tasks), fixture.tasks[0]['task'].name))
+ (len(self.celery.tasks), self.celery.tasks[0]['task'].name))
def test_start_commissioning_sets_user_data(self):
node = factory.make_node(status=NODE_STATUS.DECLARED)
@@ -726,8 +724,6 @@
Node.objects.stop_nodes(ids, stoppable_node.owner))
def test_start_nodes_starts_nodes(self):
- fixture = self.useFixture(CeleryFixture())
-
user = factory.make_user()
node, mac = self.make_node_with_mac(
user, power_type=POWER_TYPE.WAKE_ON_LAN)
@@ -737,16 +733,15 @@
self.assertEqual(
(1, 'provisioningserver.tasks.power_on', mac.mac_address),
(
- len(fixture.tasks),
- fixture.tasks[0]['task'].name,
- fixture.tasks[0]['kwargs']['mac'],
+ len(self.celery.tasks),
+ self.celery.tasks[0]['task'].name,
+ self.celery.tasks[0]['kwargs']['mac'],
))
def test_start_nodes_sets_commissioning_profile(self):
# Starting up a node should always set a profile. Here we test
# that a commissioning profile was set for nodes in the
# commissioning status.
- self.useFixture(CeleryFixture())
user = factory.make_user()
node = factory.make_node(
set_hostname=True, status=NODE_STATUS.COMMISSIONING, owner=user)
@@ -759,7 +754,6 @@
def test_start_nodes_doesnt_set_commissioning_profile(self):
# Starting up a node should always set a profile. Complement the
# above test to show that a different profile can be set.
- self.useFixture(CeleryFixture())
user = factory.make_user()
node = self.make_node(user)
factory.make_mac_address(node=node)
@@ -773,8 +767,6 @@
# If the node has a power_type set to POWER_TYPE.DEFAULT,
# NodeManager.start_node(this_node) should use the default
# power_type.
- fixture = self.useFixture(CeleryFixture())
-
Config.objects.set_config('node_power_type', POWER_TYPE.WAKE_ON_LAN)
user = factory.make_user()
node, unused = self.make_node_with_mac(
@@ -784,13 +776,11 @@
self.assertItemsEqual([node], output)
self.assertEqual(
(1, 'provisioningserver.tasks.power_on'),
- (len(fixture.tasks), fixture.tasks[0]['task'].name))
+ (len(self.celery.tasks), self.celery.tasks[0]['task'].name))
def test_start_nodes_wakeonlan_prefers_power_parameters(self):
# If power_parameters is set we should prefer it to sifting
# through related MAC addresses.
- fixture = self.useFixture(CeleryFixture())
-
user = factory.make_user()
preferred_mac = factory.getRandomMACAddress()
node, mac = self.make_node_with_mac(
@@ -802,32 +792,30 @@
self.assertEqual(
(1, 'provisioningserver.tasks.power_on', preferred_mac),
(
- len(fixture.tasks),
- fixture.tasks[0]['task'].name,
- fixture.tasks[0]['kwargs']['mac'],
+ len(self.celery.tasks),
+ self.celery.tasks[0]['task'].name,
+ self.celery.tasks[0]['kwargs']['mac'],
))
def test_start_nodes_wakeonlan_ignores_invalid_parameters(self):
# If node.power_params is set but doesn't have "mac" in it, then
# the node shouldn't be started.
- fixture = self.useFixture(CeleryFixture())
user = factory.make_user()
node, mac = self.make_node_with_mac(
user, power_type=POWER_TYPE.WAKE_ON_LAN,
power_parameters=dict(jarjar="binks"))
output = Node.objects.start_nodes([node.system_id], user)
self.assertItemsEqual([], output)
- self.assertEqual([], fixture.tasks)
+ self.assertEqual([], self.celery.tasks)
def test_start_nodes_wakeonlan_ignores_empty_mac_parameter(self):
- fixture = self.useFixture(CeleryFixture())
user = factory.make_user()
node, mac = self.make_node_with_mac(
user, power_type=POWER_TYPE.WAKE_ON_LAN,
power_parameters=dict(mac=""))
output = Node.objects.start_nodes([node.system_id], user)
self.assertItemsEqual([], output)
- self.assertEqual([], fixture.tasks)
+ self.assertEqual([], self.celery.tasks)
def test_start_nodes_ignores_nodes_without_mac(self):
user = factory.make_user()
=== modified file 'src/maasserver/tests/test_node_action.py'
--- src/maasserver/tests/test_node_action.py 2012-05-23 16:32:15 +0000
+++ src/maasserver/tests/test_node_action.py 2012-06-07 11:59:19 +0000
@@ -31,7 +31,6 @@
)
from maasserver.testing.factory import factory
from maasserver.testing.testcase import TestCase
-from maastesting.celery import CeleryFixture
from provisioningserver.enum import POWER_TYPE
@@ -188,7 +187,6 @@
urlparse(unicode(e)).path)
def test_AcceptAndCommission_starts_commissioning(self):
- fixture = self.useFixture(CeleryFixture())
node = factory.make_node(
mac=True, status=NODE_STATUS.DECLARED,
power_type=POWER_TYPE.WAKE_ON_LAN)
@@ -197,10 +195,9 @@
self.assertEqual(NODE_STATUS.COMMISSIONING, node.status)
self.assertEqual(
'provisioningserver.tasks.power_on',
- fixture.tasks[0]['task'].name)
+ self.celery.tasks[0]['task'].name)
def test_RetryCommissioning_starts_commissioning(self):
- fixture = self.useFixture(CeleryFixture())
node = factory.make_node(
mac=True, status=NODE_STATUS.FAILED_TESTS,
power_type=POWER_TYPE.WAKE_ON_LAN)
@@ -209,7 +206,7 @@
self.assertEqual(NODE_STATUS.COMMISSIONING, node.status)
self.assertEqual(
'provisioningserver.tasks.power_on',
- fixture.tasks[0]['task'].name)
+ self.celery.tasks[0]['task'].name)
def test_StartNode_inhibit_allows_user_with_SSH_key(self):
user_with_key = factory.make_user()
@@ -225,7 +222,6 @@
self.assertIn("SSH key", inhibition)
def test_StartNode_acquires_and_starts_node(self):
- fixture = self.useFixture(CeleryFixture())
node = factory.make_node(
mac=True, status=NODE_STATUS.READY,
power_type=POWER_TYPE.WAKE_ON_LAN)
@@ -235,4 +231,4 @@
self.assertEqual(user, node.owner)
self.assertEqual(
'provisioningserver.tasks.power_on',
- fixture.tasks[0]['task'].name)
+ self.celery.tasks[0]['task'].name)
