Merge lp:~jtv/maas/unify-get_name_and_vlan_from_cluster_interface into lp:~maas-committers/maas/trunk
- unify-get_name_and_vlan_from_cluster_interface
- Merge into trunk
Proposed by
Jeroen T. Vermeulen
Status: | Merged |
---|---|
Approved by: | Jeroen T. Vermeulen |
Approved revision: | no longer in the source branch. |
Merged at revision: | 3374 |
Proposed branch: | lp:~jtv/maas/unify-get_name_and_vlan_from_cluster_interface |
Merge into: | lp:~maas-committers/maas/trunk |
Diff against target: |
535 lines (+164/-142) 12 files modified
src/maasserver/forms.py (+6/-3) src/maasserver/migrations/0099_convert_cluster_interfaces_to_networks.py (+7/-18) src/maasserver/models/network.py (+0/-16) src/maasserver/models/tests/test_network.py (+0/-39) src/maasserver/testing/factory.py (+0/-23) src/maasserver/testing/tests/test_factory.py (+2/-39) src/maasserver/tests/test_forms_nodegroupinterface.py (+5/-2) src/maasserver/utils/interfaces.py (+19/-0) src/maasserver/utils/tests/test_interfaces.py (+59/-1) src/maastesting/factory.py (+22/-0) src/maastesting/tests/test_factory.py (+20/-0) src/maastesting/utils.py (+24/-1) |
To merge this branch: | bzr merge lp:~jtv/maas/unify-get_name_and_vlan_from_cluster_interface |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Gavin Panella (community) | Approve | ||
Review via email: mp+241738@code.launchpad.net |
Commit message
Unify our two implementations of get_name_
Description of the change
This was harder than I expected: there were some other helpers that needed moving. Fixing the actual bug should be easy by comparison.
Along the way I made the function independent of the model, otherwise it wouldn't be advisable to use it in migration code. It was a simple matter of externalising retrieval of two fields, and making those the parameters. But otherwise, nothing substantial changes.
Jeroen
To post a comment you must log in.
Revision history for this message
Jeroen T. Vermeulen (jtv) wrote : | # |
Thanks. Made some changes "inspired by" your notes, as they say in Hollywood nowadays. :-)
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | === modified file 'src/maasserver/forms.py' |
2 | --- src/maasserver/forms.py 2014-11-13 02:17:06 +0000 |
3 | +++ src/maasserver/forms.py 2014-11-14 14:24:50 +0000 |
4 | @@ -125,7 +125,6 @@ |
5 | Tag, |
6 | Zone, |
7 | ) |
8 | -from maasserver.models.network import get_name_and_vlan_from_cluster_interface |
9 | from maasserver.models.node import ( |
10 | fqdn_is_duplicate, |
11 | nodegroup_fqdn, |
12 | @@ -138,7 +137,10 @@ |
13 | ) |
14 | from maasserver.utils import strip_domain |
15 | from maasserver.utils.forms import compose_invalid_choice_text |
16 | -from maasserver.utils.interfaces import make_name_from_interface |
17 | +from maasserver.utils.interfaces import ( |
18 | + get_name_and_vlan_from_cluster_interface, |
19 | + make_name_from_interface, |
20 | + ) |
21 | from maasserver.utils.orm import get_one |
22 | from maasserver.utils.osystems import ( |
23 | get_distro_series_initial, |
24 | @@ -1409,7 +1411,8 @@ |
25 | # Can be None or empty string, do nothing if so. |
26 | return |
27 | |
28 | - name, vlan_tag = get_name_and_vlan_from_cluster_interface(interface) |
29 | + name, vlan_tag = get_name_and_vlan_from_cluster_interface( |
30 | + interface.name, interface.interface) |
31 | ipnetwork = make_network(interface.ip, interface.subnet_mask) |
32 | network = Network( |
33 | name=name, |
34 | |
35 | === modified file 'src/maasserver/migrations/0099_convert_cluster_interfaces_to_networks.py' |
36 | --- src/maasserver/migrations/0099_convert_cluster_interfaces_to_networks.py 2014-08-25 02:47:21 +0000 |
37 | +++ src/maasserver/migrations/0099_convert_cluster_interfaces_to_networks.py 2014-11-14 14:24:50 +0000 |
38 | @@ -5,28 +5,15 @@ |
39 | transaction, |
40 | ) |
41 | from django.db.utils import IntegrityError |
42 | +from maasserver.utils.interfaces import ( |
43 | + get_name_and_vlan_from_cluster_interface, |
44 | + ) |
45 | from netaddr import IPNetwork |
46 | from south.db import db |
47 | from south.utils import datetime_utils as datetime |
48 | from south.v2 import DataMigration |
49 | |
50 | |
51 | -def get_name_and_vlan_from_cluster_interface(cluster_interface): |
52 | - """Given a `NodeGroupInterface`, return a name suitable for a `Network`. |
53 | - |
54 | - :return: a tuple of the new name and vlan tag. vlan tag may be None |
55 | - """ |
56 | - name = cluster_interface.interface |
57 | - nodegroup_name = cluster_interface.nodegroup.name |
58 | - vlan_tag = None |
59 | - if '.' in name: |
60 | - _, vlan_tag = name.split('.', 1) |
61 | - name = name.replace('.', '-') |
62 | - name = name.replace(':', '-') |
63 | - network_name = "-".join((nodegroup_name, name)) |
64 | - return network_name, vlan_tag |
65 | - |
66 | - |
67 | class Migration(DataMigration): |
68 | |
69 | def forwards(self, orm): |
70 | @@ -36,8 +23,10 @@ |
71 | # Can be None or empty string, do nothing if so. |
72 | continue |
73 | |
74 | - name, vlan_tag = get_name_and_vlan_from_cluster_interface(interface) |
75 | - ipnetwork = IPNetwork("%s/%s" % (interface.ip, interface.subnet_mask)) |
76 | + name, vlan_tag = get_name_and_vlan_from_cluster_interface( |
77 | + interface.nodegroup.name, interface.interface) |
78 | + ipnetwork = IPNetwork( |
79 | + "%s/%s" % (interface.ip, interface.subnet_mask)) |
80 | network = orm['maasserver.Network']( |
81 | name=name, |
82 | ip=unicode(ipnetwork.network), |
83 | |
84 | === modified file 'src/maasserver/models/network.py' |
85 | --- src/maasserver/models/network.py 2014-09-17 09:20:05 +0000 |
86 | +++ src/maasserver/models/network.py 2014-11-14 14:24:50 +0000 |
87 | @@ -186,22 +186,6 @@ |
88 | return specifier_class(spec) |
89 | |
90 | |
91 | -def get_name_and_vlan_from_cluster_interface(cluster_interface): |
92 | - """Given a `NodeGroupInterface`, return a name suitable for a `Network`. |
93 | - |
94 | - :return: a tuple of the new name and vlan tag. vlan tag may be None |
95 | - """ |
96 | - name = cluster_interface.interface |
97 | - nodegroup_name = cluster_interface.nodegroup.name |
98 | - vlan_tag = None |
99 | - if '.' in name: |
100 | - _, vlan_tag = name.split('.', 1) |
101 | - name = name.replace('.', '-') |
102 | - name = name.replace(':', '-') |
103 | - network_name = "-".join((nodegroup_name, name)) |
104 | - return network_name, vlan_tag |
105 | - |
106 | - |
107 | class NetworkManager(Manager): |
108 | """Manager for :class:`Network` model class. |
109 | |
110 | |
111 | === modified file 'src/maasserver/models/tests/test_network.py' |
112 | --- src/maasserver/models/tests/test_network.py 2014-09-19 03:12:47 +0000 |
113 | +++ src/maasserver/models/tests/test_network.py 2014-11-14 14:24:50 +0000 |
114 | @@ -20,7 +20,6 @@ |
115 | from django.db.models.query import QuerySet |
116 | from maasserver.models import Network |
117 | from maasserver.models.network import ( |
118 | - get_name_and_vlan_from_cluster_interface, |
119 | get_specifier_type, |
120 | IPSpecifier, |
121 | NameSpecifier, |
122 | @@ -30,7 +29,6 @@ |
123 | from maasserver.testing.factory import factory |
124 | from maasserver.testing.orm import reload_object |
125 | from maasserver.testing.testcase import MAASServerTestCase |
126 | -from mock import sentinel |
127 | from netaddr import ( |
128 | IPAddress, |
129 | IPNetwork, |
130 | @@ -132,43 +130,6 @@ |
131 | self.assertRaises(ValidationError, parse_network_spec, '10.4.4.4') |
132 | |
133 | |
134 | -class TestGetNameAndVlanFromClusterInterface(MAASServerTestCase): |
135 | - """Tests for `get_name_and_vlan_from_cluster_interface`.""" |
136 | - |
137 | - def make_interface_name(self, basename): |
138 | - interface = sentinel.interface |
139 | - interface.nodegroup = sentinel.nodegroup |
140 | - interface.nodegroup.name = factory.make_name('name') |
141 | - interface.interface = basename |
142 | - return interface |
143 | - |
144 | - def test_returns_simple_name_unaltered(self): |
145 | - interface = self.make_interface_name(factory.make_name('iface')) |
146 | - name, vlan_tag = get_name_and_vlan_from_cluster_interface(interface) |
147 | - expected_name = '%s-%s' % ( |
148 | - interface.nodegroup.name, interface.interface) |
149 | - self.assertEqual((expected_name, None), (name, vlan_tag)) |
150 | - |
151 | - def test_substitutes_colon(self): |
152 | - interface = self.make_interface_name('eth0:0') |
153 | - name, vlan_tag = get_name_and_vlan_from_cluster_interface(interface) |
154 | - expected_name = '%s-eth0-0' % interface.nodegroup.name |
155 | - self.assertEqual((expected_name, None), (name, vlan_tag)) |
156 | - |
157 | - def test_returns_with_vlan_tag(self): |
158 | - interface = self.make_interface_name('eth0.5') |
159 | - name, vlan_tag = get_name_and_vlan_from_cluster_interface(interface) |
160 | - expected_name = '%s-eth0-5' % interface.nodegroup.name |
161 | - self.assertEqual((expected_name, '5'), (name, vlan_tag)) |
162 | - |
163 | - def test_returns_name_with_crazy_colon_and_vlan(self): |
164 | - # For truly twisted network admins. |
165 | - interface = self.make_interface_name('eth0:2.3') |
166 | - name, vlan_tag = get_name_and_vlan_from_cluster_interface(interface) |
167 | - expected_name = '%s-eth0-2-3' % interface.nodegroup.name |
168 | - self.assertEqual((expected_name, '3'), (name, vlan_tag)) |
169 | - |
170 | - |
171 | class TestNetworkManager(MAASServerTestCase): |
172 | |
173 | def test_get_from_spec_validates_first(self): |
174 | |
175 | === modified file 'src/maasserver/testing/factory.py' |
176 | --- src/maasserver/testing/factory.py 2014-11-13 02:17:06 +0000 |
177 | +++ src/maasserver/testing/factory.py 2014-11-14 14:24:50 +0000 |
178 | @@ -748,29 +748,6 @@ |
179 | |
180 | make_zone = make_Zone |
181 | |
182 | - def make_vlan_tag(self, allow_none=False, but_not=None): |
183 | - """Create a random VLAN tag. |
184 | - |
185 | - :param allow_none: Whether `None` ("no VLAN") can be allowed as an |
186 | - outcome. If `True`, `None` will be included in the possible |
187 | - results with a deliberately over-represented probability, in order |
188 | - to help trip up bugs that might only show up once in about 4094 |
189 | - calls otherwise. |
190 | - :param but_not: A list of tags that should not be returned. Any zero |
191 | - or `None` entries will be ignored. |
192 | - """ |
193 | - if but_not is None: |
194 | - but_not = [] |
195 | - if allow_none and random.randint(0, 1) == 0: |
196 | - return None |
197 | - else: |
198 | - for _ in range(100): |
199 | - vlan_tag = random.randint(1, 0xffe) |
200 | - if vlan_tag not in but_not: |
201 | - return vlan_tag |
202 | - raise maastesting.factory.TooManyRandomRetries( |
203 | - "Could not find an available VLAN tag.") |
204 | - |
205 | def make_Network(self, name=None, network=None, vlan_tag=NO_VALUE, |
206 | description=None, sortable_name=False, |
207 | disjoint_from=None, default_gateway=None, |
208 | |
209 | === modified file 'src/maasserver/testing/tests/test_factory.py' |
210 | --- src/maasserver/testing/tests/test_factory.py 2014-09-10 16:20:31 +0000 |
211 | +++ src/maasserver/testing/tests/test_factory.py 2014-11-14 14:24:50 +0000 |
212 | @@ -24,28 +24,7 @@ |
213 | from maasserver.testing.orm import reload_object |
214 | from maasserver.testing.testcase import MAASServerTestCase |
215 | from maastesting.factory import TooManyRandomRetries |
216 | - |
217 | - |
218 | -class FakeRandInt: |
219 | - """Fake `randint` which forced limitations on its range. |
220 | - |
221 | - This lets you set a forced minimum, and/or a forced maximum, on the range |
222 | - of any call. For example, if you pass `forced_maximum=3`, then a call |
223 | - will never return more than 3. If you don't set a maximum, or if the |
224 | - call's maximum argument is less than the forced maximum, then the call's |
225 | - maximum will be respected. |
226 | - """ |
227 | - def __init__(self, real_randint, forced_minimum=None, forced_maximum=None): |
228 | - self.real_randint = real_randint |
229 | - self.minimum = forced_minimum |
230 | - self.maximum = forced_maximum |
231 | - |
232 | - def __call__(self, minimum, maximum): |
233 | - if self.minimum is not None: |
234 | - minimum = max(minimum, self.minimum) |
235 | - if self.maximum is not None: |
236 | - maximum = min(maximum, self.maximum) |
237 | - return self.real_randint(minimum, maximum) |
238 | +from maastesting.utils import FakeRandInt |
239 | |
240 | |
241 | class TestFactory(MAASServerTestCase): |
242 | @@ -127,23 +106,6 @@ |
243 | node = reload_object(node) |
244 | self.assertEqual(previous_zone, node.zone) |
245 | |
246 | - def test_make_vlan_tag_excludes_None_by_default(self): |
247 | - # Artificially limit randint to a very narrow range, to guarantee |
248 | - # some repetition in its output, and virtually guarantee that we test |
249 | - # both outcomes of the flip-a-coin call in make_vlan_tag. |
250 | - self.patch(random, 'randint', FakeRandInt(random.randint, 0, 1)) |
251 | - outcomes = {factory.make_vlan_tag() for _ in range(1000)} |
252 | - self.assertEqual({1}, outcomes) |
253 | - |
254 | - def test_make_vlan_tags_includes_None_if_allow_none(self): |
255 | - self.patch(random, 'randint', FakeRandInt(random.randint, 0, 1)) |
256 | - self.assertEqual( |
257 | - {None, 1}, |
258 | - { |
259 | - factory.make_vlan_tag(allow_none=True) |
260 | - for _ in range(1000) |
261 | - }) |
262 | - |
263 | def test_make_Networks_lowers_names_if_sortable_name(self): |
264 | networks = factory.make_Networks(10, sortable_name=True) |
265 | self.assertEqual( |
266 | @@ -183,4 +145,5 @@ |
267 | def test_make_Networks_gives_up_if_random_tags_keep_clashing(self): |
268 | self.patch(factory, 'make_Network') |
269 | self.patch(random, 'randint', lambda *args: 1) |
270 | + self.patch(factory, 'pick_bool', lambda *args: False) |
271 | self.assertRaises(TooManyRandomRetries, factory.make_Networks, 2) |
272 | |
273 | === modified file 'src/maasserver/tests/test_forms_nodegroupinterface.py' |
274 | --- src/maasserver/tests/test_forms_nodegroupinterface.py 2014-10-30 11:08:43 +0000 |
275 | +++ src/maasserver/tests/test_forms_nodegroupinterface.py 2014-11-14 14:24:50 +0000 |
276 | @@ -29,10 +29,12 @@ |
277 | Network, |
278 | NodeGroupInterface, |
279 | ) |
280 | -from maasserver.models.network import get_name_and_vlan_from_cluster_interface |
281 | from maasserver.models.staticipaddress import StaticIPAddress |
282 | from maasserver.testing.factory import factory |
283 | from maasserver.testing.testcase import MAASServerTestCase |
284 | +from maasserver.utils.interfaces import ( |
285 | + get_name_and_vlan_from_cluster_interface, |
286 | + ) |
287 | from maastesting.matchers import MockCalledOnceWith |
288 | from netaddr import ( |
289 | IPAddress, |
290 | @@ -369,7 +371,8 @@ |
291 | form = NodeGroupInterfaceForm(data=int_settings, instance=interface) |
292 | form.save() |
293 | [network] = Network.objects.all() |
294 | - expected, _ = get_name_and_vlan_from_cluster_interface(interface) |
295 | + expected, _ = get_name_and_vlan_from_cluster_interface( |
296 | + interface.name, interface.interface) |
297 | self.assertEqual(expected, network.name) |
298 | |
299 | def test_sets_vlan_tag(self): |
300 | |
301 | === modified file 'src/maasserver/utils/interfaces.py' |
302 | --- src/maasserver/utils/interfaces.py 2014-08-13 21:49:35 +0000 |
303 | +++ src/maasserver/utils/interfaces.py 2014-11-14 14:24:50 +0000 |
304 | @@ -13,6 +13,7 @@ |
305 | |
306 | __metaclass__ = type |
307 | __all__ = [ |
308 | + 'get_name_and_vlan_from_cluster_interface', |
309 | 'make_name_from_interface', |
310 | ] |
311 | |
312 | @@ -34,3 +35,21 @@ |
313 | else: |
314 | base_name = interface |
315 | return re.sub(u'[^\w:.-]', '--', base_name) |
316 | + |
317 | + |
318 | +def get_name_and_vlan_from_cluster_interface(cluster_name, interface): |
319 | + """Return a name suitable for a `Network` managed by a cluster interface. |
320 | + |
321 | + :param interface: Network interface name, e.g. `eth0:1`. |
322 | + :param cluster_name: Name of the cluster. |
323 | + :return: a tuple of the new name and the interface's VLAN tag. The VLAN |
324 | + tag may be None. |
325 | + """ |
326 | + name = interface |
327 | + vlan_tag = None |
328 | + if '.' in name: |
329 | + _, vlan_tag = name.split('.', 1) |
330 | + name = name.replace('.', '-') |
331 | + name = name.replace(':', '-') |
332 | + network_name = "-".join((cluster_name, name)) |
333 | + return network_name, vlan_tag |
334 | |
335 | === modified file 'src/maasserver/utils/tests/test_interfaces.py' |
336 | --- src/maasserver/utils/tests/test_interfaces.py 2014-07-04 12:10:01 +0000 |
337 | +++ src/maasserver/utils/tests/test_interfaces.py 2014-11-14 14:24:50 +0000 |
338 | @@ -14,7 +14,12 @@ |
339 | __metaclass__ = type |
340 | __all__ = [] |
341 | |
342 | -from maasserver.utils.interfaces import make_name_from_interface |
343 | +from random import randint |
344 | + |
345 | +from maasserver.utils.interfaces import ( |
346 | + get_name_and_vlan_from_cluster_interface, |
347 | + make_name_from_interface, |
348 | + ) |
349 | from maastesting.factory import factory |
350 | from maastesting.testcase import MAASTestCase |
351 | |
352 | @@ -38,3 +43,56 @@ |
353 | self.assertNotEqual( |
354 | make_name_from_interface(''), |
355 | make_name_from_interface('')) |
356 | + |
357 | + |
358 | +class TestGetNameAndVlanFromClusterInterface(MAASTestCase): |
359 | + """Tests for `get_name_and_vlan_from_cluster_interface`.""" |
360 | + |
361 | + def make_interface(self): |
362 | + """Return a simple network interface name.""" |
363 | + return 'eth%d' % randint(0, 99) |
364 | + |
365 | + def test_returns_simple_name_unaltered(self): |
366 | + cluster = factory.make_name('cluster') |
367 | + interface = factory.make_name('iface') |
368 | + expected_name = '%s-%s' % (cluster, interface) |
369 | + self.assertEqual( |
370 | + (expected_name, None), |
371 | + get_name_and_vlan_from_cluster_interface(cluster, interface)) |
372 | + |
373 | + def test_substitutes_colon(self): |
374 | + cluster = factory.make_name('cluster') |
375 | + base_interface = self.make_interface() |
376 | + alias = randint(0, 99) |
377 | + interface = '%s:%d' % (base_interface, alias) |
378 | + expected_name = '%s-%s-%d' % (cluster, base_interface, alias) |
379 | + self.assertEqual( |
380 | + (expected_name, None), |
381 | + get_name_and_vlan_from_cluster_interface(cluster, interface)) |
382 | + |
383 | + def test_returns_with_vlan_tag(self): |
384 | + cluster = factory.make_name('cluster') |
385 | + base_interface = self.make_interface() |
386 | + vlan_tag = factory.make_vlan_tag() |
387 | + interface = '%s.%d' % (base_interface, vlan_tag) |
388 | + expected_name = '%s-%s-%d' % (cluster, base_interface, vlan_tag) |
389 | + self.assertEqual( |
390 | + (expected_name, '%d' % vlan_tag), |
391 | + get_name_and_vlan_from_cluster_interface(cluster, interface)) |
392 | + |
393 | + def test_returns_name_with_crazy_colon_and_vlan(self): |
394 | + # For truly twisted network admins. |
395 | + cluster = factory.make_name('cluster') |
396 | + base_interface = self.make_interface() |
397 | + vlan_tag = factory.make_vlan_tag() |
398 | + alias = randint(0, 99) |
399 | + interface = '%s:%d.%d' % (base_interface, alias, vlan_tag) |
400 | + expected_name = '%s-%s-%d-%d' % ( |
401 | + cluster, |
402 | + base_interface, |
403 | + alias, |
404 | + vlan_tag, |
405 | + ) |
406 | + self.assertEqual( |
407 | + (expected_name, '%d' % vlan_tag), |
408 | + get_name_and_vlan_from_cluster_interface(cluster, interface)) |
409 | |
410 | === modified file 'src/maastesting/factory.py' |
411 | --- src/maastesting/factory.py 2014-11-07 16:11:53 +0000 |
412 | +++ src/maastesting/factory.py 2014-11-14 14:24:50 +0000 |
413 | @@ -133,6 +133,28 @@ |
414 | assert port_min >= 0 and port_max <= 65535 |
415 | return random.randint(port_min, port_max) |
416 | |
417 | + def make_vlan_tag(self, allow_none=False, but_not=None): |
418 | + """Create a random VLAN tag. |
419 | + |
420 | + :param allow_none: Whether `None` ("no VLAN") can be allowed as an |
421 | + outcome. If `True`, `None` will be included in the possible |
422 | + results with a deliberately over-represented probability, in order |
423 | + to help trip up bugs that might only show up once in about 4094 |
424 | + calls otherwise. |
425 | + :param but_not: A list of tags that should not be returned. Any zero |
426 | + or `None` entries will be ignored. |
427 | + """ |
428 | + if but_not is None: |
429 | + but_not = [] |
430 | + if allow_none and self.pick_bool(): |
431 | + return None |
432 | + else: |
433 | + for _ in range(100): |
434 | + vlan_tag = random.randint(1, 0xffe) |
435 | + if vlan_tag not in but_not: |
436 | + return vlan_tag |
437 | + raise TooManyRandomRetries("Could not find an available VLAN tag.") |
438 | + |
439 | def make_ipv4_address(self): |
440 | octets = islice(self.random_octets, 4) |
441 | return '%d.%d.%d.%d' % tuple(octets) |
442 | |
443 | === modified file 'src/maastesting/tests/test_factory.py' |
444 | --- src/maastesting/tests/test_factory.py 2014-09-17 08:31:08 +0000 |
445 | +++ src/maastesting/tests/test_factory.py 2014-11-14 14:24:50 +0000 |
446 | @@ -1,4 +1,5 @@ |
447 | # Copyright 2012-2014 Canonical Ltd. This software is licensed under the |
448 | + |
449 | # GNU Affero General Public License version 3 (see the file LICENSE). |
450 | |
451 | """Test the factory where appropriate. Don't overdo this.""" |
452 | @@ -17,6 +18,7 @@ |
453 | from datetime import datetime |
454 | from itertools import count |
455 | import os.path |
456 | +import random |
457 | from random import randint |
458 | import subprocess |
459 | |
460 | @@ -25,6 +27,7 @@ |
461 | TooManyRandomRetries, |
462 | ) |
463 | from maastesting.testcase import MAASTestCase |
464 | +from maastesting.utils import FakeRandInt |
465 | from netaddr import ( |
466 | IPAddress, |
467 | IPNetwork, |
468 | @@ -52,6 +55,23 @@ |
469 | def test_pick_port_returns_int(self): |
470 | self.assertIsInstance(factory.pick_port(), int) |
471 | |
472 | + def test_make_vlan_tag_excludes_None_by_default(self): |
473 | + # Artificially limit randint to a very narrow range, to guarantee |
474 | + # some repetition in its output, and virtually guarantee that we test |
475 | + # both outcomes of the flip-a-coin call in make_vlan_tag. |
476 | + self.patch(random, 'randint', FakeRandInt(random.randint, 0, 1)) |
477 | + outcomes = {factory.make_vlan_tag() for _ in range(1000)} |
478 | + self.assertEqual({1}, outcomes) |
479 | + |
480 | + def test_make_vlan_tag_includes_None_if_allow_none(self): |
481 | + self.patch(random, 'randint', FakeRandInt(random.randint, 0, 1)) |
482 | + self.assertEqual( |
483 | + {None, 1}, |
484 | + { |
485 | + factory.make_vlan_tag(allow_none=True) |
486 | + for _ in range(1000) |
487 | + }) |
488 | + |
489 | def test_make_ipv4_address(self): |
490 | ip_address = factory.make_ipv4_address() |
491 | self.assertIsInstance(ip_address, unicode) |
492 | |
493 | === modified file 'src/maastesting/utils.py' |
494 | --- src/maastesting/utils.py 2014-08-22 13:32:56 +0000 |
495 | +++ src/maastesting/utils.py 2014-11-14 14:24:50 +0000 |
496 | @@ -1,4 +1,4 @@ |
497 | -# Copyright 2012 Canonical Ltd. This software is licensed under the |
498 | +# Copyright 2012-2014 Canonical Ltd. This software is licensed under the |
499 | # GNU Affero General Public License version 3 (see the file LICENSE). |
500 | |
501 | """Testing utilities.""" |
502 | @@ -17,6 +17,7 @@ |
503 | "content_from_file", |
504 | "extract_word_list", |
505 | "get_write_time", |
506 | + "FakeRandInt", |
507 | "preexec_fn", |
508 | "run_isolated", |
509 | "sample_binary_data", |
510 | @@ -130,3 +131,25 @@ |
511 | # (1) Provided, of course, that man know only about ASCII and |
512 | # UTF. |
513 | sample_binary_data = codecs.BOM64_LE + codecs.BOM64_BE + b'\x00\xff\x00' |
514 | + |
515 | + |
516 | +class FakeRandInt: |
517 | + """Fake `randint` with forced limitations on its range. |
518 | + |
519 | + This lets you set a forced minimum, and/or a forced maximum, on the range |
520 | + of any call. For example, if you pass `forced_maximum=3`, then a call |
521 | + will never return more than 3. If you don't set a maximum, or if the |
522 | + call's maximum argument is less than the forced maximum, then the call's |
523 | + maximum will be respected. |
524 | + """ |
525 | + def __init__(self, real_randint, forced_minimum=None, forced_maximum=None): |
526 | + self.real_randint = real_randint |
527 | + self.minimum = forced_minimum |
528 | + self.maximum = forced_maximum |
529 | + |
530 | + def __call__(self, minimum, maximum): |
531 | + if self.minimum is not None: |
532 | + minimum = max(minimum, self.minimum) |
533 | + if self.maximum is not None: |
534 | + maximum = min(maximum, self.maximum) |
535 | + return self.real_randint(minimum, maximum) |
Looks fine. I don't like FakeRandInt much, but I don't have a better suggestion offhand, and you've only moved it anyway.