Merge lp:~allenap/maas/parallel-tests-reliability into lp:~maas-committers/maas/trunk
- parallel-tests-reliability
- Merge into trunk
Status: | Merged |
---|---|
Approved by: | Gavin Panella |
Approved revision: | no longer in the source branch. |
Merged at revision: | 5664 |
Proposed branch: | lp:~allenap/maas/parallel-tests-reliability |
Merge into: | lp:~maas-committers/maas/trunk |
Diff against target: |
694 lines (+157/-94) 21 files modified
Makefile (+2/-7) media/README (+4/-4) required-packages/forbidden (+3/-0) src/maasserver/api/tests/test_discoveries.py (+5/-1) src/maasserver/djangosettings/development.py (+3/-1) src/maasserver/regiondservices/tests/test_ntp.py (+0/-6) src/maasserver/testing/resources.py (+6/-1) src/maasserver/utils/tests/test_dblocks.py (+30/-15) src/maasserver/utils/tests/test_dbtasks.py (+1/-1) src/maasserver/websockets/handlers/tests/test_subnet.py (+11/-7) src/maastesting/fixtures.py (+21/-7) src/maastesting/testcase.py (+8/-1) src/maastesting/tests/test_conflict_markers.py (+20/-17) src/maastesting/tests/test_fixtures.py (+11/-0) src/metadataserver/models/scriptset.py (+4/-0) src/provisioningserver/dhcp/__init__.py (+5/-5) src/provisioningserver/rpc/tests/test_clusterservice.py (+2/-0) src/provisioningserver/tests/test_config.py (+4/-2) src/provisioningserver/utils/fs.py (+5/-4) src/provisioningserver/utils/tests/test_fs.py (+4/-3) src/provisioningserver/utils/tests/test_utils.py (+8/-12) |
To merge this branch: | bzr merge lp:~allenap/maas/parallel-tests-reliability |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Blake Rouse (community) | Approve | ||
Review via email: mp+315553@code.launchpad.net |
Commit message
Make bin/test.parallel reliable.
Description of the change
I have run bin/test.parallel 10 times with only one failure. This turned out to be a broken test; see the fix in https:/
I haven't swapped bin/test.parallel into the `make test` slot because it does not yet produce XUnit/JUnit output which Jenkins consumes. I also would like to get feedback from other team members to see if it is working reliably for them in different environments to my own.
Gavin Panella (allenap) wrote : | # |
Thanks. I fixed a couple of problems you encountered when running bin/test.parallel.
I'm not sure how to fix the "No address found for host localhost" problem you hit (http://
I would like to get test.js working again, locally, but there's no obvious reason why it breaks.
Blake Rouse (blake-rouse) wrote : | # |
Well test.js works locally for me. It also must work on the lander as its a requirement for branches to land. As long as this branch doesn't remove that requirement I am good with landing it, once that test failure I am getting is fixed.
Gavin Panella (allenap) wrote : | # |
Re-enabled test.js, and test failure fixed. Thanks!
Preview Diff
1 | === modified file 'Makefile' | |||
2 | --- Makefile 2017-01-20 11:30:38 +0000 | |||
3 | +++ Makefile 2017-01-25 16:51:52 +0000 | |||
4 | @@ -90,10 +90,6 @@ | |||
5 | 90 | echo '/src/**/*.pyc' >> $@ | 90 | echo '/src/**/*.pyc' >> $@ |
6 | 91 | echo '/etc/**/*.pyc' >> $@ | 91 | echo '/etc/**/*.pyc' >> $@ |
7 | 92 | 92 | ||
8 | 93 | run/etc/ntp.conf: templates/ntp.conf | ||
9 | 94 | @mkdir -p $(@D) | ||
10 | 95 | @cp templates/ntp.conf $@ | ||
11 | 96 | |||
12 | 97 | configure-buildout: | 93 | configure-buildout: |
13 | 98 | utilities/configure-buildout | 94 | utilities/configure-buildout |
14 | 99 | 95 | ||
15 | @@ -163,8 +159,7 @@ | |||
16 | 163 | @touch --no-create $@ | 159 | @touch --no-create $@ |
17 | 164 | 160 | ||
18 | 165 | bin/test.rack: \ | 161 | bin/test.rack: \ |
21 | 166 | bin/buildout buildout.cfg versions.cfg setup.py bin/maas-rack \ | 162 | bin/buildout buildout.cfg versions.cfg setup.py bin/maas-rack |
20 | 167 | run/etc/ntp.conf | ||
22 | 168 | $(buildout) install rack-test | 163 | $(buildout) install rack-test |
23 | 169 | @touch --no-create $@ | 164 | @touch --no-create $@ |
24 | 170 | 165 | ||
25 | @@ -397,7 +392,7 @@ | |||
26 | 397 | find . -type d -name '__pycache__' -print0 | xargs -r0 $(RM) -r | 392 | find . -type d -name '__pycache__' -print0 | xargs -r0 $(RM) -r |
27 | 398 | find . -type f -name '*~' -print0 | xargs -r0 $(RM) | 393 | find . -type f -name '*~' -print0 | xargs -r0 $(RM) |
28 | 399 | find . -type f -name dropin.cache -print0 | xargs -r0 $(RM) | 394 | find . -type f -name dropin.cache -print0 | xargs -r0 $(RM) |
30 | 400 | $(RM) -r media/demo/* media/development | 395 | $(RM) -r media/demo/* media/development media/development.* |
31 | 401 | $(RM) $(js_enums) $(js_enums).tmp | 396 | $(RM) $(js_enums) $(js_enums).tmp |
32 | 402 | $(RM) src/maasserver/data/templates.py | 397 | $(RM) src/maasserver/data/templates.py |
33 | 403 | $(RM) *.log | 398 | $(RM) *.log |
34 | 404 | 399 | ||
35 | === modified file 'media/README' | |||
36 | --- media/README 2016-12-07 12:46:14 +0000 | |||
37 | +++ media/README 2017-01-25 16:51:52 +0000 | |||
38 | @@ -1,5 +1,5 @@ | |||
39 | 1 | This folder contains somewhat ephemeral things: subfolders serve as | 1 | This folder contains somewhat ephemeral things: subfolders serve as |
44 | 2 | MEDIA_ROOT for maasserver.djangosettings.demo and .development | 2 | MEDIA_ROOT for `demo` and `development` environments. The media/demo |
45 | 3 | environments. The media/demo directory should always exist and not be | 3 | directory should always exist and not be deleted, though its contents |
46 | 4 | deleted, though its contents can be. The media/development directory | 4 | can be. The media/development.$$ (where $$ is the PID of the running |
47 | 5 | should be created and destroyed by tests, as needed. | 5 | process) directories are created and destroyed by tests, as needed. |
48 | 6 | 6 | ||
49 | === modified file 'required-packages/forbidden' | |||
50 | --- required-packages/forbidden 2014-09-24 13:05:22 +0000 | |||
51 | +++ required-packages/forbidden 2017-01-25 16:51:52 +0000 | |||
52 | @@ -0,0 +1,3 @@ | |||
53 | 1 | python3-testresources | ||
54 | 2 | python3-testscenarios | ||
55 | 3 | python3-testtools | ||
56 | 0 | 4 | ||
57 | === renamed file 'templates/ntp.conf' => 'run/etc/ntp.conf' | |||
58 | === modified file 'src/maasserver/api/tests/test_discoveries.py' | |||
59 | --- src/maasserver/api/tests/test_discoveries.py 2016-12-12 14:17:23 +0000 | |||
60 | +++ src/maasserver/api/tests/test_discoveries.py 2017-01-25 16:51:52 +0000 | |||
61 | @@ -9,6 +9,7 @@ | |||
62 | 9 | import http.client | 9 | import http.client |
63 | 10 | import json | 10 | import json |
64 | 11 | import random | 11 | import random |
65 | 12 | from unittest.mock import ANY | ||
66 | 12 | 13 | ||
67 | 13 | from django.conf import settings | 14 | from django.conf import settings |
68 | 14 | from django.core.urlresolvers import reverse | 15 | from django.core.urlresolvers import reverse |
69 | @@ -519,8 +520,11 @@ | |||
70 | 519 | scan_all_rack_networks(cidrs=cidrs) | 520 | scan_all_rack_networks(cidrs=cidrs) |
71 | 520 | self.assertThat( | 521 | self.assertThat( |
72 | 521 | self.call_racks_sync_mock, MockCalledOnceWith( | 522 | self.call_racks_sync_mock, MockCalledOnceWith( |
74 | 522 | cluster.ScanNetworks, controllers=self.started, | 523 | cluster.ScanNetworks, controllers=ANY, |
75 | 523 | kwargs={'cidrs': cidrs})) | 524 | kwargs={'cidrs': cidrs})) |
76 | 525 | # Check `controllers` separately because its order may vary. | ||
77 | 526 | controllers = self.call_racks_sync_mock.call_args[1]["controllers"] | ||
78 | 527 | self.assertItemsEqual(self.started, controllers) | ||
79 | 524 | 528 | ||
80 | 525 | def test__calls_racks_synchronously_with_force_ping(self): | 529 | def test__calls_racks_synchronously_with_force_ping(self): |
81 | 526 | scan_all_rack_networks(ping=True) | 530 | scan_all_rack_networks(ping=True) |
82 | 527 | 531 | ||
83 | === modified file 'src/maasserver/djangosettings/development.py' | |||
84 | --- src/maasserver/djangosettings/development.py 2017-01-20 08:22:34 +0000 | |||
85 | +++ src/maasserver/djangosettings/development.py 2017-01-25 16:51:52 +0000 | |||
86 | @@ -63,7 +63,9 @@ | |||
87 | 63 | 63 | ||
88 | 64 | # Absolute filesystem path to the directory that will hold user-uploaded files. | 64 | # Absolute filesystem path to the directory that will hold user-uploaded files. |
89 | 65 | # Example: "/home/media/media.lawrence.com/media/" | 65 | # Example: "/home/media/media.lawrence.com/media/" |
91 | 66 | MEDIA_ROOT = abspath("media/development") | 66 | # To allow tests to run in parallel the current process's PID is part of this |
92 | 67 | # path. See MediaRootFixture for details. | ||
93 | 68 | MEDIA_ROOT = abspath("media/development.%d" % os.getpid()) | ||
94 | 67 | 69 | ||
95 | 68 | INTERNAL_IPS = ('127.0.0.1', '::1') | 70 | INTERNAL_IPS = ('127.0.0.1', '::1') |
96 | 69 | 71 | ||
97 | 70 | 72 | ||
98 | === modified file 'src/maasserver/regiondservices/tests/test_ntp.py' | |||
99 | --- src/maasserver/regiondservices/tests/test_ntp.py 2016-12-12 14:17:23 +0000 | |||
100 | +++ src/maasserver/regiondservices/tests/test_ntp.py 2017-01-25 16:51:52 +0000 | |||
101 | @@ -5,8 +5,6 @@ | |||
102 | 5 | 5 | ||
103 | 6 | __all__ = [] | 6 | __all__ = [] |
104 | 7 | 7 | ||
105 | 8 | from os.path import join | ||
106 | 9 | |||
107 | 10 | from crochet import wait_for | 8 | from crochet import wait_for |
108 | 11 | from maasserver.models.config import Config | 9 | from maasserver.models.config import Config |
109 | 12 | from maasserver.regiondservices import ntp | 10 | from maasserver.regiondservices import ntp |
110 | @@ -123,10 +121,6 @@ | |||
111 | 123 | # Ensure that we never actually execute against systemd. | 121 | # Ensure that we never actually execute against systemd. |
112 | 124 | self.patch_autospec(service_monitor, "restartService") | 122 | self.patch_autospec(service_monitor, "restartService") |
113 | 125 | 123 | ||
114 | 126 | maasroot = self.useFixture(MAASRootFixture()).path | ||
115 | 127 | with open(join(maasroot, "etc", "ntp.conf"), "w") as ntp_conf: | ||
116 | 128 | ntp_conf.write("# Placeholder NTP configuration file.\n") | ||
117 | 129 | |||
118 | 130 | with TwistedLoggerFixture() as logger: | 124 | with TwistedLoggerFixture() as logger: |
119 | 131 | yield service._tryUpdate() | 125 | yield service._tryUpdate() |
120 | 132 | 126 | ||
121 | 133 | 127 | ||
122 | === modified file 'src/maasserver/testing/resources.py' | |||
123 | --- src/maasserver/testing/resources.py 2017-01-20 08:21:22 +0000 | |||
124 | +++ src/maasserver/testing/resources.py 2017-01-25 16:51:52 +0000 | |||
125 | @@ -217,6 +217,7 @@ | |||
126 | 217 | 217 | ||
127 | 218 | def make(self, dependencies): | 218 | def make(self, dependencies): |
128 | 219 | databases = dependencies["templates"] | 219 | databases = dependencies["templates"] |
129 | 220 | clusterlock = databases.cluster.lock | ||
130 | 220 | with databases.cluster.connect() as conn: | 221 | with databases.cluster.connect() as conn: |
131 | 221 | with conn.cursor() as cursor: | 222 | with conn.cursor() as cursor: |
132 | 222 | for database in databases: | 223 | for database in databases: |
133 | @@ -226,7 +227,11 @@ | |||
134 | 226 | stmt = ( | 227 | stmt = ( |
135 | 227 | "CREATE DATABASE %s WITH TEMPLATE %s" | 228 | "CREATE DATABASE %s WITH TEMPLATE %s" |
136 | 228 | % (dbname, template)) | 229 | % (dbname, template)) |
138 | 229 | cursor.execute(stmt) | 230 | # Create the database with a shared lock to the cluster to |
139 | 231 | # avoid racing a DjangoPristineDatabaseManager.make in a | ||
140 | 232 | # concurrently running test process. | ||
141 | 233 | with clusterlock.shared: | ||
142 | 234 | cursor.execute(stmt) | ||
143 | 230 | debug( | 235 | debug( |
144 | 231 | "Created {dbname}; statement: {stmt}", | 236 | "Created {dbname}; statement: {stmt}", |
145 | 232 | dbname=dbname, stmt=stmt) | 237 | dbname=dbname, stmt=stmt) |
146 | 233 | 238 | ||
147 | === modified file 'src/maasserver/utils/tests/test_dblocks.py' | |||
148 | --- src/maasserver/utils/tests/test_dblocks.py 2015-12-01 18:12:59 +0000 | |||
149 | +++ src/maasserver/utils/tests/test_dblocks.py 2017-01-25 16:51:52 +0000 | |||
150 | @@ -9,7 +9,7 @@ | |||
151 | 9 | closing, | 9 | closing, |
152 | 10 | contextmanager, | 10 | contextmanager, |
153 | 11 | ) | 11 | ) |
155 | 12 | from random import randint | 12 | from random import randrange |
156 | 13 | import sys | 13 | import sys |
157 | 14 | 14 | ||
158 | 15 | from django.db import ( | 15 | from django.db import ( |
159 | @@ -25,18 +25,24 @@ | |||
160 | 25 | from maasserver.utils import dblocks | 25 | from maasserver.utils import dblocks |
161 | 26 | from testtools.matchers import Equals | 26 | from testtools.matchers import Equals |
162 | 27 | 27 | ||
163 | 28 | # Use "high" objid numbers to avoid conflicts with predeclared locks. | ||
164 | 29 | objid_min = 2 << 10 | ||
165 | 30 | objid_max = 2 << 16 | ||
166 | 31 | |||
167 | 28 | 32 | ||
168 | 29 | def get_locks(): | 33 | def get_locks(): |
171 | 30 | """Return the set of locks held.""" | 34 | """Return the set of locks held between `objid_min` and `objid_max`.""" |
170 | 31 | stmt = "SELECT objid FROM pg_locks WHERE classid = %s" | ||
172 | 32 | with closing(connection.cursor()) as cursor: | 35 | with closing(connection.cursor()) as cursor: |
174 | 33 | cursor.execute(stmt, [dblocks.classid]) | 36 | cursor.execute( |
175 | 37 | "SELECT objid FROM pg_locks " | ||
176 | 38 | "WHERE classid = %s AND objid >= %s AND objid < %s", | ||
177 | 39 | [dblocks.classid, objid_min, objid_max]) | ||
178 | 34 | return {result[0] for result in cursor.fetchall()} | 40 | return {result[0] for result in cursor.fetchall()} |
179 | 35 | 41 | ||
180 | 36 | 42 | ||
181 | 37 | def random_objid(): | 43 | def random_objid(): |
182 | 38 | """Return a 'high' objid that's won't coincide with predeclared locks.""" | 44 | """Return a 'high' objid that's won't coincide with predeclared locks.""" |
184 | 39 | return randint(2 << 10, 2 << 16) | 45 | return randrange(objid_min, objid_max) |
185 | 40 | 46 | ||
186 | 41 | 47 | ||
187 | 42 | @transaction.atomic | 48 | @transaction.atomic |
188 | @@ -99,10 +105,15 @@ | |||
189 | 99 | objid = random_objid() | 105 | objid = random_objid() |
190 | 100 | lock = self.make_lock(objid) | 106 | lock = self.make_lock(objid) |
191 | 101 | 107 | ||
196 | 102 | locks_held_before = get_locks() | 108 | # Take an exclusive lock on the database cluster to prevent this |
197 | 103 | with lock: | 109 | # section from running concurrently with any other thusly delimited |
198 | 104 | locks_held = get_locks() | 110 | # section. Note that `self.databases` is the test resource for the |
199 | 105 | locks_held_after = get_locks() | 111 | # database(s), which then has a reference to the cluster resource. |
200 | 112 | with self.databases.cluster.lock.exclusive: | ||
201 | 113 | locks_held_before = get_locks() | ||
202 | 114 | with lock: | ||
203 | 115 | locks_held = get_locks() | ||
204 | 116 | locks_held_after = get_locks() | ||
205 | 106 | 117 | ||
206 | 107 | locks_obtained = locks_held - locks_held_before | 118 | locks_obtained = locks_held - locks_held_before |
207 | 108 | self.assertEqual({objid}, locks_obtained) | 119 | self.assertEqual({objid}, locks_obtained) |
208 | @@ -266,12 +277,16 @@ | |||
209 | 266 | objid = random_objid() | 277 | objid = random_objid() |
210 | 267 | lock = self.make_lock(objid) | 278 | lock = self.make_lock(objid) |
211 | 268 | 279 | ||
218 | 269 | with transaction.atomic(): | 280 | # Take an exclusive lock on the database cluster to prevent this |
219 | 270 | locks_held_before = get_locks() | 281 | # section from running concurrently with any other thusly delimited |
220 | 271 | with lock: | 282 | # section. |
221 | 272 | locks_held = get_locks() | 283 | with self.databases.cluster.lock.exclusive: |
222 | 273 | locks_held_after = get_locks() | 284 | with transaction.atomic(): |
223 | 274 | locks_held_after_txn = get_locks() | 285 | locks_held_before = get_locks() |
224 | 286 | with lock: | ||
225 | 287 | locks_held = get_locks() | ||
226 | 288 | locks_held_after = get_locks() | ||
227 | 289 | locks_held_after_txn = get_locks() | ||
228 | 275 | 290 | ||
229 | 276 | locks_obtained = locks_held - locks_held_before | 291 | locks_obtained = locks_held - locks_held_before |
230 | 277 | self.assertEqual({objid}, locks_obtained) | 292 | self.assertEqual({objid}, locks_obtained) |
231 | 278 | 293 | ||
232 | === modified file 'src/maasserver/utils/tests/test_dbtasks.py' | |||
233 | --- src/maasserver/utils/tests/test_dbtasks.py 2016-06-22 17:03:02 +0000 | |||
234 | +++ src/maasserver/utils/tests/test_dbtasks.py 2017-01-25 16:51:52 +0000 | |||
235 | @@ -291,7 +291,7 @@ | |||
236 | 291 | 291 | ||
237 | 292 | self.assertDocTestMatches( | 292 | self.assertDocTestMatches( |
238 | 293 | """\ | 293 | """\ |
240 | 294 | Unhandled failure in database task. | 294 | ...Unhandled failure in database task. |
241 | 295 | Traceback (most recent call last): | 295 | Traceback (most recent call last): |
242 | 296 | ... | 296 | ... |
243 | 297 | builtins.ZeroDivisionError: ... | 297 | builtins.ZeroDivisionError: ... |
244 | 298 | 298 | ||
245 | === modified file 'src/maasserver/websockets/handlers/tests/test_subnet.py' | |||
246 | --- src/maasserver/websockets/handlers/tests/test_subnet.py 2016-12-20 06:52:13 +0000 | |||
247 | +++ src/maasserver/websockets/handlers/tests/test_subnet.py 2017-01-25 16:51:52 +0000 | |||
248 | @@ -5,6 +5,7 @@ | |||
249 | 5 | 5 | ||
250 | 6 | __all__ = [] | 6 | __all__ = [] |
251 | 7 | 7 | ||
252 | 8 | import re | ||
253 | 8 | from unittest.mock import sentinel | 9 | from unittest.mock import sentinel |
254 | 9 | 10 | ||
255 | 10 | from fixtures import FakeLogger | 11 | from fixtures import FakeLogger |
256 | @@ -15,14 +16,14 @@ | |||
257 | 15 | from maasserver.utils.orm import reload_object | 16 | from maasserver.utils.orm import reload_object |
258 | 16 | from maasserver.websockets.base import dehydrate_datetime | 17 | from maasserver.websockets.base import dehydrate_datetime |
259 | 17 | from maasserver.websockets.handlers.subnet import SubnetHandler | 18 | from maasserver.websockets.handlers.subnet import SubnetHandler |
264 | 18 | from maastesting.matchers import ( | 19 | from maastesting.matchers import MockCalledOnceWith |
261 | 19 | DocTestMatches, | ||
262 | 20 | MockCalledOnceWith, | ||
263 | 21 | ) | ||
265 | 22 | from netaddr import IPNetwork | 20 | from netaddr import IPNetwork |
266 | 23 | from provisioningserver.utils.network import IPRangeStatistics | 21 | from provisioningserver.utils.network import IPRangeStatistics |
267 | 24 | from testtools import ExpectedException | 22 | from testtools import ExpectedException |
269 | 25 | from testtools.matchers import Equals | 23 | from testtools.matchers import ( |
270 | 24 | Equals, | ||
271 | 25 | MatchesRegex, | ||
272 | 26 | ) | ||
273 | 26 | 27 | ||
274 | 27 | 28 | ||
275 | 28 | class TestSubnetHandler(MAASServerTestCase): | 29 | class TestSubnetHandler(MAASServerTestCase): |
276 | @@ -160,8 +161,11 @@ | |||
277 | 160 | handler.scan({ | 161 | handler.scan({ |
278 | 161 | "id": subnet.id, | 162 | "id": subnet.id, |
279 | 162 | }) | 163 | }) |
282 | 163 | self.assertThat(logger.output, DocTestMatches( | 164 | # Use MatchesRegex here rather than DocTestMatches because usernames |
283 | 164 | "User...%s...scan...%s" % (user.username, cidr))) | 165 | # can contain characters that confuse DocTestMatches (e.g. periods). |
284 | 166 | self.assertThat(logger.output, MatchesRegex( | ||
285 | 167 | "User '%s' initiated a neighbour discovery scan against subnet: %s" | ||
286 | 168 | % (re.escape(user.username), re.escape(str(cidr))))) | ||
287 | 165 | 169 | ||
288 | 166 | def test__scan_ipv6_fails(self): | 170 | def test__scan_ipv6_fails(self): |
289 | 167 | user = factory.make_admin() | 171 | user = factory.make_admin() |
290 | 168 | 172 | ||
291 | === modified file 'src/maastesting/fixtures.py' | |||
292 | --- src/maastesting/fixtures.py 2016-08-09 00:14:06 +0000 | |||
293 | +++ src/maastesting/fixtures.py 2017-01-25 16:51:52 +0000 | |||
294 | @@ -22,10 +22,9 @@ | |||
295 | 22 | ) | 22 | ) |
296 | 23 | import logging | 23 | import logging |
297 | 24 | import os | 24 | import os |
298 | 25 | from pathlib import Path | ||
299 | 25 | from subprocess import ( | 26 | from subprocess import ( |
300 | 26 | CalledProcessError, | 27 | CalledProcessError, |
301 | 27 | check_output, | ||
302 | 28 | DEVNULL, | ||
303 | 29 | PIPE, | 28 | PIPE, |
304 | 30 | Popen, | 29 | Popen, |
305 | 31 | ) | 30 | ) |
306 | @@ -404,7 +403,7 @@ | |||
307 | 404 | 403 | ||
308 | 405 | 404 | ||
309 | 406 | class MAASRootFixture(fixtures.Fixture): | 405 | class MAASRootFixture(fixtures.Fixture): |
311 | 407 | """Copy an existing `MAAS_ROOT` to a new temporary location. | 406 | """Create a new, pristine, `MAAS_ROOT` in a temporary location. |
312 | 408 | 407 | ||
313 | 409 | Also updates `MAAS_ROOT` in the environment to point to this new location. | 408 | Also updates `MAAS_ROOT` in the environment to point to this new location. |
314 | 410 | """ | 409 | """ |
315 | @@ -416,10 +415,25 @@ | |||
316 | 416 | raise NotADirectoryError("MAAS_ROOT is not defined.") | 415 | raise NotADirectoryError("MAAS_ROOT is not defined.") |
317 | 417 | elif os.path.isdir(maasroot): | 416 | elif os.path.isdir(maasroot): |
318 | 418 | self.path = self.useFixture(TempDirectory()).join("run") | 417 | self.path = self.useFixture(TempDirectory()).join("run") |
323 | 419 | # Neither shutil.copytree nor /bin/cp deal with relative symlinks, | 418 | # Work only in $MAAS_ROOT/run; reference the old $MAAS_ROOT. |
324 | 420 | # but /bin/cp is much faster at copying dereferenced files. | 419 | etc = Path(self.path).joinpath("etc") |
325 | 421 | command = "/bin/cp", "-r", "--dereference", maasroot, self.path | 420 | src = Path(maasroot) |
326 | 422 | check_output(command, stdin=DEVNULL, stderr=PIPE) | 421 | # Create and populate $MAAS_ROOT/run/etc/{ntp,ntp.conf}. The |
327 | 422 | # `.keep` file is not strictly necessary, but it's created for | ||
328 | 423 | # consistency with the source tree's `run` directory. | ||
329 | 424 | ntp = etc.joinpath("ntp") | ||
330 | 425 | ntp.mkdir(parents=True) | ||
331 | 426 | ntp.joinpath(".keep").touch() | ||
332 | 427 | ntp_conf = etc.joinpath("ntp.conf") | ||
333 | 428 | ntp_conf.write_bytes(src.joinpath("etc", "ntp.conf").read_bytes()) | ||
334 | 429 | # Create and populate $MAAS_ROOT/run/etc/maas. | ||
335 | 430 | maas = etc.joinpath("maas") | ||
336 | 431 | maas.mkdir(parents=True) | ||
337 | 432 | maas.joinpath("drivers.yaml").symlink_to( | ||
338 | 433 | src.joinpath("etc", "maas", "drivers.yaml").resolve()) | ||
339 | 434 | maas.joinpath("templates").symlink_to( | ||
340 | 435 | src.joinpath("etc", "maas", "templates").resolve()) | ||
341 | 436 | # Update the environment. | ||
342 | 423 | self.useFixture(EnvironmentVariable("MAAS_ROOT", self.path)) | 437 | self.useFixture(EnvironmentVariable("MAAS_ROOT", self.path)) |
343 | 424 | else: | 438 | else: |
344 | 425 | raise NotADirectoryError( | 439 | raise NotADirectoryError( |
345 | 426 | 440 | ||
346 | === modified file 'src/maastesting/testcase.py' | |||
347 | --- src/maastesting/testcase.py 2017-01-11 09:19:28 +0000 | |||
348 | +++ src/maastesting/testcase.py 2017-01-25 16:51:52 +0000 | |||
349 | @@ -24,7 +24,10 @@ | |||
350 | 24 | import crochet | 24 | import crochet |
351 | 25 | from maastesting.crochet import EventualResultCatchingMixin | 25 | from maastesting.crochet import EventualResultCatchingMixin |
352 | 26 | from maastesting.factory import factory | 26 | from maastesting.factory import factory |
354 | 27 | from maastesting.fixtures import TempDirectory | 27 | from maastesting.fixtures import ( |
355 | 28 | MAASRootFixture, | ||
356 | 29 | TempDirectory, | ||
357 | 30 | ) | ||
358 | 28 | from maastesting.matchers import DocTestMatches | 31 | from maastesting.matchers import DocTestMatches |
359 | 29 | from maastesting.runtest import ( | 32 | from maastesting.runtest import ( |
360 | 30 | MAASCrochetRunTest, | 33 | MAASCrochetRunTest, |
361 | @@ -154,6 +157,10 @@ | |||
362 | 154 | return MAASRunTest | 157 | return MAASRunTest |
363 | 155 | 158 | ||
364 | 156 | def setUp(self): | 159 | def setUp(self): |
365 | 160 | # Every test gets a pristine MAAS_ROOT, when it is defined. | ||
366 | 161 | if "MAAS_ROOT" in os.environ: | ||
367 | 162 | self.useFixture(MAASRootFixture()) | ||
368 | 163 | |||
369 | 157 | # Capture Twisted logs and add them as a test detail. | 164 | # Capture Twisted logs and add them as a test detail. |
370 | 158 | twistedLog = self.useFixture(TwistedLoggerFixture()) | 165 | twistedLog = self.useFixture(TwistedLoggerFixture()) |
371 | 159 | self.addDetail("Twisted logs", twistedLog.getContent()) | 166 | self.addDetail("Twisted logs", twistedLog.getContent()) |
372 | 160 | 167 | ||
373 | === modified file 'src/maastesting/tests/test_conflict_markers.py' | |||
374 | --- src/maastesting/tests/test_conflict_markers.py 2015-12-01 18:12:59 +0000 | |||
375 | +++ src/maastesting/tests/test_conflict_markers.py 2017-01-25 16:51:52 +0000 | |||
376 | @@ -5,12 +5,16 @@ | |||
377 | 5 | 5 | ||
378 | 6 | __all__ = [] | 6 | __all__ = [] |
379 | 7 | 7 | ||
381 | 8 | from pipes import quote | 8 | from os.path import ( |
382 | 9 | isdir, | ||
383 | 10 | join, | ||
384 | 11 | ) | ||
385 | 9 | from subprocess import ( | 12 | from subprocess import ( |
386 | 10 | PIPE, | 13 | PIPE, |
387 | 11 | Popen, | 14 | Popen, |
388 | 12 | STDOUT, | 15 | STDOUT, |
389 | 13 | ) | 16 | ) |
390 | 17 | from unittest import skipIf | ||
391 | 14 | 18 | ||
392 | 15 | from maastesting import root | 19 | from maastesting import root |
393 | 16 | from maastesting.testcase import MAASTestCase | 20 | from maastesting.testcase import MAASTestCase |
394 | @@ -19,25 +23,24 @@ | |||
395 | 19 | UTF8_TEXT, | 23 | UTF8_TEXT, |
396 | 20 | ) | 24 | ) |
397 | 21 | 25 | ||
398 | 22 | # Do not use '=======' as a conflict marker since it's | ||
399 | 23 | # used in docstrings. | ||
400 | 24 | # Express the conflict markers so that this very file won't contain | ||
401 | 25 | # them. | ||
402 | 26 | CONFLICT_MARKERS = "<" * 7, ">" * 7 | ||
403 | 27 | |||
404 | 28 | 26 | ||
405 | 29 | class TestConflictMarkers(MAASTestCase): | 27 | class TestConflictMarkers(MAASTestCase): |
406 | 28 | """Search for merge conflict markers if this is a branch.""" | ||
407 | 30 | 29 | ||
410 | 31 | def execute(self, *command): | 30 | @skipIf(not isdir(join(root, ".bzr")), "Not a branch.") |
411 | 32 | process = Popen(command, stdout=PIPE, stderr=STDOUT, cwd=root) | 31 | def test_no_conflict_markers(self): |
412 | 32 | # Do not search for '=======' as a conflict marker since it's used in | ||
413 | 33 | # docstrings, search for angle brackets instead. Express the conflict | ||
414 | 34 | # markers as a regular expression so that this very file won't match. | ||
415 | 35 | command = ( | ||
416 | 36 | "bzr ls --kind=file --recursive --versioned --null | " | ||
417 | 37 | "xargs -r0 egrep -I '[<]{7}|[>]{7}' -C 3") | ||
418 | 38 | process = Popen( | ||
419 | 39 | command, shell=True, stdout=PIPE, stderr=STDOUT, cwd=root) | ||
420 | 33 | output, _ = process.communicate() | 40 | output, _ = process.communicate() |
421 | 34 | if len(output) != 0: | 41 | if len(output) != 0: |
423 | 35 | name = "stdout/err from `%s`" % " ".join(map(quote, command)) | 42 | name = "stdout/err from `%s`" % command |
424 | 36 | self.addDetail(name, Content(UTF8_TEXT, lambda: [output])) | 43 | self.addDetail(name, Content(UTF8_TEXT, lambda: [output])) |
432 | 37 | self.assertEqual('', output, "Conflict markers present!") | 44 | self.fail("Conflict markers present!") |
433 | 38 | self.assertEqual(1, process.wait(), "(return code is not one)") | 45 | # Don't check the process's exit code because xargs muddles things. |
434 | 39 | 46 | # Checking the output should suffice. | |
428 | 40 | def test_no_conflict_markers(self): | ||
429 | 41 | command = ["egrep", "-rI", "--exclude=*~", "--exclude-dir=include"] | ||
430 | 42 | command.append("|".join(CONFLICT_MARKERS)) | ||
431 | 43 | self.execute(*command) | ||
435 | 44 | 47 | ||
436 | === modified file 'src/maastesting/tests/test_fixtures.py' | |||
437 | --- src/maastesting/tests/test_fixtures.py 2016-08-09 00:14:06 +0000 | |||
438 | +++ src/maastesting/tests/test_fixtures.py 2017-01-25 16:51:52 +0000 | |||
439 | @@ -266,6 +266,17 @@ | |||
440 | 266 | self.assertThat(fixture.path, Not(SamePath(self.maasroot))) | 266 | self.assertThat(fixture.path, Not(SamePath(self.maasroot))) |
441 | 267 | files_expected = set(listdirs(self.maasroot)) | 267 | files_expected = set(listdirs(self.maasroot)) |
442 | 268 | files_observed = set(listdirs(fixture.path)) | 268 | files_observed = set(listdirs(fixture.path)) |
443 | 269 | |||
444 | 270 | # Some files, like regiond.conf and rackd.conf, may be created by | ||
445 | 271 | # concurrently running test suites. In particular, regiond.conf is | ||
446 | 272 | # created as a side-effect of loading Django's settings which | ||
447 | 273 | # takes place before tests are running. We ignore these files. | ||
448 | 274 | files_missing = files_expected - files_observed | ||
449 | 275 | if "etc/maas/regiond.conf" in files_missing: | ||
450 | 276 | files_expected.remove("etc/maas/regiond.conf") | ||
451 | 277 | if "etc/maas/rackd.conf" in files_missing: | ||
452 | 278 | files_expected.remove("etc/maas/rackd.conf") | ||
453 | 279 | |||
454 | 269 | self.assertThat(files_observed, Equals(files_expected)) | 280 | self.assertThat(files_observed, Equals(files_expected)) |
455 | 270 | self.assertThat(fixture.path, Not(PathExists())) | 281 | self.assertThat(fixture.path, Not(PathExists())) |
456 | 271 | 282 | ||
457 | 272 | 283 | ||
458 | === modified file 'src/metadataserver/models/scriptset.py' | |||
459 | --- src/metadataserver/models/scriptset.py 2017-01-21 00:49:34 +0000 | |||
460 | +++ src/metadataserver/models/scriptset.py 2017-01-25 16:51:52 +0000 | |||
461 | @@ -16,6 +16,7 @@ | |||
462 | 16 | ) | 16 | ) |
463 | 17 | from maasserver.models.cleansave import CleanSave | 17 | from maasserver.models.cleansave import CleanSave |
464 | 18 | from maasserver.preseed import CURTIN_INSTALL_LOG | 18 | from maasserver.preseed import CURTIN_INSTALL_LOG |
465 | 19 | from metadataserver import DefaultMeta | ||
466 | 19 | from metadataserver.enum import ( | 20 | from metadataserver.enum import ( |
467 | 20 | RESULT_TYPE, | 21 | RESULT_TYPE, |
468 | 21 | RESULT_TYPE_CHOICES, | 22 | RESULT_TYPE_CHOICES, |
469 | @@ -91,6 +92,9 @@ | |||
470 | 91 | 92 | ||
471 | 92 | class ScriptSet(CleanSave, Model): | 93 | class ScriptSet(CleanSave, Model): |
472 | 93 | 94 | ||
473 | 95 | class Meta(DefaultMeta): | ||
474 | 96 | """Needed for South to recognize this model.""" | ||
475 | 97 | |||
476 | 94 | objects = ScriptSetManager() | 98 | objects = ScriptSetManager() |
477 | 95 | 99 | ||
478 | 96 | last_ping = DateTimeField(blank=True, null=True) | 100 | last_ping = DateTimeField(blank=True, null=True) |
479 | 97 | 101 | ||
480 | === modified file 'src/provisioningserver/dhcp/__init__.py' | |||
481 | --- src/provisioningserver/dhcp/__init__.py 2016-04-12 22:22:23 +0000 | |||
482 | +++ src/provisioningserver/dhcp/__init__.py 2017-01-25 16:51:52 +0000 | |||
483 | @@ -13,7 +13,7 @@ | |||
484 | 13 | abstractproperty, | 13 | abstractproperty, |
485 | 14 | ) | 14 | ) |
486 | 15 | 15 | ||
488 | 16 | from provisioningserver.path import get_path | 16 | from provisioningserver.path import get_tentative_path |
489 | 17 | 17 | ||
490 | 18 | # Location of the DHCPv4 configuration file. | 18 | # Location of the DHCPv4 configuration file. |
491 | 19 | DHCPv4_CONFIG_FILE = '/var/lib/maas/dhcpd.conf' | 19 | DHCPv4_CONFIG_FILE = '/var/lib/maas/dhcpd.conf' |
492 | @@ -65,8 +65,8 @@ | |||
493 | 65 | 65 | ||
494 | 66 | descriptive_name = "DHCPv4" | 66 | descriptive_name = "DHCPv4" |
495 | 67 | template_basename = 'dhcpd.conf.template' | 67 | template_basename = 'dhcpd.conf.template' |
498 | 68 | interfaces_filename = get_path(DHCPv4_INTERFACES_FILE) | 68 | interfaces_filename = get_tentative_path(DHCPv4_INTERFACES_FILE) |
499 | 69 | config_filename = get_path(DHCPv4_CONFIG_FILE) | 69 | config_filename = get_tentative_path(DHCPv4_CONFIG_FILE) |
500 | 70 | dhcp_service = "dhcpd" | 70 | dhcp_service = "dhcpd" |
501 | 71 | ipv6 = False | 71 | ipv6 = False |
502 | 72 | 72 | ||
503 | @@ -79,7 +79,7 @@ | |||
504 | 79 | 79 | ||
505 | 80 | descriptive_name = "DHCPv6" | 80 | descriptive_name = "DHCPv6" |
506 | 81 | template_basename = 'dhcpd6.conf.template' | 81 | template_basename = 'dhcpd6.conf.template' |
509 | 82 | interfaces_filename = get_path(DHCPv6_INTERFACES_FILE) | 82 | interfaces_filename = get_tentative_path(DHCPv6_INTERFACES_FILE) |
510 | 83 | config_filename = get_path(DHCPv6_CONFIG_FILE) | 83 | config_filename = get_tentative_path(DHCPv6_CONFIG_FILE) |
511 | 84 | dhcp_service = "dhcpd6" | 84 | dhcp_service = "dhcpd6" |
512 | 85 | ipv6 = True | 85 | ipv6 = True |
513 | 86 | 86 | ||
514 | === modified file 'src/provisioningserver/rpc/tests/test_clusterservice.py' | |||
515 | --- src/provisioningserver/rpc/tests/test_clusterservice.py 2017-01-24 09:24:36 +0000 | |||
516 | +++ src/provisioningserver/rpc/tests/test_clusterservice.py 2017-01-25 16:51:52 +0000 | |||
517 | @@ -1229,6 +1229,7 @@ | |||
518 | 1229 | # self.assertTrue(client.isSecure()) | 1229 | # self.assertTrue(client.isSecure()) |
519 | 1230 | 1230 | ||
520 | 1231 | def test_authenticateRegion_accepts_matching_digests(self): | 1231 | def test_authenticateRegion_accepts_matching_digests(self): |
521 | 1232 | set_shared_secret_on_filesystem(factory.make_bytes()) | ||
522 | 1232 | client = self.make_running_client() | 1233 | client = self.make_running_client() |
523 | 1233 | 1234 | ||
524 | 1234 | def calculate_digest(_, message): | 1235 | def calculate_digest(_, message): |
525 | @@ -1243,6 +1244,7 @@ | |||
526 | 1243 | self.assertTrue(extract_result(d)) | 1244 | self.assertTrue(extract_result(d)) |
527 | 1244 | 1245 | ||
528 | 1245 | def test_authenticateRegion_rejects_non_matching_digests(self): | 1246 | def test_authenticateRegion_rejects_non_matching_digests(self): |
529 | 1247 | set_shared_secret_on_filesystem(factory.make_bytes()) | ||
530 | 1246 | client = self.make_running_client() | 1248 | client = self.make_running_client() |
531 | 1247 | 1249 | ||
532 | 1248 | def calculate_digest(_, message): | 1250 | def calculate_digest(_, message): |
533 | 1249 | 1251 | ||
534 | === modified file 'src/provisioningserver/tests/test_config.py' | |||
535 | --- src/provisioningserver/tests/test_config.py 2016-08-11 01:50:21 +0000 | |||
536 | +++ src/provisioningserver/tests/test_config.py 2017-01-25 16:51:52 +0000 | |||
537 | @@ -19,6 +19,7 @@ | |||
538 | 19 | from fixtures import EnvironmentVariableFixture | 19 | from fixtures import EnvironmentVariableFixture |
539 | 20 | import formencode | 20 | import formencode |
540 | 21 | import formencode.validators | 21 | import formencode.validators |
541 | 22 | import maastesting | ||
542 | 22 | from maastesting.factory import factory | 23 | from maastesting.factory import factory |
543 | 23 | from maastesting.fixtures import ImportErrorFixture | 24 | from maastesting.fixtures import ImportErrorFixture |
544 | 24 | from maastesting.matchers import ( | 25 | from maastesting.matchers import ( |
545 | @@ -638,8 +639,9 @@ | |||
546 | 638 | self.assertEqual({"tftp_port": example_port}, config.store) | 639 | self.assertEqual({"tftp_port": example_port}, config.store) |
547 | 639 | 640 | ||
548 | 640 | def test_default_tftp_root(self): | 641 | def test_default_tftp_root(self): |
551 | 641 | maas_root = os.getenv("MAAS_ROOT") | 642 | # The default tftp_root is calculated relative to MAAS_ROOT at module |
552 | 642 | self.assertIsNotNone(maas_root) | 643 | # import time, so we need to recreate that value. |
553 | 644 | maas_root = os.path.join(maastesting.root, "run") | ||
554 | 643 | config = ClusterConfiguration({}) | 645 | config = ClusterConfiguration({}) |
555 | 644 | self.assertEqual( | 646 | self.assertEqual( |
556 | 645 | os.path.join(maas_root, "var/lib/maas/boot-resources/current"), | 647 | os.path.join(maas_root, "var/lib/maas/boot-resources/current"), |
557 | 646 | 648 | ||
558 | === modified file 'src/provisioningserver/utils/fs.py' | |||
559 | --- src/provisioningserver/utils/fs.py 2016-12-14 08:43:09 +0000 | |||
560 | +++ src/provisioningserver/utils/fs.py 2017-01-25 16:51:52 +0000 | |||
561 | @@ -45,6 +45,7 @@ | |||
562 | 45 | import threading | 45 | import threading |
563 | 46 | from time import sleep | 46 | from time import sleep |
564 | 47 | 47 | ||
565 | 48 | from provisioningserver.path import get_path | ||
566 | 48 | from provisioningserver.utils import sudo | 49 | from provisioningserver.utils import sudo |
567 | 49 | from provisioningserver.utils.shell import ExternalProcessError | 50 | from provisioningserver.utils.shell import ExternalProcessError |
568 | 50 | from provisioningserver.utils.twisted import retries | 51 | from provisioningserver.utils.twisted import retries |
569 | @@ -485,7 +486,7 @@ | |||
570 | 485 | 486 | ||
571 | 486 | 487 | ||
572 | 487 | class RunLock(SystemLock): | 488 | class RunLock(SystemLock): |
574 | 488 | """Always create a lock file at ``/run/lock/maas@${path,modified}``. | 489 | """Lock file at ``${MAAS_ROOT}/run/lock/maas@${path,modified}``. |
575 | 489 | 490 | ||
576 | 490 | This implements an advisory file lock, by proxy, on the given file-system | 491 | This implements an advisory file lock, by proxy, on the given file-system |
577 | 491 | path. This is especially useful if you do not have permissions to the | 492 | path. This is especially useful if you do not have permissions to the |
578 | @@ -498,12 +499,12 @@ | |||
579 | 498 | def __init__(self, path): | 499 | def __init__(self, path): |
580 | 499 | abspath = FilePath(path).asTextMode().path.lstrip("/") | 500 | abspath = FilePath(path).asTextMode().path.lstrip("/") |
581 | 500 | discriminator = abspath.replace(":", "::").replace("/", ":") | 501 | discriminator = abspath.replace(":", "::").replace("/", ":") |
583 | 501 | lockpath = "/run/lock/maas@%s" % discriminator | 502 | lockpath = get_path("run", "lock", "maas@%s" % discriminator) |
584 | 502 | super(RunLock, self).__init__(lockpath) | 503 | super(RunLock, self).__init__(lockpath) |
585 | 503 | 504 | ||
586 | 504 | 505 | ||
587 | 505 | class NamedLock(SystemLock): | 506 | class NamedLock(SystemLock): |
589 | 506 | """Always create a lock file at ``/run/lock/maas.${name}``. | 507 | """Lock file at ``${MAAS_ROOT}/run/lock/maas.${name}``. |
590 | 507 | 508 | ||
591 | 508 | This implements an advisory lock, by proxy, for an abstract name. The name | 509 | This implements an advisory lock, by proxy, for an abstract name. The name |
592 | 509 | must be a string and can contain only numerical digits, hyphens, and ASCII | 510 | must be a string and can contain only numerical digits, hyphens, and ASCII |
593 | @@ -524,5 +525,5 @@ | |||
594 | 524 | "Lock name contains illegal characters: %s" | 525 | "Lock name contains illegal characters: %s" |
595 | 525 | % "".join(sorted(illegal))) | 526 | % "".join(sorted(illegal))) |
596 | 526 | else: | 527 | else: |
598 | 527 | lockpath = "/run/lock/maas:%s" % name | 528 | lockpath = get_path("run", "lock", "maas:%s" % name) |
599 | 528 | super(NamedLock, self).__init__(lockpath) | 529 | super(NamedLock, self).__init__(lockpath) |
600 | 529 | 530 | ||
601 | === modified file 'src/provisioningserver/utils/tests/test_fs.py' | |||
602 | --- src/provisioningserver/utils/tests/test_fs.py 2017-01-06 09:50:17 +0000 | |||
603 | +++ src/provisioningserver/utils/tests/test_fs.py 2017-01-25 16:51:52 +0000 | |||
604 | @@ -33,6 +33,7 @@ | |||
605 | 33 | from maastesting.testcase import MAASTestCase | 33 | from maastesting.testcase import MAASTestCase |
606 | 34 | from maastesting.utils import age_file | 34 | from maastesting.utils import age_file |
607 | 35 | import provisioningserver.config | 35 | import provisioningserver.config |
608 | 36 | from provisioningserver.path import get_tentative_path | ||
609 | 36 | from provisioningserver.utils.fs import ( | 37 | from provisioningserver.utils.fs import ( |
610 | 37 | atomic_copy, | 38 | atomic_copy, |
611 | 38 | atomic_delete, | 39 | atomic_delete, |
612 | @@ -773,13 +774,13 @@ | |||
613 | 773 | 774 | ||
614 | 774 | def test__string_path(self): | 775 | def test__string_path(self): |
615 | 775 | filename = '/foo/bar/123:456.txt' | 776 | filename = '/foo/bar/123:456.txt' |
617 | 776 | expected = '/run/lock/maas@foo:bar:123::456.txt' | 777 | expected = get_tentative_path('/run/lock/maas@foo:bar:123::456.txt') |
618 | 777 | observed = RunLock(filename).path | 778 | observed = RunLock(filename).path |
619 | 778 | self.assertEqual(expected, observed) | 779 | self.assertEqual(expected, observed) |
620 | 779 | 780 | ||
621 | 780 | def test__byte_path(self): | 781 | def test__byte_path(self): |
622 | 781 | filename = b'/foo/bar/123:456.txt' | 782 | filename = b'/foo/bar/123:456.txt' |
624 | 782 | expected = '/run/lock/maas@foo:bar:123::456.txt' | 783 | expected = get_tentative_path('/run/lock/maas@foo:bar:123::456.txt') |
625 | 783 | observed = RunLock(filename).path | 784 | observed = RunLock(filename).path |
626 | 784 | self.assertEqual(expected, observed) | 785 | self.assertEqual(expected, observed) |
627 | 785 | 786 | ||
628 | @@ -789,7 +790,7 @@ | |||
629 | 789 | 790 | ||
630 | 790 | def test__string_name(self): | 791 | def test__string_name(self): |
631 | 791 | name = factory.make_name("lock") | 792 | name = factory.make_name("lock") |
633 | 792 | expected = '/run/lock/maas:' + name | 793 | expected = get_tentative_path('/run/lock/maas:' + name) |
634 | 793 | observed = NamedLock(name).path | 794 | observed = NamedLock(name).path |
635 | 794 | self.assertEqual(expected, observed) | 795 | self.assertEqual(expected, observed) |
636 | 795 | 796 | ||
637 | 796 | 797 | ||
638 | === modified file 'src/provisioningserver/utils/tests/test_utils.py' | |||
639 | --- src/provisioningserver/utils/tests/test_utils.py 2016-09-22 02:53:33 +0000 | |||
640 | +++ src/provisioningserver/utils/tests/test_utils.py 2017-01-25 16:51:52 +0000 | |||
641 | @@ -13,7 +13,6 @@ | |||
642 | 13 | from unittest.mock import sentinel | 13 | from unittest.mock import sentinel |
643 | 14 | 14 | ||
644 | 15 | from fixtures import EnvironmentVariableFixture | 15 | from fixtures import EnvironmentVariableFixture |
645 | 16 | from maastesting import root | ||
646 | 17 | from maastesting.factory import factory | 16 | from maastesting.factory import factory |
647 | 18 | from maastesting.testcase import MAASTestCase | 17 | from maastesting.testcase import MAASTestCase |
648 | 19 | import provisioningserver | 18 | import provisioningserver |
649 | @@ -40,20 +39,17 @@ | |||
650 | 40 | ) | 39 | ) |
651 | 41 | 40 | ||
652 | 42 | 41 | ||
660 | 43 | def get_branch_dir(*path): | 42 | def get_run_path(*path): |
661 | 44 | """Locate a file or directory relative to ``MAAS_ROOT``. | 43 | """Locate a file or directory relative to ``MAAS_ROOT``.""" |
662 | 45 | 44 | maas_root = os.environ["MAAS_ROOT"] | |
663 | 46 | This function assumes that ``MAAS_ROOT`` has been set to a ``run`` | 45 | return os.path.abspath(os.path.join(maas_root, *path)) |
657 | 47 | subdirectory of this working-tree's root. | ||
658 | 48 | """ | ||
659 | 49 | return os.path.abspath(os.path.join(root, "run", *path)) | ||
664 | 50 | 46 | ||
665 | 51 | 47 | ||
666 | 52 | class TestLocateConfig(MAASTestCase): | 48 | class TestLocateConfig(MAASTestCase): |
667 | 53 | """Tests for `locate_config`.""" | 49 | """Tests for `locate_config`.""" |
668 | 54 | 50 | ||
669 | 55 | def test_returns_branch_etc_maas(self): | 51 | def test_returns_branch_etc_maas(self): |
671 | 56 | self.assertEqual(get_branch_dir('etc/maas'), locate_config()) | 52 | self.assertEqual(get_run_path('etc/maas'), locate_config()) |
672 | 57 | self.assertThat(locate_config(), DirExists()) | 53 | self.assertThat(locate_config(), DirExists()) |
673 | 58 | 54 | ||
674 | 59 | def test_defaults_to_global_etc_maas_if_variable_is_unset(self): | 55 | def test_defaults_to_global_etc_maas_if_variable_is_unset(self): |
675 | @@ -71,18 +67,18 @@ | |||
676 | 71 | def test_locates_config_file(self): | 67 | def test_locates_config_file(self): |
677 | 72 | filename = factory.make_string() | 68 | filename = factory.make_string() |
678 | 73 | self.assertEqual( | 69 | self.assertEqual( |
680 | 74 | get_branch_dir('etc/maas/', filename), | 70 | get_run_path('etc/maas/', filename), |
681 | 75 | locate_config(filename)) | 71 | locate_config(filename)) |
682 | 76 | 72 | ||
683 | 77 | def test_locates_full_path(self): | 73 | def test_locates_full_path(self): |
684 | 78 | path = [factory.make_string() for counter in range(3)] | 74 | path = [factory.make_string() for counter in range(3)] |
685 | 79 | self.assertEqual( | 75 | self.assertEqual( |
687 | 80 | get_branch_dir('etc/maas/', *path), | 76 | get_run_path('etc/maas/', *path), |
688 | 81 | locate_config(*path)) | 77 | locate_config(*path)) |
689 | 82 | 78 | ||
690 | 83 | def test_normalizes_path(self): | 79 | def test_normalizes_path(self): |
691 | 84 | self.assertEqual( | 80 | self.assertEqual( |
693 | 85 | get_branch_dir('etc/maas/bar/szot'), | 81 | get_run_path('etc/maas/bar/szot'), |
694 | 86 | locate_config('foo/.././bar///szot')) | 82 | locate_config('foo/.././bar///szot')) |
695 | 87 | 83 | ||
696 | 88 | 84 |
Looks good. Just a couple of questions.