From b67bd60cdd00564891dafb9576a379a634281a27 Mon Sep 17 00:00:00 2001 From: Alexander Gordeev Date: Wed, 25 May 2016 21:12:46 +0300 Subject: [PATCH] Improve test coverage 37 unit tests were skipped. This patch dramatically decreases this number to just 3. Change-Id: Ia90e18cf08df3db24fbaba2fbe29e3e8570c147a --- bareon/drivers/data/ironic.py | 24 +-------------- bareon/drivers/data/ks_spaces_validator.py | 8 ++--- bareon/drivers/data/nailgun.py | 6 ++-- bareon/drivers/deploy/nailgun.py | 13 +++++++-- bareon/tests/test_build_utils.py | 6 ---- bareon/tests/test_ironic_data_driver.py | 23 ++++----------- .../tests/test_ironic_ks_spaces_validator.py | 1 - bareon/tests/test_manager.py | 29 +++++++++---------- bareon/tests/test_nailgun.py | 4 --- bareon/tests/test_utils.py | 13 +++++++++ bareon/utils/utils.py | 22 ++++++++++++++ 11 files changed, 73 insertions(+), 76 deletions(-) diff --git a/bareon/drivers/data/ironic.py b/bareon/drivers/data/ironic.py index 763fe44..ffd8a5c 100644 --- a/bareon/drivers/data/ironic.py +++ b/bareon/drivers/data/ironic.py @@ -726,29 +726,7 @@ def convert_string_sizes(data): any(x in v for x in ('%', 'remaining'))): continue if k in ('size', 'lvm_meta_size'): - data[k] = human2bytes(v) + data[k] = utils.human2bytes(v) else: data[k] = convert_string_sizes(v) return data - - -def human2bytes(value, default='MiB', target='MiB'): - symbols = {'custom': ('B', 'KB', 'MB', 'GB', 'TB', 'PB', 'EB', 'ZB', 'YB'), - 'iec': ('KiB', 'MiB', 'GiB', 'TiB', 'PiB', 'EiB', 'ZiB', 'YiB')} - bytes = {} - bytes.update({e: 1000.0 ** n for n, e in enumerate(symbols['custom'])}) - bytes.update({e: 1024.0 ** n for n, e in enumerate(symbols['iec'], 1)}) - try: - number = '' - prefix = default - for index, letter in enumerate(value): - if letter and letter.isdigit() or letter == '.': - number += letter - else: - if value[index] == ' ': - index += 1 - prefix = value[index:] - break - return int(float(number) * bytes[prefix] / bytes[target]) - except Exception as ex: - raise ValueError('Can\'t convert size %s. Error: %s' % (value, ex)) diff --git a/bareon/drivers/data/ks_spaces_validator.py b/bareon/drivers/data/ks_spaces_validator.py index 2ae8a31..f261a7d 100644 --- a/bareon/drivers/data/ks_spaces_validator.py +++ b/bareon/drivers/data/ks_spaces_validator.py @@ -47,10 +47,10 @@ def validate(data, schema_file='nailgun'): # TODO(lobur): Must be done after unit conversion # for space in data: - # for volume in space.get('volumes', []): - # if volume['size'] > 16777216 and volume.get('mount') == '/': - # raise errors.WrongPartitionSchemeError( - # 'Root file system must be less than 16T') + # for volume in space.get('volumes', []): + # if volume['size'] > 16777216 and volume.get('mount') == '/': + # raise errors.WrongPartitionSchemeError( + # 'Root file system must be less than 16T') # TODO(kozhukalov): need to have additional logical verifications # maybe sizes and format of string values diff --git a/bareon/drivers/data/nailgun.py b/bareon/drivers/data/nailgun.py index 20474f7..caef472 100644 --- a/bareon/drivers/data/nailgun.py +++ b/bareon/drivers/data/nailgun.py @@ -454,9 +454,9 @@ class Nailgun(BaseDataDriver, # TODO(lobur): port https://review.openstack.org/#/c/261562/ to fix # # checking if /boot is created - # if not self._boot_partition_done or not self._boot_done: - # raise errors.WrongPartitionSchemeError( - # '/boot partition has not been created for some reasons') + if not self._boot_partition_done or not self._boot_done: + raise errors.WrongPartitionSchemeError( + '/boot partition has not been created for some reasons') LOG.debug('Looping over all volume groups in provision data') for vg in self.ks_vgs: diff --git a/bareon/drivers/deploy/nailgun.py b/bareon/drivers/deploy/nailgun.py index cd9b029..d27350e 100644 --- a/bareon/drivers/deploy/nailgun.py +++ b/bareon/drivers/deploy/nailgun.py @@ -115,8 +115,17 @@ opts = [ ), ] +cli_opts = [ + cfg.StrOpt( + 'image_build_dir', + default='/tmp', + help='Directory where the image is supposed to be built', + ), +] + CONF = cfg.CONF CONF.register_opts(opts) +CONF.register_cli_opts(cli_opts) LOG = logging.getLogger(__name__) @@ -562,9 +571,9 @@ class Manager(BaseDeployDriver): return LOG.debug('At least one of the necessary images is unavailable. ' 'Starting build process.') + chroot = bu.mkdtemp_smart( + CONF.image_build_dir, CONF.image_build_suffix) try: - chroot = bu.mkdtemp_smart( - CONF.image_build_dir, CONF.image_build_suffix) self.install_base_os(chroot) packages = self.driver.operating_system.packages metadata['packages'] = packages diff --git a/bareon/tests/test_build_utils.py b/bareon/tests/test_build_utils.py index e8d5155..fd960a7 100644 --- a/bareon/tests/test_build_utils.py +++ b/bareon/tests/test_build_utils.py @@ -153,7 +153,6 @@ class BuildUtilsTestCase(unittest2.TestCase): self.assertEqual([mock.call('chroot', f) for f in files], mock_path.join.call_args_list) - @unittest2.skip("Fix after cray rebase") @mock.patch.object(bu, 'remove_files') @mock.patch.object(bu, 'clean_dirs') def test_clean_apt_settings(self, mock_dirs, mock_files): @@ -170,11 +169,6 @@ class BuildUtilsTestCase(unittest2.TestCase): self.assertEqual('chroot', mock_files.call_args[0][0]) self.assertEqual(files, set(mock_files.call_args[0][1])) - mock_files.assert_called_once_with( - 'chroot', ['etc/apt/sources.list', 'etc/apt/preferences', - 'etc/apt/apt.conf.d/%s' % 'force_ipv4', - 'etc/apt/apt.conf.d/%s' % 'unsigned']) - @mock.patch('bareon.utils.build.open', create=True, new_callable=mock.mock_open) @mock.patch('bareon.utils.build.os.path') diff --git a/bareon/tests/test_ironic_data_driver.py b/bareon/tests/test_ironic_data_driver.py index 5a5c8e4..3b09341 100644 --- a/bareon/tests/test_ironic_data_driver.py +++ b/bareon/tests/test_ironic_data_driver.py @@ -358,7 +358,7 @@ class TestFindHwFstab(unittest2.TestCase): class TestConvertStringSize(unittest2.TestCase): - @mock.patch.object(ironic, 'human2bytes') + @mock.patch.object(ironic.utils, 'human2bytes') def test_success_single_disk(self, mock_converter): data = {'image_deploy_flags': {'rsync_flags': '-a -A -X'}, 'partitions': [{'extra': [], @@ -378,7 +378,7 @@ class TestConvertStringSize(unittest2.TestCase): [mock.call('10000 MB'), mock.call('5 GB'), mock.call('4000')], any_order=True) - @mock.patch.object(ironic, 'human2bytes') + @mock.patch.object(ironic.utils, 'human2bytes') def test_success_two_disks(self, mock_converter): data = {'image_deploy_flags': {'rsync_flags': '-a -A -X'}, 'partitions': [{'extra': [], @@ -406,7 +406,7 @@ class TestConvertStringSize(unittest2.TestCase): [mock.call('10000 MB'), mock.call('5 GB'), mock.call('4000'), mock.call('2000 MB'), mock.call('2 GB')], any_order=True) - @mock.patch.object(ironic, 'human2bytes') + @mock.patch.object(ironic.utils, 'human2bytes') def test_success_lvm_meta_size(self, mock_converter): data = {'image_deploy_flags': {'rsync_flags': '-a -A -X'}, 'partitions': [{'extra': [], @@ -427,7 +427,7 @@ class TestConvertStringSize(unittest2.TestCase): [mock.call('10000 MB'), mock.call('5 GB'), mock.call('4 GB'), mock.call('64')], any_order=True) - @mock.patch.object(ironic, 'human2bytes') + @mock.patch.object(ironic.utils, 'human2bytes') def test_success_ignore_percent(self, mock_converter): data = {'image_deploy_flags': {'rsync_flags': '-a -A -X'}, 'partitions': [{'extra': [], @@ -447,7 +447,7 @@ class TestConvertStringSize(unittest2.TestCase): [mock.call('10000 MB'), mock.call('4000')], any_order=True) - @mock.patch.object(ironic, 'human2bytes') + @mock.patch.object(ironic.utils, 'human2bytes') def test_success_ignore_remaining(self, mock_converter): data = {'image_deploy_flags': {'rsync_flags': '-a -A -X'}, 'partitions': [{'extra': [], @@ -468,19 +468,6 @@ class TestConvertStringSize(unittest2.TestCase): any_order=True) -class TestHumantoBytesConverter(unittest2.TestCase): - def test_default_convertion(self): - result = ironic.human2bytes('1000', default='GiB') - self.assertEqual(result, 1024000) - - def test_target_convertion(self): - result = ironic.human2bytes('1024 MiB', target='GiB') - self.assertEqual(result, 1) - - def test_invalid_data(self): - self.assertRaises(ValueError, ironic.human2bytes, 'invalid data') - - class TestConvertPercentSizes(unittest2.TestCase): GRUB = ironic.DEFAULT_GRUB_SIZE LVM = ironic.DEFAULT_LVM_META_SIZE diff --git a/bareon/tests/test_ironic_ks_spaces_validator.py b/bareon/tests/test_ironic_ks_spaces_validator.py index 9491867..d18c81a 100644 --- a/bareon/tests/test_ironic_ks_spaces_validator.py +++ b/bareon/tests/test_ironic_ks_spaces_validator.py @@ -192,7 +192,6 @@ class TestKSSpacesValidator(unittest2.TestCase): self.assertRaises(errors.WrongPartitionSchemeError, kssv.validate, self.fake_scheme[-2:], 'ironic') - @unittest2.skip("Fix after cray rebase") def test_validate_16T_root_volume_fail(self): self.fake_scheme[3]['volumes'][0]['size'] = 16777216 + 1 self.assertRaises(errors.WrongPartitionSchemeError, diff --git a/bareon/tests/test_manager.py b/bareon/tests/test_manager.py index 8547637..a32c9e0 100644 --- a/bareon/tests/test_manager.py +++ b/bareon/tests/test_manager.py @@ -19,8 +19,8 @@ from oslo_config import cfg import six import unittest2 -import bareon from bareon.drivers.data import nailgun as nailgun_data +from bareon.drivers.deploy import nailgun as nailgun_deploy from bareon import objects from bareon.utils import utils @@ -32,7 +32,6 @@ elif six.PY3: CONF = cfg.CONF -@unittest2.skip("Fix after cray rebase") class TestImageBuild(unittest2.TestCase): @mock.patch('yaml.load') @@ -62,26 +61,26 @@ class TestImageBuild(unittest2.TestCase): ], "codename": "trusty" } - self.mgr = bareon.drivers.deploy.nailgun(image_conf) + self.mgr = nailgun_deploy.Manager( + nailgun_data.NailgunBuildImage(image_conf)) - @mock.patch.object(bareon.drivers.deploy.nailgun, '_set_apt_repos') - @mock.patch('bareon.drivers.deploy.nailgun.bu', autospec=True) - @mock.patch('bareon.drivers.deploy.nailgun.fu', autospec=True) - @mock.patch('bareon.drivers.deploy.nailgun.utils', autospec=True) - @mock.patch('bareon.drivers.deploy.nailgun.os', autospec=True) - @mock.patch('bareon.drivers.deploy.nailgun.shutil.move') - @mock.patch('bareon.drivers.deploy.nailgun.open', - create=True, new_callable=mock.mock_open) - @mock.patch('bareon.drivers.deploy.nailgun.yaml.safe_dump') - @mock.patch.object(bareon.drivers.deploy.nailgun, 'mount_target') - @mock.patch.object(bareon.drivers.deploy.nailgun, 'umount_target') + @mock.patch.object(nailgun_deploy.Manager, '_set_apt_repos') + @mock.patch.object(nailgun_deploy, 'bu', autospec=True) + @mock.patch.object(nailgun_deploy, 'fu', autospec=True) + @mock.patch.object(nailgun_deploy, 'utils', autospec=True) + @mock.patch.object(nailgun_deploy, 'os', autospec=True) + @mock.patch.object(nailgun_deploy.shutil, 'move') + @mock.patch.object(nailgun_deploy, 'open', create=True, + new_callable=mock.mock_open) + @mock.patch.object(nailgun_deploy.yaml, 'safe_dump') + @mock.patch.object(nailgun_deploy.Manager, 'mount_target') + @mock.patch.object(nailgun_deploy.Manager, 'umount_target') def test_do_build_image(self, mock_umount_target, mock_mount_target, mock_yaml_dump, mock_open, mock_shutil_move, mock_os, mock_utils, mock_fu, mock_bu, mock_set_apt_repos): loops = [objects.Loop(), objects.Loop()] - self.mgr.driver._image_scheme = objects.ImageScheme([ objects.Image('file:///fake/img.img.gz', loops[0], 'ext4', 'gzip'), objects.Image('file:///fake/img-boot.img.gz', diff --git a/bareon/tests/test_nailgun.py b/bareon/tests/test_nailgun.py index be6ceec..e1a807d 100644 --- a/bareon/tests/test_nailgun.py +++ b/bareon/tests/test_nailgun.py @@ -790,7 +790,6 @@ SINGLE_NVME_DISK_KS_SPACES = [ ] -@unittest2.skip("Fix after cray rebase") class TestNailgunMatch(unittest2.TestCase): def test_match_device_by_id_matches(self): # matches by 'by-id' links @@ -904,7 +903,6 @@ class TestNailgunMatch(unittest2.TestCase): self.assertFalse(nailgun.match_device(fake_hu_disk, fake_ks_disk)) -@unittest2.skip @mock.patch.object(nailgun.Nailgun, '__init__', return_value=None) class TestNailgunGetOSMethods(unittest2.TestCase): def test_parse_operating_system_test_profiles(self, mock_nailgun): @@ -940,7 +938,6 @@ class TestNailgunGetOSMethods(unittest2.TestCase): self.assertEqual('unknown', os_name) -@unittest2.skip("Fix after cray rebase") @mock.patch.object(nailgun.Nailgun, 'parse_image_meta', return_value={}) @mock.patch('bareon.drivers.data.nailgun.hu.list_block_devices') class TestNailgunMockedMeta(unittest2.TestCase): @@ -1289,7 +1286,6 @@ class TestNailgunMockedMeta(unittest2.TestCase): self.assertEqual('default', drv.partition_scheme.mds[0].metadata) -@unittest2.skip("Fix after cray rebase") @mock.patch.object(utils, 'init_http_request') @mock.patch('bareon.drivers.data.nailgun.hu.list_block_devices') class TestNailgunImageMeta(unittest2.TestCase): diff --git a/bareon/tests/test_utils.py b/bareon/tests/test_utils.py index 4207f3e..834dd42 100644 --- a/bareon/tests/test_utils.py +++ b/bareon/tests/test_utils.py @@ -486,6 +486,19 @@ class GetIPTestCase(unittest2.TestCase): mock_execute.assert_called_once_with(*self.cmd) +class TestHumantoBytesConverter(unittest2.TestCase): + def test_default_convertion(self): + result = utils.human2bytes('1000', default='GiB') + self.assertEqual(result, 1024000) + + def test_target_convertion(self): + result = utils.human2bytes('1024 MiB', target='GiB') + self.assertEqual(result, 1) + + def test_invalid_data(self): + self.assertRaises(ValueError, utils.human2bytes, 'invalid data') + + class ParseKernelCmdline(unittest2.TestCase): def test_parse_kernel_cmdline(self): diff --git a/bareon/utils/utils.py b/bareon/utils/utils.py index 3804cfb..fc3dddc 100644 --- a/bareon/utils/utils.py +++ b/bareon/utils/utils.py @@ -402,6 +402,28 @@ def text_diff(text1, text2, sfrom="from", sto="to"): return "\n".join(diff) +def human2bytes(value, default='MiB', target='MiB'): + symbols = {'custom': ('B', 'KB', 'MB', 'GB', 'TB', 'PB', 'EB', 'ZB', 'YB'), + 'iec': ('KiB', 'MiB', 'GiB', 'TiB', 'PiB', 'EiB', 'ZiB', 'YiB')} + bytes = {} + bytes.update({e: 1000.0 ** n for n, e in enumerate(symbols['custom'])}) + bytes.update({e: 1024.0 ** n for n, e in enumerate(symbols['iec'], 1)}) + try: + number = '' + prefix = default + for index, letter in enumerate(value): + if letter and letter.isdigit() or letter == '.': + number += letter + else: + if value[index] == ' ': + index += 1 + prefix = value[index:] + break + return int(float(number) * bytes[prefix] / bytes[target]) + except Exception as ex: + raise ValueError('Can\'t convert size %s. Error: %s' % (value, ex)) + + def list_opts(): """Returns a list of oslo.config options available in the library.