MAAS

Merge lp:~allenap/maas/tests-clean-reactor into lp:~maas-committers/maas/trunk

  1. tests-clean-reactor
  2. Merge into trunk
Proposed by Gavin Panella
Status: Rejected
Rejected by: MAAS Lander
Proposed branch: lp:~allenap/maas/tests-clean-reactor
Merge into: lp:~maas-committers/maas/trunk
Prerequisite: lp:~blake-rouse/maas/tests-clean-reactor
Diff against target: 266 lines (+84/-50)
4 files modified
src/maasserver/rpc/regionservice.py (+15/-7)
src/maasserver/rpc/tests/test_regionservice.py (+38/-27)
src/maasserver/websockets/tests/test_listener.py (+5/-4)
src/maastesting/testcase.py (+26/-12)
To merge this branch: bzr merge lp:~allenap/maas/tests-clean-reactor
Reviewer Review Type Date Requested Status
MAAS Maintainers Pending
Review via email: mp+264347@code.launchpad.net
To post a comment you must log in.
Revision history for this message
MAAS Lander (maas-lander) wrote :

Transitioned to Git.

lp:maas has now moved from Bzr to Git.
Please propose your branches with Launchpad using Git.

git clone https://git.launchpad.net/maas

Unmerged revisions

3621. By Gavin Panella

Address some more failures.

3620. By Gavin Panella

Don't start the notifier LoopingCall. Instead ensure that it would be started.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/maasserver/rpc/regionservice.py'
2--- src/maasserver/rpc/regionservice.py 2015-06-16 21:10:26 +0000
3+++ src/maasserver/rpc/regionservice.py 2015-07-09 20:34:26 +0000
4@@ -689,6 +689,7 @@
5 lock = threading.Lock()
6
7 starting = None
8+ prepared = False
9 stopping = None
10
11 def __init__(self, interval=60):
12@@ -705,6 +706,7 @@
13 self.starting = self._prepareService()
14
15 def prepared(_):
16+ self.prepared = True
17 return super(RegionAdvertisingService, self).startService()
18 self.starting.addCallback(prepared)
19
20@@ -718,16 +720,22 @@
21 @asynchronous
22 def stopService(self):
23 if self.starting.called:
24- # Start-up is complete; remove all records then up-call in
25- # the usual way.
26- self.stopping = deferToThread(self.remove)
27- self.stopping.addCallback(lambda ignore: (
28- super(RegionAdvertisingService, self).stopService()))
29- return self.stopping
30+ if self.prepared:
31+ # Start-up is complete; remove all records then up-call in
32+ # the usual way.
33+ self.stopping = deferToThread(self.remove)
34+ self.stopping.addCallback(lambda ignore: (
35+ super(RegionAdvertisingService, self).stopService()))
36+ return self.stopping
37+ else:
38+ # It didn't start fully; nothing to do.
39+ self.stopping = defer.succeed(None)
40+ return self.stopping
41 else:
42 # Start-up has not yet finished; cancel it.
43+ self.stopping = self.starting
44 self.starting.cancel()
45- return self.starting
46+ return self.stopping
47
48 @inlineCallbacks
49 def _prepareService(self):
50
51=== modified file 'src/maasserver/rpc/tests/test_regionservice.py'
52--- src/maasserver/rpc/tests/test_regionservice.py 2015-07-09 20:34:25 +0000
53+++ src/maasserver/rpc/tests/test_regionservice.py 2015-07-09 20:34:26 +0000
54@@ -1200,6 +1200,12 @@
55
56 class TestRegionService(MAASTestCase):
57
58+ def setUp(self):
59+ super(TestRegionService, self).setUp()
60+ # Prevent real pauses; pretend they're immediately successful.
61+ pause = self.patch(regionservice, "pause")
62+ pause.side_effect = always_succeed_with(None)
63+
64 def test_init_sets_appropriate_instance_attributes(self):
65 service = RegionService()
66 self.assertThat(service, IsInstance(Service))
67@@ -1296,23 +1302,27 @@
68 ]
69
70 yield service.startService()
71+ self.addCleanup(wait_for_reactor(service.stopService))
72+
73 self.assertThat(
74- logged_failures, MatchesListwise(logged_failures_expected))
75+ logged_failures, MatchesListwise(
76+ logged_failures_expected))
77
78 @wait_for_reactor
79 @inlineCallbacks
80 def test_start_up_binds_first_of_endpoint_options(self):
81 service = RegionService()
82
83- endpoint_1 = Mock()
84- endpoint_1.listen.return_value = succeed(sentinel.port1)
85- endpoint_2 = Mock()
86- endpoint_2.listen.return_value = succeed(sentinel.port2)
87+ endpoint_1, port_1 = Mock(), Mock()
88+ endpoint_1.listen.return_value = succeed(port_1)
89+ endpoint_2, port_2 = Mock(), Mock()
90+ endpoint_2.listen.return_value = succeed(port_2)
91 service.endpoints = [[endpoint_1, endpoint_2]]
92
93 yield service.startService()
94+ self.addCleanup(wait_for_reactor(service.stopService))
95
96- self.assertThat(service.ports, Equals([sentinel.port1]))
97+ self.assertThat(service.ports, Equals([port_1]))
98
99 @wait_for_reactor
100 @inlineCallbacks
101@@ -1454,9 +1464,6 @@
102 def test_stopping_when_start_up_failed(self):
103 service = RegionService()
104
105- # Prevent real pauses.
106- mock_pause = self.patch(regionservice, "pause")
107- mock_pause.return_value = succeed(None)
108 # Ensure that endpoint.listen fails with a obvious error.
109 exception = ValueError("This is a very naughty boy.")
110 endpoints = self.patch(service, "endpoints", [[Mock()]])
111@@ -1681,6 +1688,12 @@
112
113 class TestRegionAdvertisingService(MAASTestCase):
114
115+ def setUp(self):
116+ super(TestRegionAdvertisingService, self).setUp()
117+ # Prevent real pauses; pretend they're immediately successful.
118+ pause = self.patch(regionservice, "pause")
119+ pause.side_effect = always_succeed_with(None)
120+
121 def tearDown(self):
122 super(TestRegionAdvertisingService, self).tearDown()
123 # Django doesn't notice that the database needs to be reset.
124@@ -1759,29 +1772,29 @@
125 return service.starting.addCallback(check)
126
127 @wait_for_reactor
128+ @inlineCallbacks
129 def test_start_up_errors_are_logged(self):
130 service = RegionAdvertisingService()
131- # Prevent real pauses.
132- mock_pause = self.patch(regionservice, "pause")
133- mock_pause.return_value = succeed(None)
134+ # Neuter the usual periodic task.
135+ service.call = (lambda: None), (), {}
136+ # Neuter the clean-up that's called when stopping the service.
137+ service.remove = lambda: None
138 # Ensure that service.prepare fails with a obvious error.
139 exception = ValueError("You don't vote for kings!")
140 self.patch(service, "prepare").side_effect = [exception, None]
141 # Capture all Twisted logs.
142 logger = self.useFixture(TwistedLoggerFixture())
143
144- def check_logs(ignore):
145- self.assertDocTestMatches(
146- """\
147- Preparation of ... failed; will try again in 5 seconds.
148- Traceback (most recent call last):...
149- Failure: exceptions.ValueError: You don't vote for kings!
150- """,
151- logger.dump())
152+ yield service.startService()
153+ self.addCleanup(wait_for_reactor(service.stopService))
154
155- service.startService()
156- service.starting.addCallback(check_logs)
157- return service.starting
158+ self.assertDocTestMatches(
159+ """\
160+ Preparation of ... failed; will try again in 5 seconds.
161+ Traceback (most recent call last):...
162+ Failure: exceptions.ValueError: You don't vote for kings!
163+ """,
164+ logger.dump())
165
166 @wait_for_reactor
167 def test_stopping_cancels_startup(self):
168@@ -1806,12 +1819,10 @@
169 @wait_for_reactor
170 def test_stopping_when_start_up_failed(self):
171 service = RegionAdvertisingService()
172- # Prevent real pauses.
173- mock_pause = self.patch(regionservice, "pause")
174- mock_pause.return_value = succeed(None)
175 # Ensure that service.prepare fails with a obvious error.
176 exception = ValueError("First, shalt thou take out the holy pin.")
177- self.patch(service, "prepare").side_effect = exception
178+ _prepareService = self.patch(service, "_prepareService")
179+ _prepareService.side_effect = always_fail_with(exception)
180 # Suppress logged messages.
181 self.patch(log.theLogPublisher, "observers", [])
182
183
184=== modified file 'src/maasserver/websockets/tests/test_listener.py'
185--- src/maasserver/websockets/tests/test_listener.py 2015-05-27 14:00:48 +0000
186+++ src/maasserver/websockets/tests/test_listener.py 2015-07-09 20:34:26 +0000
187@@ -145,11 +145,12 @@
188
189 self.patch(listener, "startConnection")
190 self.patch(listener, "registerChannels")
191- mock_addReader = self.patch(reactor, "addReader")
192+ mock_addReader = self.patch_autospec(reactor, "addReader")
193+ mock_runHandleNotify = self.patch_autospec(listener, "runHandleNotify")
194 yield listener.tryConnection()
195- self.assertThat(
196- mock_addReader,
197- MockCalledOnceWith(listener))
198+ self.assertThat(mock_addReader, MockCalledOnceWith(listener))
199+ self.assertThat(mock_runHandleNotify, MockCalledOnceWith(
200+ delay=PostgresListener.HANDLE_NOTIFY_DELAY))
201
202 @wait_for_reactor
203 @inlineCallbacks
204
205=== modified file 'src/maastesting/testcase.py'
206--- src/maastesting/testcase.py 2015-07-09 20:34:25 +0000
207+++ src/maastesting/testcase.py 2015-07-09 20:34:26 +0000
208@@ -26,6 +26,7 @@
209 import types
210 import unittest
211
212+from crochet import wait_for_reactor
213 from maastesting.crochet import EventualResultCatchingMixin
214 from maastesting.factory import factory
215 from maastesting.fixtures import TempDirectory
216@@ -44,6 +45,7 @@
217 )
218 from twisted.internet.base import DelayedCall
219 from twisted.internet.task import LoopingCall
220+from twisted.python.threadable import isInIOThread
221 from twisted.trial import util
222
223
224@@ -101,18 +103,30 @@
225 Allows only the crochet delayed call to remain in the reactor. All
226 other delayed calls are an error.
227 """
228- # Flush short-range timers.
229- reactor.iterate(0)
230- reactor.iterate(0)
231-
232- # Get all delayed calls that are active except the call LoopingCall
233- # created by crochet.
234- delayedCallStrings = []
235- for call in reactor.getDelayedCalls():
236- if call.active() and not self._call_is_crochet_eventloop(call):
237- delayedCallStrings.append("%s" % call)
238- call.cancel()
239- return delayedCallStrings
240+ if not reactor.running:
241+ # Nothing to do.
242+ return
243+
244+ def clean():
245+ # Flush short-range timers. FIXME: This seems to cause deadlocks
246+ # when run within a reactor managed by crochet, so it has been
247+ # disabled for now.
248+ # reactor.iterate(0)
249+ # reactor.iterate(0)
250+
251+ # Get all delayed calls that are active except the LoopingCall
252+ # created by crochet.
253+ delayedCallStrings = []
254+ for call in reactor.getDelayedCalls():
255+ if call.active() and not self._call_is_crochet_eventloop(call):
256+ delayedCallStrings.append("%s" % call)
257+ call.cancel()
258+ return delayedCallStrings
259+
260+ if not isInIOThread():
261+ clean = wait_for_reactor(clean)
262+
263+ return clean()
264
265 def _call_is_crochet_eventloop(self, call):
266 """Return True when the `call` is the crochet event loop."""