MAAS

Merge lp:~allenap/maas/rack-controller-service--bug-1578800 into lp:~maas-committers/maas/trunk

  1. rack-controller-service--bug-1578800
  2. Merge into trunk
Proposed by Gavin Panella
Status: Merged
Approved by: Gavin Panella
Approved revision: no longer in the source branch.
Merged at revision: 4999
Proposed branch: lp:~allenap/maas/rack-controller-service--bug-1578800
Merge into: lp:~maas-committers/maas/trunk
Diff against target: 233 lines (+51/-37)
2 files modified
src/maasserver/rack_controller.py (+5/-5)
src/maasserver/tests/test_rack_controller.py (+46/-32)
To merge this branch: bzr merge lp:~allenap/maas/rack-controller-service--bug-1578800
Reviewer Review Type Date Requested Status
Mike Pontillo (community) Approve
Review via email: mp+293993@code.launchpad.net

Commit message

Update RackControllerService to use the new RegionAdvertisingService API.

Description of the change

RackControllerService's tests relied a little too heavily on mocks, meaning changes to RegionAdvertisingService's API did not cause test failures. This branch still uses mocks, but I've specified spec objects, and used real DeferredValue and RegionAdvertising instances where possible.

To post a comment you must log in.
Revision history for this message
Mike Pontillo (mpontillo) wrote :

Looks good to me; thanks for the fix.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/maasserver/rack_controller.py'
2--- src/maasserver/rack_controller.py 2016-04-11 16:23:26 +0000
3+++ src/maasserver/rack_controller.py 2016-05-06 10:30:32 +0000
4@@ -91,12 +91,12 @@
5 """Start listening for messages."""
6 super(RackControllerService, self).startService()
7
8- def cb_registerWithPostgres(processId):
9+ def cb_registerWithPostgres(advertising):
10 # Register the coreHandler with postgres.
11- self.processId = processId
12+ self.processId = advertising.process_id
13 self.postgresListener.register(
14- "sys_core_%d" % processId, self.coreHandler)
15- return processId
16+ "sys_core_%d" % self.processId, self.coreHandler)
17+ return self.processId
18
19 @transactional
20 def cb_getManagingProcesses(processId):
21@@ -122,7 +122,7 @@
22 failure.trap(CancelledError)
23 self.starting = None
24
25- self.starting = self.advertisingService.processId.get()
26+ self.starting = self.advertisingService.advertising.get()
27 self.starting.addCallback(cb_registerWithPostgres)
28 self.starting.addCallback(
29 partial(deferToDatabase, cb_getManagingProcesses))
30
31=== modified file 'src/maasserver/tests/test_rack_controller.py'
32--- src/maasserver/tests/test_rack_controller.py 2016-04-11 16:23:26 +0000
33+++ src/maasserver/tests/test_rack_controller.py 2016-05-06 10:30:32 +0000
34@@ -6,10 +6,19 @@
35 __all__ = []
36
37 import random
38+from unittest.mock import (
39+ call,
40+ Mock,
41+ sentinel,
42+)
43
44 from crochet import wait_for
45 from maasserver import rack_controller
46 from maasserver.rack_controller import RackControllerService
47+from maasserver.rpc.regionservice import (
48+ RegionAdvertising,
49+ RegionAdvertisingService,
50+)
51 from maasserver.testing.factory import factory
52 from maasserver.testing.testcase import MAASServerTestCase
53 from maasserver.utils.orm import transactional
54@@ -20,11 +29,7 @@
55 MockCallsMatch,
56 MockNotCalled,
57 )
58-from mock import (
59- call,
60- MagicMock,
61- sentinel,
62-)
63+from provisioningserver.utils.twisted import DeferredValue
64 from testtools import ExpectedException
65 from testtools.matchers import MatchesStructure
66 from twisted.internet import reactor
67@@ -54,21 +59,23 @@
68 advertisingService=sentinel.advertiser))
69
70 def test_startService_sets_starting_to_result_of_processId_get(self):
71- defer = MagicMock()
72- advertiser = MagicMock()
73- advertiser.processId.get.return_value = defer
74+ advertising = Mock(DeferredValue, get=Mock())
75+ advertiser = Mock(RegionAdvertisingService, advertising=advertising)
76 service = RackControllerService(sentinel.listener, advertiser)
77 observed = service.startService()
78- self.assertEqual(defer, observed)
79- self.assertEqual(defer, service.starting)
80+ self.assertEqual(advertising.get.return_value, observed)
81+ self.assertEqual(advertising.get.return_value, service.starting)
82
83 @wait_for_reactor
84 @inlineCallbacks
85 def test_startService_registers_with_postgres_listener(self):
86 regionProcessId = random.randint(0, 100)
87- advertiser = MagicMock()
88- advertiser.processId.get.return_value = succeed(regionProcessId)
89- listener = MagicMock()
90+
91+ advertising = DeferredValue()
92+ advertising.set(RegionAdvertising(sentinel.region_id, regionProcessId))
93+ advertiser = Mock(RegionAdvertisingService, advertising=advertising)
94+
95+ listener = Mock()
96 service = RackControllerService(listener, advertiser)
97 yield service.startService()
98 self.assertThat(
99@@ -82,18 +89,22 @@
100 @inlineCallbacks
101 def test_startService_clears_starting_once_complete(self):
102 regionProcessId = random.randint(0, 100)
103- advertiser = MagicMock()
104- advertiser.processId.get.return_value = succeed(regionProcessId)
105- listener = MagicMock()
106+
107+ advertising = DeferredValue()
108+ advertising.set(RegionAdvertising(sentinel.region_id, regionProcessId))
109+ advertiser = Mock(RegionAdvertisingService, advertising=advertising)
110+
111+ listener = Mock()
112 service = RackControllerService(listener, advertiser)
113 yield service.startService()
114 self.assertIsNone(service.starting)
115
116 @wait_for_reactor
117 def test_startService_handles_cancel(self):
118- advertiser = MagicMock()
119- advertiser.processId.get.return_value = Deferred()
120- listener = MagicMock()
121+ advertising = DeferredValue()
122+ advertiser = Mock(RegionAdvertisingService, advertising=advertising)
123+
124+ listener = Mock()
125 service = RackControllerService(listener, advertiser)
126 starting = service.startService()
127 self.assertIs(starting, service.starting)
128@@ -116,9 +127,12 @@
129
130 process, rack_controllers = yield deferToDatabase(
131 create_process_and_racks)
132- advertiser = MagicMock()
133- advertiser.processId.get.return_value = succeed(process.id)
134- listener = MagicMock()
135+
136+ advertising = DeferredValue()
137+ advertising.set(RegionAdvertising(sentinel.region_id, process.id))
138+ advertiser = Mock(RegionAdvertisingService, advertising=advertising)
139+
140+ listener = Mock()
141 service = RackControllerService(listener, advertiser)
142 mock_coreHandler = self.patch(service, "coreHandler")
143 yield service.startService()
144@@ -132,7 +146,7 @@
145 @wait_for_reactor
146 @inlineCallbacks
147 def test_stopService_handles_canceling_startup(self):
148- listener = MagicMock()
149+ listener = Mock()
150 service = RackControllerService(
151 listener, sentinel.advertiser)
152 service.processId = random.randint(0, 100)
153@@ -148,7 +162,7 @@
154 @inlineCallbacks
155 def test_stopService_calls_unregister_for_the_process(self):
156 processId = random.randint(0, 100)
157- listener = MagicMock()
158+ listener = Mock()
159 service = RackControllerService(
160 listener, sentinel.advertiser)
161 service.processId = processId
162@@ -166,7 +180,7 @@
163 random.randint(0, 100)
164 for _ in range(3)
165 }
166- listener = MagicMock()
167+ listener = Mock()
168 service = RackControllerService(
169 listener, sentinel.advertiser)
170 service.processId = processId
171@@ -183,7 +197,7 @@
172 def test_coreHandler_unwatch_calls_unregister(self):
173 processId = random.randint(0, 100)
174 rack_id = random.randint(0, 100)
175- listener = MagicMock()
176+ listener = Mock()
177 service = RackControllerService(
178 listener, sentinel.advertiser)
179 service.processId = processId
180@@ -199,7 +213,7 @@
181 def test_coreHandler_unwatch_doesnt_call_unregister(self):
182 processId = random.randint(0, 100)
183 rack_id = random.randint(0, 100)
184- listener = MagicMock()
185+ listener = Mock()
186 service = RackControllerService(
187 listener, sentinel.advertiser)
188 service.processId = processId
189@@ -210,7 +224,7 @@
190 def test_coreHandler_watch_calls_register_and_startProcessing(self):
191 processId = random.randint(0, 100)
192 rack_id = random.randint(0, 100)
193- listener = MagicMock()
194+ listener = Mock()
195 service = RackControllerService(
196 listener, sentinel.advertiser)
197 service.processId = processId
198@@ -226,7 +240,7 @@
199 def test_coreHandler_watch_doesnt_call_register(self):
200 processId = random.randint(0, 100)
201 rack_id = random.randint(0, 100)
202- listener = MagicMock()
203+ listener = Mock()
204 service = RackControllerService(
205 listener, sentinel.advertiser)
206 service.processId = processId
207@@ -242,7 +256,7 @@
208 def test_coreHandler_raises_ValueError_for_unknown_action(self):
209 processId = random.randint(0, 100)
210 rack_id = random.randint(0, 100)
211- listener = MagicMock()
212+ listener = Mock()
213 service = RackControllerService(
214 listener, sentinel.advertiser)
215 service.processId = processId
216@@ -252,7 +266,7 @@
217
218 def test_dhcpHandler_adds_to_needsDHCPUpdate(self):
219 rack_id = random.randint(0, 100)
220- listener = MagicMock()
221+ listener = Mock()
222 service = RackControllerService(
223 listener, sentinel.advertiser)
224 service.watching = set([rack_id])
225@@ -263,7 +277,7 @@
226
227 def test_dhcpHandler_doesnt_add_to_needsDHCPUpdate(self):
228 rack_id = random.randint(0, 100)
229- listener = MagicMock()
230+ listener = Mock()
231 service = RackControllerService(
232 listener, sentinel.advertiser)
233 mock_startProcessing = self.patch(service, "startProcessing")