launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #11111
[Merge] lp:~jtv/maas/make-random-leases into lp:maas
Jeroen T. Vermeulen has proposed merging lp:~jtv/maas/make-random-leases into lp:maas.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~jtv/maas/make-random-leases/+merge/120341
This extracts something I found myself repeating too much in tests: create a random leases dict, mapping IP addresses to MAC addresses. It used to make more sense to repeat this, since the format was not particularly prevalent. Nowadays it's used a lot, and very uniformly, so we might as well have a helper for it.
Maybe I'm being unnecessarily nit-picky by guarding against clashes of random IP addresses. We have plenty of other places in the test suite where accidental non-uniqueness of random values would cause transient failures. I just didn't feel like adding to that risk; we might for example decide someday that all random IP addresses have to come from 192.168.254.0/24.
Jeroen
--
https://code.launchpad.net/~jtv/maas/make-random-leases/+merge/120341
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/maas/make-random-leases into lp:maas.
=== modified file 'src/maasserver/tests/test_dhcplease.py'
--- src/maasserver/tests/test_dhcplease.py 2012-08-07 07:35:34 +0000
+++ src/maasserver/tests/test_dhcplease.py 2012-08-20 07:33:24 +0000
@@ -59,10 +59,9 @@
def test_update_leases_creates_new_lease(self):
nodegroup = factory.make_node_group()
- ip = factory.getRandomIPAddress()
- mac = factory.getRandomMACAddress()
- DHCPLease.objects.update_leases(nodegroup, {ip: mac})
- self.assertEqual({ip: mac}, map_leases(nodegroup))
+ lease = factory.make_random_leases()
+ DHCPLease.objects.update_leases(nodegroup, lease)
+ self.assertEqual(lease, map_leases(nodegroup))
def test_update_leases_deletes_obsolete_lease(self):
nodegroup = factory.make_node_group()
@@ -108,8 +107,7 @@
innocent_nodegroup = factory.make_node_group()
innocent_lease = factory.make_dhcp_lease(nodegroup=innocent_nodegroup)
DHCPLease.objects.update_leases(
- factory.make_node_group(),
- {factory.getRandomIPAddress(): factory.getRandomMACAddress()})
+ factory.make_node_group(), factory.make_random_leases())
self.assertItemsEqual(
[innocent_lease], get_leases(innocent_nodegroup))
=== modified file 'src/maastesting/factory.py'
--- src/maastesting/factory.py 2012-08-10 13:10:53 +0000
+++ src/maastesting/factory.py 2012-08-20 07:33:24 +0000
@@ -87,6 +87,15 @@
octets = islice(self.random_octets, 6)
return delimiter.join(format(octet, b"02x") for octet in octets)
+ def make_random_leases(self, num_leases=1):
+ """Create a dict of arbitrary ip-to-mac address mappings."""
+ # This could be a dict comprehension, but the current loop
+ # guards against shortfalls as random IP addresses collide.
+ leases = {}
+ while len(leases) < num_leases:
+ leases[self.getRandomIPAddress()] = self.getRandomMACAddress()
+ return leases
+
def getRandomDate(self, year=2011):
start = time.mktime(datetime.datetime(year, 1, 1).timetuple())
end = time.mktime(datetime.datetime(year + 1, 1, 1).timetuple())
=== modified file 'src/maastesting/tests/test_factory.py'
--- src/maastesting/tests/test_factory.py 2012-08-10 13:10:53 +0000
+++ src/maastesting/tests/test_factory.py 2012-08-20 07:33:24 +0000
@@ -28,6 +28,7 @@
FileContains,
FileExists,
MatchesAll,
+ MatchesRegex,
Not,
StartsWith,
)
@@ -79,6 +80,28 @@
mac_address = factory.getRandomMACAddress(delimiter=b"-")
self.assertEqual("3a-3b-3c-3d-3e-3f", mac_address)
+ def test_make_random_leases_maps_ips_to_macs(self):
+ [(ip, mac)] = factory.make_random_leases().items()
+ self.assertThat(ip, MatchesRegex('[0-9]{1,3}(?:\\.[0-9]{1,3}){3}'))
+ self.assertThat(
+ mac, MatchesRegex('[0-9a-fA-F]{2}(?::[0-9a-fA-F]{2}){5}'))
+
+ def test_make_random_leases_randomizes_ips(self):
+ self.assertNotEqual(
+ factory.make_random_leases().keys(),
+ factory.make_random_leases().keys())
+
+ def test_make_random_leases_randomizes_macs(self):
+ self.assertNotEqual(
+ factory.make_random_leases().values(),
+ factory.make_random_leases().values())
+
+ def test_make_random_leases_returns_requested_number_of_leases(self):
+ num_leases = randint(0, 3)
+ self.assertEqual(
+ num_leases,
+ len(factory.make_random_leases(num_leases)))
+
def test_make_file_creates_file(self):
self.assertThat(factory.make_file(self.make_dir()), FileExists())
=== modified file 'src/provisioningserver/dhcp/tests/test_leases.py'
--- src/provisioningserver/dhcp/tests/test_leases.py 2012-08-17 05:20:19 +0000
+++ src/provisioningserver/dhcp/tests/test_leases.py 2012-08-20 07:33:24 +0000
@@ -60,7 +60,7 @@
def test_record_lease_state_records_time_and_leases(self):
time = datetime.utcnow()
- leases = {factory.getRandomIPAddress(): factory.getRandomMACAddress()}
+ leases = factory.make_random_leases()
record_lease_state(time, leases)
self.assertEqual(
(time, leases),
@@ -78,10 +78,6 @@
class TestUpdateLeases(PservTestCase):
- def make_lease(self):
- """Create a leases dict with one, arbitrary lease in it."""
- return {factory.getRandomIPAddress(): factory.getRandomMACAddress()}
-
def redirect_parser(self, path):
"""Make the leases parser read from a file at `path`."""
self.patch(leases_module, 'DHCP_LEASES_FILE', path)
@@ -183,7 +179,7 @@
def test_record_lease_state_sets_leases_and_timestamp(self):
time = datetime.utcnow()
- leases = {factory.getRandomIPAddress(): factory.getRandomMACAddress()}
+ leases = factory.make_random_leases()
self.set_lease_state()
record_lease_state(time, leases)
self.assertEqual(
@@ -192,7 +188,7 @@
def test_check_lease_changes_returns_tuple_if_no_state_cached(self):
self.set_lease_state()
- leases = self.make_lease()
+ leases = factory.make_random_leases()
leases_file = self.fake_leases_file(leases)
self.assertEqual(
(get_write_time(leases_file), leases),
@@ -218,7 +214,7 @@
self.assertSequenceEqual([], parser.calls)
def test_check_lease_changes_returns_tuple_if_lease_added(self):
- leases = self.make_lease()
+ leases = factory.make_random_leases()
self.set_lease_state(
datetime.utcnow() - timedelta(seconds=10), leases.copy())
leases[factory.getRandomIPAddress()] = factory.getRandomMACAddress()
@@ -229,20 +225,21 @@
def test_check_lease_changes_returns_tuple_if_leases_dropped(self):
self.set_lease_state(
- datetime.utcnow() - timedelta(seconds=10), self.make_lease())
+ datetime.utcnow() - timedelta(seconds=10),
+ factory.make_random_leases())
leases_file = self.fake_leases_file({})
self.assertEqual(
(get_write_time(leases_file), {}),
check_lease_changes())
def test_check_lease_changes_returns_None_if_no_change(self):
- leases = self.make_lease()
+ leases = factory.make_random_leases()
leases_file = self.fake_leases_file(leases)
self.set_lease_state(get_write_time(leases_file), leases.copy())
self.assertIsNone(check_lease_changes())
def test_check_lease_changes_ignores_irrelevant_changes(self):
- leases = self.make_lease()
+ leases = factory.make_random_leases()
self.fake_leases_file(leases, age=10)
self.set_lease_state(datetime.utcnow(), leases.copy())
self.assertIsNone(check_lease_changes())
@@ -250,7 +247,7 @@
def test_update_leases_processes_leases_if_changed(self):
self.set_lease_state()
self.patch(leases_module, 'send_leases', FakeMethod())
- leases = self.make_lease()
+ leases = factory.make_random_leases()
self.fake_leases_file(leases)
self.patch(Omshell, 'create', FakeMethod())
update_leases()
@@ -261,7 +258,7 @@
def test_update_leases_does_nothing_without_lease_changes(self):
fake_send_leases = FakeMethod()
self.patch(leases_module, 'send_leases', fake_send_leases)
- leases = self.make_lease()
+ leases = factory.make_random_leases()
leases_file = self.fake_leases_file(leases)
self.set_lease_state(get_write_time(leases_file), leases.copy())
self.assertEqual([], leases_module.send_leases.calls)
@@ -269,9 +266,7 @@
def test_process_leases_records_update(self):
self.set_lease_state()
self.patch(leases_module, 'send_leases', FakeMethod())
- new_leases = {
- factory.getRandomIPAddress(): factory.getRandomMACAddress(),
- }
+ new_leases = factory.make_random_leases()
self.fake_leases_file(new_leases)
process_leases(datetime.utcnow(), new_leases)
self.assertIsNone(check_lease_changes())
@@ -282,9 +277,7 @@
self.fake_leases_file({})
self.patch(
leases_module, 'send_leases', FakeMethod(failure=StopExecuting()))
- new_leases = {
- factory.getRandomIPAddress(): factory.getRandomMACAddress(),
- }
+ new_leases = factory.make_random_leases()
try:
process_leases(datetime.utcnow(), new_leases)
except StopExecuting:
@@ -322,7 +315,7 @@
self.assertEqual([(ip, mac)], Omshell.create.extract_args())
def test_upload_leases_processes_leases_unconditionally(self):
- leases = self.make_lease()
+ leases = factory.make_random_leases()
leases_file = self.fake_leases_file(leases)
self.set_lease_state(get_write_time(leases_file), leases.copy())
self.patch(leases_module, 'send_leases', FakeMethod())
@@ -352,27 +345,18 @@
def test_identify_new_leases_identifies_everything_first_time(self):
self.patch(leases_module, 'recorded_leases', None)
- current_leases = {
- factory.getRandomIPAddress(): factory.getRandomMACAddress(),
- factory.getRandomIPAddress(): factory.getRandomMACAddress(),
- }
+ current_leases = factory.make_random_leases(2)
self.assertEqual(current_leases, identify_new_leases(current_leases))
def test_identify_new_leases_identifies_previously_unknown_leases(self):
self.patch(leases_module, 'recorded_leases', {})
- current_leases = {
- factory.getRandomIPAddress(): factory.getRandomMACAddress(),
- }
+ current_leases = factory.make_random_leases()
self.assertEqual(current_leases, identify_new_leases(current_leases))
def test_identify_new_leases_leaves_out_previously_known_leases(self):
- old_leases = {
- factory.getRandomIPAddress(): factory.getRandomMACAddress(),
- }
+ old_leases = factory.make_random_leases()
cache.set(LEASES_CACHE_KEY, old_leases)
- new_leases = {
- factory.getRandomIPAddress(): factory.getRandomMACAddress(),
- }
+ new_leases = factory.make_random_leases()
current_leases = old_leases.copy()
current_leases.update(new_leases)
self.assertEqual(new_leases, identify_new_leases(current_leases))
@@ -391,9 +375,7 @@
self.patch(Omshell, 'create', FakeMethod())
self.set_omapi_key()
self.set_nodegroup_name()
- old_leases = {
- factory.getRandomIPAddress(): factory.getRandomMACAddress(),
- }
+ old_leases = factory.make_random_leases()
self.set_lease_state(None, old_leases)
register_new_leases(old_leases)
self.assertEqual([], Omshell.create.calls)
@@ -403,9 +385,7 @@
self.set_lease_state()
self.set_items_needed_for_lease_update()
self.clear_omapi_key()
- new_leases = {
- factory.getRandomIPAddress(): factory.getRandomMACAddress(),
- }
+ new_leases = factory.make_random_leases()
register_new_leases(new_leases)
self.assertEqual([], Omshell.create.calls)
@@ -414,9 +394,7 @@
self.set_items_needed_for_lease_update()
nodegroup_name = self.set_nodegroup_name()
self.patch(MAASClient, 'post', FakeMethod())
- leases = {
- factory.getRandomIPAddress(): factory.getRandomMACAddress(),
- }
+ leases = factory.make_random_leases()
send_leases(leases)
self.assertEqual([(
('nodegroups/%s/' % nodegroup_name, 'update_leases'),
@@ -429,7 +407,7 @@
self.set_lease_state()
self.set_items_needed_for_lease_update()
self.clear_maas_url()
- leases = {factory.getRandomIPAddress(): factory.getRandomMACAddress()}
+ leases = factory.make_random_leases()
send_leases(leases)
self.assertEqual([], MAASClient.post.calls)
@@ -437,9 +415,7 @@
self.patch(MAASClient, 'post', FakeMethod())
self.set_items_needed_for_lease_update()
self.clear_api_credentials()
- leases = {
- factory.getRandomIPAddress(): factory.getRandomMACAddress(),
- }
+ leases = factory.make_random_leases()
send_leases(leases)
self.assertEqual([], MAASClient.post.calls)
@@ -448,6 +424,6 @@
self.set_lease_state()
self.set_items_needed_for_lease_update()
self.clear_nodegroup_name()
- leases = {factory.getRandomIPAddress(): factory.getRandomMACAddress()}
+ leases = factory.make_random_leases()
send_leases(leases)
self.assertEqual([], MAASClient.post.calls)
