From 3701438aa9240c4eb1a19a41047e2bff3d782561 Mon Sep 17 00:00:00 2001 From: Michael Hudson-Doyle Date: Wed, 29 May 2019 11:32:41 +1200 Subject: [PATCH] add a tonne of tests for when various device actions are permitted Fix a bug on ppc64el this found. --- .../controllers/tests/test_filesystem.py | 12 +- subiquity/models/filesystem.py | 2 +- subiquity/models/tests/test_filesystem.py | 344 +++++++++++++++++- 3 files changed, 346 insertions(+), 12 deletions(-) diff --git a/subiquity/controllers/tests/test_filesystem.py b/subiquity/controllers/tests/test_filesystem.py index b1b7d43a..09e1df76 100644 --- a/subiquity/controllers/tests/test_filesystem.py +++ b/subiquity/controllers/tests/test_filesystem.py @@ -24,6 +24,7 @@ from subiquity.models.tests.test_filesystem import ( ) from subiquity.models.filesystem import ( Bootloader, + DeviceAction, ) @@ -32,15 +33,15 @@ class Thing: pass -def make_controller_and_disk(): +def make_controller_and_disk(bootloader=None): common = defaultdict(type(None)) bm = Thing() - bm.filesystem, disk = make_model_and_disk() + bm.filesystem, disk = make_model_and_disk(bootloader) common['base_model'] = bm common['answers'] = {} opts = Thing() opts.dry_run = True - opts.bootloader = "UEFI" + opts.bootloader = None common['opts'] = opts controller = FilesystemController(common) return controller, disk @@ -65,10 +66,9 @@ class TestFilesystemController(unittest.TestCase): # This is really testing model code but it's much easier to test with a # controller around. for bl in Bootloader: - if bl == Bootloader.NONE: + controller, disk = make_controller_and_disk(bl) + if DeviceAction.MAKE_BOOT not in disk.supported_actions: continue - controller, disk = make_controller_and_disk() - controller.model.bootloader = bl controller.make_boot_disk(disk) self.assertFalse( disk._can_MAKE_BOOT, diff --git a/subiquity/models/filesystem.py b/subiquity/models/filesystem.py index a82de38c..8cd93cdd 100644 --- a/subiquity/models/filesystem.py +++ b/subiquity/models/filesystem.py @@ -620,7 +620,7 @@ class Disk(_Device): return False elif bl == Bootloader.PREP: install_dev = self._m.grub_install_device - if install_dev.device is self: + if install_dev is not None and install_dev.device is self: return False return self._fs is None and self._constructed_device is None diff --git a/subiquity/models/tests/test_filesystem.py b/subiquity/models/tests/test_filesystem.py index eeb85667..6503b6ac 100644 --- a/subiquity/models/tests/test_filesystem.py +++ b/subiquity/models/tests/test_filesystem.py @@ -17,7 +17,10 @@ from collections import namedtuple import unittest from subiquity.models.filesystem import ( + Bootloader, dehumanize_size, + DeviceAction, + Disk, FilesystemModel, humanize_size, ) @@ -106,12 +109,62 @@ FakeStorageInfo = namedtuple( FakeStorageInfo.__new__.__defaults__ = (None,) * len(FakeStorageInfo._fields) -def make_model_and_disk(): +def make_model(bootloader=None): model = FilesystemModel() - model._disk_info.append(FakeStorageInfo( - name='disk-name', size=100*(2**30), free=50*(2**30))) - model.reset() - return model, model._actions[0] + if bootloader is not None: + model.bootloader = bootloader + return model + + +def make_disk(model): + serial = 'serial%s' % len(model._actions) + model._actions.append(Disk( + m=model, serial=serial, + info=FakeStorageInfo(size=100*(2**30)))) + return model._actions[-1] + + +def make_model_and_disk(bootloader=None): + model = make_model(bootloader) + return model, make_disk(model) + + +def make_model_and_partition(bootloader=None): + model, disk = make_model_and_disk(bootloader) + return model, model.add_partition(disk, size=disk.free_for_partitions//2) + + +def make_raid(model): + name = 'md%s' % len(model._actions) + return model.add_raid( + name, 'raid1', {make_disk(model), make_disk(model)}, set()) + + +def make_model_and_raid(bootloader=None): + model = make_model(bootloader) + return model, make_raid(model) + + +def make_vg(model): + name = 'vg%s' % len(model._actions) + return model.add_volgroup( + name, {make_disk(model)}) + + +def make_model_and_vg(bootloader=None): + model = make_model(bootloader) + return model, make_vg(model) + + +def make_lv(model): + vg = make_vg(model) + name = 'lv%s' % len(model._actions) + return model.add_logical_volume(vg, name, vg.free_for_partitions//2) + + +def make_model_and_lv(bootloader=None): + model = make_model(bootloader) + return model, make_lv(model) class TestFilesystemModel(unittest.TestCase): @@ -126,3 +179,284 @@ class TestFilesystemModel(unittest.TestCase): dm_crypt = model.add_dm_crypt(disk, key='passw0rd') vg = model.add_volgroup('vg-0', {dm_crypt}) self.assertEqual(vg.annotations, ['encrypted']) + + def assertActionNotSupported(self, obj, action): + self.assertNotIn(action, obj.supported_actions) + + def assertActionPossible(self, obj, action): + self.assertIn(action, obj.supported_actions) + self.assertTrue(obj.action_possible(action)) + + def assertActionNotPossible(self, obj, action): + self.assertIn(action, obj.supported_actions) + outcome = obj.action_possible(action) + if isinstance(outcome, tuple): + outcome = outcome[0] + self.assertFalse(outcome) + + def _test_remove_action(self, model, objects): + self.assertActionNotPossible(objects[0], DeviceAction.REMOVE) + + vg = model.add_volgroup('vg1', {objects[0], objects[1]}) + self.assertActionPossible(objects[0], DeviceAction.REMOVE) + # Probably this removal should be a model method? + vg.devices.remove(objects[1]) + objects[1]._constructed_device = None + self.assertActionNotPossible(objects[0], DeviceAction.REMOVE) + raid = model.add_raid('md0', 'raid1', set(objects[2:]), set()) + self.assertActionPossible(objects[2], DeviceAction.REMOVE) + # Probably this removal should be a model method? + raid.devices.remove(objects[4]) + objects[4]._constructed_device = None + self.assertActionNotPossible(objects[2], DeviceAction.REMOVE) + + def test_disk_action_INFO(self): + model, disk = make_model_and_disk() + self.assertActionPossible(disk, DeviceAction.INFO) + + def test_disk_action_EDIT(self): + model, disk = make_model_and_disk() + self.assertActionNotSupported(disk, DeviceAction.EDIT) + + def test_disk_action_PARTITION(self): + model, disk = make_model_and_disk() + self.assertActionPossible(disk, DeviceAction.PARTITION) + model.add_partition(disk, size=disk.free_for_partitions//2) + self.assertActionPossible(disk, DeviceAction.PARTITION) + model.add_partition(disk, size=disk.free_for_partitions) + self.assertActionNotPossible(disk, DeviceAction.PARTITION) + + def test_disk_action_CREATE_LV(self): + model, disk = make_model_and_disk() + self.assertActionNotSupported(disk, DeviceAction.CREATE_LV) + + def test_disk_action_FORMAT(self): + model, disk = make_model_and_disk() + self.assertActionPossible(disk, DeviceAction.FORMAT) + model.add_partition(disk, size=disk.free_for_partitions//2) + self.assertActionNotPossible(disk, DeviceAction.FORMAT) + disk2 = make_disk(model) + model.add_volgroup('vg1', {disk2}) + self.assertActionNotPossible(disk2, DeviceAction.FORMAT) + + def test_disk_action_REMOVE(self): + model = make_model() + disks = [make_disk(model) for i in range(5)] + self._test_remove_action(model, disks) + + def test_disk_action_DELETE(self): + model, disk = make_model_and_disk() + self.assertActionNotSupported(disk, DeviceAction.DELETE) + + def test_disk_action_MAKE_BOOT(self): + for bl in Bootloader: + model, disk = make_model_and_disk(bl) + if bl == Bootloader.NONE: + self.assertActionNotSupported(disk, DeviceAction.MAKE_BOOT) + else: + self.assertActionPossible(disk, DeviceAction.MAKE_BOOT) + + def test_partition_action_INFO(self): + model, part = make_model_and_partition() + self.assertActionNotSupported(part, DeviceAction.INFO) + + def test_partition_action_EDIT(self): + model, part = make_model_and_partition() + self.assertActionPossible(part, DeviceAction.EDIT) + model.add_volgroup('vg1', {part}) + self.assertActionNotPossible(part, DeviceAction.EDIT) + + def test_partition_action_PARTITION(self): + model, part = make_model_and_partition() + self.assertActionNotSupported(part, DeviceAction.PARTITION) + + def test_partition_action_CREATE_LV(self): + model, part = make_model_and_partition() + self.assertActionNotSupported(part, DeviceAction.CREATE_LV) + + def test_partition_action_FORMAT(self): + model, part = make_model_and_partition() + self.assertActionNotSupported(part, DeviceAction.FORMAT) + + def test_partition_action_REMOVE(self): + model = make_model() + model, disk = make_model_and_disk() + parts = [] + for i in range(5): + parts.append(model.add_partition( + disk, size=disk.free_for_partitions//5)) + self._test_remove_action(model, parts) + + def test_partition_action_DELETE(self): + model, disk = make_model_and_disk() + part1 = model.add_partition(disk, size=disk.free_for_partitions//2) + self.assertActionPossible(part1, DeviceAction.DELETE) + fs = model.add_filesystem(part1, 'ext4') + self.assertActionPossible(part1, DeviceAction.DELETE) + model.add_mount(fs, '/') + self.assertActionPossible(part1, DeviceAction.DELETE) + + part2 = model.add_partition(disk, size=disk.free_for_partitions//2) + model.add_volgroup('vg1', {part2}) + self.assertActionNotPossible(part2, DeviceAction.DELETE) + + for flag in 'bios_grub', 'boot', 'prep': + # Possibly we should change this to only prevent the + # deletion of a partition with a flag that matters to the + # current bootloader. + part = model.add_partition( + disk, size=disk.free_for_partitions//2, flag=flag) + self.assertActionNotPossible(part, DeviceAction.DELETE) + + def test_partition_action_MAKE_BOOT(self): + model, part = make_model_and_partition() + self.assertActionNotSupported(part, DeviceAction.MAKE_BOOT) + + def test_raid_action_INFO(self): + model, raid = make_model_and_raid() + self.assertActionNotSupported(raid, DeviceAction.INFO) + + def test_raid_action_EDIT(self): + model = make_model() + raid1 = make_raid(model) + self.assertActionPossible(raid1, DeviceAction.EDIT) + model.add_volgroup('vg1', {raid1}) + self.assertActionNotPossible(raid1, DeviceAction.EDIT) + raid2 = make_raid(model) + model.add_partition(raid2, size=raid2.free_for_partitions//2) + self.assertActionNotPossible(raid2, DeviceAction.EDIT) + + def test_raid_action_PARTITION(self): + model, raid = make_model_and_raid() + self.assertActionPossible(raid, DeviceAction.PARTITION) + model.add_partition(raid, size=raid.free_for_partitions//2) + self.assertActionPossible(raid, DeviceAction.PARTITION) + model.add_partition(raid, size=raid.free_for_partitions) + self.assertActionNotPossible(raid, DeviceAction.PARTITION) + + def test_raid_action_CREATE_LV(self): + model, raid = make_model_and_raid() + self.assertActionNotSupported(raid, DeviceAction.CREATE_LV) + + def test_raid_action_FORMAT(self): + model, raid = make_model_and_raid() + self.assertActionPossible(raid, DeviceAction.FORMAT) + model.add_partition(raid, size=raid.free_for_partitions//2) + self.assertActionNotPossible(raid, DeviceAction.FORMAT) + raid2 = make_raid(model) + model.add_volgroup('vg1', {raid2}) + self.assertActionNotPossible(raid2, DeviceAction.FORMAT) + + def test_raid_action_REMOVE(self): + model = make_model() + raids = [make_raid(model) for i in range(5)] + self._test_remove_action(model, raids) + + def test_raid_action_DELETE(self): + model, raid = make_model_and_raid() + + raid1 = make_raid(model) + self.assertActionPossible(raid1, DeviceAction.DELETE) + part = model.add_partition(raid1, size=raid.free_for_partitions//2) + self.assertActionPossible(raid1, DeviceAction.DELETE) + fs = model.add_filesystem(part, 'ext4') + self.assertActionPossible(raid1, DeviceAction.DELETE) + model.add_mount(fs, '/') + self.assertActionNotPossible(raid1, DeviceAction.DELETE) + + raid2 = make_raid(model) + self.assertActionPossible(raid2, DeviceAction.DELETE) + fs = model.add_filesystem(raid2, 'ext4') + self.assertActionPossible(raid2, DeviceAction.DELETE) + model.add_mount(fs, '/') + self.assertActionPossible(raid2, DeviceAction.DELETE) + + raid2 = make_raid(model) + model.add_volgroup('vg0', {raid2}) + self.assertActionNotPossible(raid2, DeviceAction.DELETE) + + def test_raid_action_MAKE_BOOT(self): + model, raid = make_model_and_raid() + self.assertActionNotSupported(raid, DeviceAction.MAKE_BOOT) + + def test_vg_action_INFO(self): + model, vg = make_model_and_vg() + self.assertActionNotSupported(vg, DeviceAction.INFO) + + def test_vg_action_EDIT(self): + model, vg = make_model_and_vg() + self.assertActionPossible(vg, DeviceAction.EDIT) + model.add_partition(vg, size=vg.free_for_partitions//2) + self.assertActionNotPossible(vg, DeviceAction.EDIT) + + def test_vg_action_PARTITION(self): + model, vg = make_model_and_vg() + self.assertActionNotSupported(vg, DeviceAction.PARTITION) + + def test_vg_action_CREATE_LV(self): + model, vg = make_model_and_vg() + self.assertActionPossible(vg, DeviceAction.CREATE_LV) + model.add_logical_volume(vg, 'lv1', size=vg.free_for_partitions//2) + self.assertActionPossible(vg, DeviceAction.CREATE_LV) + model.add_partition(vg, size=vg.free_for_partitions) + self.assertActionNotPossible(vg, DeviceAction.CREATE_LV) + + def test_vg_action_FORMAT(self): + model, vg = make_model_and_vg() + self.assertActionNotSupported(vg, DeviceAction.FORMAT) + + def test_vg_action_REMOVE(self): + model, vg = make_model_and_vg() + self.assertActionNotSupported(vg, DeviceAction.REMOVE) + + def test_vg_action_DELETE(self): + model, vg = make_model_and_vg() + self.assertActionPossible(vg, DeviceAction.DELETE) + self.assertActionPossible(vg, DeviceAction.DELETE) + lv = model.add_logical_volume( + vg, 'lv0', size=vg.free_for_partitions//2) + self.assertActionPossible(vg, DeviceAction.DELETE) + fs = model.add_filesystem(lv, 'ext4') + self.assertActionPossible(vg, DeviceAction.DELETE) + model.add_mount(fs, '/') + self.assertActionNotPossible(vg, DeviceAction.DELETE) + + def test_vg_action_MAKE_BOOT(self): + model, vg = make_model_and_vg() + self.assertActionNotSupported(vg, DeviceAction.MAKE_BOOT) + + def test_lv_action_INFO(self): + model, lv = make_model_and_lv() + self.assertActionNotSupported(lv, DeviceAction.INFO) + + def test_lv_action_EDIT(self): + model, lv = make_model_and_lv() + self.assertActionPossible(lv, DeviceAction.EDIT) + + def test_lv_action_PARTITION(self): + model, lv = make_model_and_lv() + self.assertActionNotSupported(lv, DeviceAction.PARTITION) + + def test_lv_action_CREATE_LV(self): + model, lv = make_model_and_lv() + self.assertActionNotSupported(lv, DeviceAction.CREATE_LV) + + def test_lv_action_FORMAT(self): + model, lv = make_model_and_lv() + self.assertActionNotSupported(lv, DeviceAction.FORMAT) + + def test_lv_action_REMOVE(self): + model, lv = make_model_and_lv() + self.assertActionNotSupported(lv, DeviceAction.REMOVE) + + def test_lv_action_DELETE(self): + model, lv = make_model_and_lv() + self.assertActionPossible(lv, DeviceAction.DELETE) + fs = model.add_filesystem(lv, 'ext4') + self.assertActionPossible(lv, DeviceAction.DELETE) + model.add_mount(fs, '/') + self.assertActionPossible(lv, DeviceAction.DELETE) + + def test_lv_action_MAKE_BOOT(self): + model, lv = make_model_and_lv() + self.assertActionNotSupported(lv, DeviceAction.MAKE_BOOT)