From 49f7ce8cf4efafa6f2340f28470f504d048c6b31 Mon Sep 17 00:00:00 2001 From: Eric Harney Date: Fri, 12 Jun 2015 15:47:30 -0400 Subject: [PATCH] Targets tests: Clean up long lines Clean up long lines in unit tests, mostly by using a variable for volume UUID and shortening unnecessarily long CHAP credentials. This is not intended to be complete, just a good start that greatly reduces #noqa and improves readability. Change-Id: Iddc26a5722731d76e6d9e4b0ff493ed806bf148d --- cinder/tests/unit/targets/targets_fixture.py | 5 +- cinder/tests/unit/targets/test_cxt_driver.py | 31 +++++--- cinder/tests/unit/targets/test_lio_driver.py | 15 ++-- cinder/tests/unit/targets/test_tgt_driver.py | 80 ++++++++------------ 4 files changed, 59 insertions(+), 72 deletions(-) diff --git a/cinder/tests/unit/targets/targets_fixture.py b/cinder/tests/unit/targets/targets_fixture.py index 902dd6dbe..c67974020 100644 --- a/cinder/tests/unit/targets/targets_fixture.py +++ b/cinder/tests/unit/targets/targets_fixture.py @@ -91,9 +91,10 @@ class TargetDriverFixture(test.TestCase): 'target_portal': '10.10.7.1:3260', 'volume_id': self.fake_volume_id} - self.volume_name = 'volume-83c2e877-feed-46be-8435-77884fe55b45' + self.VOLUME_ID = '83c2e877-feed-46be-8435-77884fe55b45' + self.VOLUME_NAME = 'volume-' + self.VOLUME_ID self.test_vol = (self.iscsi_target_prefix + - self.volume_name) + self.VOLUME_NAME) def _cleanup(self): if os.path.exists(self.fake_volumes_dir): diff --git a/cinder/tests/unit/targets/test_cxt_driver.py b/cinder/tests/unit/targets/test_cxt_driver.py index 9134fc3fe..9a844ec3e 100644 --- a/cinder/tests/unit/targets/test_cxt_driver.py +++ b/cinder/tests/unit/targets/test_cxt_driver.py @@ -31,38 +31,45 @@ class TestCxtAdmDriver(tf.TargetDriverFixture): self.cxt_subdir = cxt.CxtAdm.cxt_subdir self.target = cxt.CxtAdm(root_helper=utils.get_root_helper(), configuration=self.configuration) - self.fake_iscsi_scan =\ + self.VG = 'stack-volumes-lvmdriver-1' + self.fake_iscsi_scan = \ ('\n' - 'TARGET: iqn.2010-10.org.openstack:%s, id=1, login_ip=0\n' # noqa + 'TARGET: iqn.2010-10.org.openstack:%(vol)s, id=1, login_ip=0\n' ' PortalGroup=1@10.9.8.7:3260,timeout=0\n' - ' TargetDevice=/dev/stack-volumes-lvmdriver-1/%s,BLK,PROD=CHISCSI Target,SN=0N0743000000000,ID=0D074300000000000000000,WWN=:W00743000000000\n' # noqa - % (self.volume_name, self.volume_name)) + ' TargetDevice=/dev/%(vg)s/%(vol)s' + ',BLK,PROD=CHISCSI ' + 'Target,SN=0N0743000000000,ID=0D074300000000000000000,' + 'WWN=:W00743000000000\n' + % {'vol': self.VOLUME_NAME, 'vg': self.VG}) def test_get_target(self): with mock.patch.object(self.target, '_get_volumes_dir', return_value=self.fake_volumes_dir),\ mock.patch('cinder.utils.execute', return_value=(self.fake_iscsi_scan, None)) as m_exec: - self.assertEqual('1', - self.target._get_target( - 'iqn.2010-10.org.openstack:volume-83c2e877-feed-46be-8435-77884fe55b45' # noqa - )) + self.assertEqual( + '1', + self.target._get_target( + 'iqn.2010-10.org.openstack:volume-%s' % self.VOLUME_ID + ) + ) self.assertTrue(m_exec.called) def test_get_target_chap_auth(self): tmp_file = StringIO.StringIO() tmp_file.write( 'target:\n' - ' TargetName=iqn.2010-10.org.openstack:volume-83c2e877-feed-46be-8435-77884fe55b45\n' # noqa - ' TargetDevice=/dev/stack-volumes-lvmdriver-1/volume-83c2e877-feed-46be-8435-77884fe55b45\n' # noqa + ' TargetName=iqn.2010-10.org.openstack:volume-%(id)s\n' + ' TargetDevice=/dev/%(vg)s/volume-%(id)s\n' ' PortalGroup=1@10.9.8.7:3260\n' ' AuthMethod=CHAP\n' ' Auth_CHAP_Policy=Oneway\n' - ' Auth_CHAP_Initiator="otzLy2UYbYfnP4zXLG5z":"234Zweo38VGBBvrpK9nt"\n' # noqa + ' Auth_CHAP_Initiator="otzL":"234Z"\n' % + {'id': self.VOLUME_ID, 'vg': self.VG} ) tmp_file.seek(0) - expected = ('otzLy2UYbYfnP4zXLG5z', '234Zweo38VGBBvrpK9nt') + expected = ('otzL', '234Z') with mock.patch('__builtin__.open') as mock_open: ctx = context.get_admin_context() mock_open.return_value = contextlib.closing(tmp_file) diff --git a/cinder/tests/unit/targets/test_lio_driver.py b/cinder/tests/unit/targets/test_lio_driver.py index 79213cb31..dd01fd9a6 100644 --- a/cinder/tests/unit/targets/test_lio_driver.py +++ b/cinder/tests/unit/targets/test_lio_driver.py @@ -79,7 +79,7 @@ class TestLioAdmDriver(tf.TargetDriverFixture): 1, 0, self.fake_volumes_dir)) - mpersist_cfg.assert_called_once_with(self.volume_name) + mpersist_cfg.assert_called_once_with(self.VOLUME_NAME) mexecute.assert_called_once_with( 'cinder-rtstool', 'create', @@ -96,15 +96,13 @@ class TestLioAdmDriver(tf.TargetDriverFixture): @mock.patch.object(lio.LioAdm, '_get_target', return_value=1) def test_create_iscsi_target_port_ip(self, mget_target, mexecute, mpersist_cfg, mlock_exec): - test_vol = 'iqn.2010-10.org.openstack:'\ - 'volume-83c2e877-feed-46be-8435-77884fe55b45' ip = '10.0.0.15' port = 3261 self.assertEqual( 1, self.target.create_iscsi_target( - name=test_vol, + name=self.test_vol, tid=1, lun=0, path=self.fake_volumes_dir, @@ -114,7 +112,7 @@ class TestLioAdmDriver(tf.TargetDriverFixture): 'cinder-rtstool', 'create', self.fake_volumes_dir, - test_vol, + self.test_vol, '', '', self.target.iscsi_protocol == 'iser', @@ -123,7 +121,7 @@ class TestLioAdmDriver(tf.TargetDriverFixture): mlock_exec.assert_any_call(*expected_args, run_as_root=True) mexecute.assert_any_call(*expected_args, run_as_root=True) - mpersist_cfg.assert_called_once_with(self.volume_name) + mpersist_cfg.assert_called_once_with(self.VOLUME_NAME) @mock.patch.object(lio.LioAdm, '_execute', side_effect=lio.LioAdm._execute) @mock.patch.object(lio.LioAdm, '_persist_configuration') @@ -131,8 +129,7 @@ class TestLioAdmDriver(tf.TargetDriverFixture): @mock.patch.object(lio.LioAdm, '_get_target', return_value=1) def test_create_iscsi_target_port_ips(self, mget_target, mexecute, mpersist_cfg, mlock_exec): - test_vol = 'iqn.2010-10.org.openstack:'\ - 'volume-83c2e877-feed-46be-8435-77884fe55b45' + test_vol = 'iqn.2010-10.org.openstack:' + self.VOLUME_NAME ips = ['10.0.0.15', '127.0.0.1'] port = 3261 @@ -158,7 +155,7 @@ class TestLioAdmDriver(tf.TargetDriverFixture): mlock_exec.assert_any_call(*expected_args, run_as_root=True) mexecute.assert_any_call(*expected_args, run_as_root=True) - mpersist_cfg.assert_called_once_with(self.volume_name) + mpersist_cfg.assert_called_once_with(self.VOLUME_NAME) @mock.patch.object(lio.LioAdm, '_execute', side_effect=lio.LioAdm._execute) @mock.patch.object(lio.LioAdm, '_persist_configuration') diff --git a/cinder/tests/unit/targets/test_tgt_driver.py b/cinder/tests/unit/targets/test_tgt_driver.py index 83ed96097..e9455af85 100644 --- a/cinder/tests/unit/targets/test_tgt_driver.py +++ b/cinder/tests/unit/targets/test_tgt_driver.py @@ -31,11 +31,10 @@ class TestTgtAdmDriver(tf.TargetDriverFixture): self.target = tgt.TgtAdm(root_helper=utils.get_root_helper(), configuration=self.configuration) self.testvol_path = \ - '/dev/stack-volumes-lvmdriver-1/'\ - 'volume-83c2e877-feed-46be-8435-77884fe55b45' + '/dev/stack-volumes-lvmdriver-1/%s' % self.VOLUME_NAME self.fake_iscsi_scan =\ - ('Target 1: iqn.2010-10.org.openstack:volume-83c2e877-feed-46be-8435-77884fe55b45\n' # noqa + ('Target 1: %(test_vol)s\n' ' System information:\n' ' Driver: iscsi\n' ' State: ready\n' @@ -67,12 +66,13 @@ class TestTgtAdmDriver(tf.TargetDriverFixture): ' SWP: No\n' ' Thin-provisioning: No\n' ' Backing store type: rdwr\n' - ' Backing store path: /dev/stack-volumes-lvmdriver-1/volume-83c2e877-feed-46be-8435-77884fe55b45\n' # noqa + ' Backing store path: %(bspath)s\n' ' Backing store flags:\n' ' Account information:\n' ' mDVpzk8cZesdahJC9h73\n' ' ACL information:\n' - ' ALL"\n') + ' ALL"\n' % {'test_vol': self.test_vol, + 'bspath': self.testvol_path}) def test_iscsi_protocol(self): self.assertEqual(self.target.iscsi_protocol, 'iscsi') @@ -80,30 +80,22 @@ class TestTgtAdmDriver(tf.TargetDriverFixture): def test_get_target(self): with mock.patch('cinder.utils.execute', return_value=(self.fake_iscsi_scan, None)): - self.assertEqual('1', - self.target._get_target( - 'iqn.2010-10.org.openstack:' - 'volume-83c2e877-feed-46be-' - '8435-77884fe55b45')) + iqn = self.test_vol + self.assertEqual('1', self.target._get_target(iqn)) def test_verify_backing_lun(self): + iqn = self.test_vol + with mock.patch('cinder.utils.execute', return_value=(self.fake_iscsi_scan, None)): - - self.assertTrue(self.target._verify_backing_lun( - 'iqn.2010-10.org.openstack:' - 'volume-83c2e877-feed-46be-' - '8435-77884fe55b45', '1')) + self.assertTrue(self.target._verify_backing_lun(iqn, '1')) # Test the failure case bad_scan = self.fake_iscsi_scan.replace('LUN: 1', 'LUN: 3') with mock.patch('cinder.utils.execute', return_value=(bad_scan, None)): - self.assertFalse(self.target._verify_backing_lun( - 'iqn.2010-10.org.openstack:' - 'volume-83c2e877-feed-46be-' - '8435-77884fe55b45', '1')) + self.assertFalse(self.target._verify_backing_lun(iqn, '1')) @mock.patch.object(time, 'sleep') @mock.patch('cinder.utils.execute') @@ -116,8 +108,7 @@ class TestTgtAdmDriver(tf.TargetDriverFixture): expected_command = ('tgtadm', '--lld', 'iscsi', '--op', 'new', '--mode', 'logicalunit', '--tid', '1', '--lun', '1', '-b', - '/dev/stack-volumes-lvmdriver-1/' - 'volume-83c2e877-feed-46be-8435-77884fe55b45') + self.testvol_path) mock_execute.assert_called_once_with(*expected_command, run_as_root=True) @@ -147,18 +138,19 @@ class TestTgtAdmDriver(tf.TargetDriverFixture): def test_get_target_chap_auth(self): persist_file =\ - '\n'\ - ' backing-store /dev/stack-volumes-lvmdriver-1/volume-83c2e877-feed-46be-8435-77884fe55b45\n'\ + '\n'\ + ' backing-store %(bspath)s\n'\ ' driver iscsi\n'\ - ' incominguser otzLy2UYbYfnP4zXLG5z 234Zweo38VGBBvrpK9nt\n'\ + ' incominguser otzL 234Z\n'\ ' write-cache on\n'\ - '' + '' % {'id': self.VOLUME_ID, + 'bspath': self.testvol_path} with open(os.path.join(self.fake_volumes_dir, self.test_vol.split(':')[1]), 'wb') as tmp_file: tmp_file.write(persist_file) ctxt = context.get_admin_context() - expected = ('otzLy2UYbYfnP4zXLG5z', '234Zweo38VGBBvrpK9nt') + expected = ('otzL', '234Z') self.assertEqual(expected, self.target._get_target_chap_auth(ctxt, self.test_vol)) @@ -240,25 +232,22 @@ class TestTgtAdmDriver(tf.TargetDriverFixture): mock_exec.side_effect = _fake_execute - test_vol_id = '83c2e877-feed-46be-8435-77884fe55b45' - test_vol_name = 'volume-83c2e877-feed-46be-8435-77884fe55b45' - with mock.patch.object(self.target, '_get_target', return_value=False): self.assertEqual( None, self.target.remove_iscsi_target( 1, 0, - test_vol_id, - test_vol_name)) + self.VOLUME_ID, + self.VOLUME_NAME)) mock_exec.side_effect = _fake_execute_wrong_message self.assertRaises(exception.ISCSITargetRemoveFailed, self.target.remove_iscsi_target, 1, 0, - test_vol_id, - test_vol_name) + self.VOLUME_ID, + self.VOLUME_NAME) @mock.patch('os.path.isfile', return_value=True) @mock.patch('os.path.exists', return_value=True) @@ -285,25 +274,22 @@ class TestTgtAdmDriver(tf.TargetDriverFixture): mock_exec.side_effect = _fake_execute - test_vol_id = '83c2e877-feed-46be-8435-77884fe55b45' - test_vol_name = 'volume-83c2e877-feed-46be-8435-77884fe55b45' - with mock.patch.object(self.target, '_get_target', return_value=False): self.assertEqual( None, self.target.remove_iscsi_target( 1, 0, - test_vol_id, - test_vol_name)) + self.VOLUME_ID, + self.VOLUME_NAME)) mock_exec.side_effect = _fake_execute_wrong_message self.assertRaises(exception.ISCSITargetRemoveFailed, self.target.remove_iscsi_target, 1, 0, - test_vol_id, - test_vol_name) + self.VOLUME_ID, + self.VOLUME_NAME) @mock.patch.object(tgt.TgtAdm, '_get_iscsi_properties') def test_initialize_connection(self, mock_get_iscsi): @@ -359,8 +345,7 @@ class TestTgtAdmDriver(tf.TargetDriverFixture): expected_result = {'location': '10.9.8.7:3260,1 ' + self.iscsi_target_prefix + self.testvol['name'] + ' 1', - 'auth': 'CHAP ' - 'QZJbisG9AL954FNF4D P68eE7u9eFqDGexd28DQ'} + 'auth': 'CHAP QZJb P68e'} with mock.patch('cinder.utils.execute', return_value=('', '')),\ mock.patch.object(self.target, '_get_target', @@ -370,9 +355,9 @@ class TestTgtAdmDriver(tf.TargetDriverFixture): mock.patch.object(self.target, '_get_target_chap_auth', side_effect=lambda x, y: None) as m_chap,\ mock.patch.object(vutils, 'generate_username', - side_effect=lambda: 'QZJbisG9AL954FNF4D'),\ + side_effect=lambda: 'QZJb'),\ mock.patch.object(vutils, 'generate_password', - side_effect=lambda: 'P68eE7u9eFqDGexd28DQ'): + side_effect=lambda: 'P68e'): ctxt = context.get_admin_context() self.assertEqual(expected_result, @@ -380,12 +365,9 @@ class TestTgtAdmDriver(tf.TargetDriverFixture): self.testvol, self.fake_volumes_dir)) - m_chap.side_effect = lambda x, y: ('otzLy2UYbYfnP4zXLG5z', - '234Zweo38VGBBvrpK9nt') + m_chap.side_effect = lambda x, y: ('otzL', '234Z') - expected_result['auth'] = ('CHAP ' - 'otzLy2UYbYfnP4zXLG5z ' - '234Zweo38VGBBvrpK9nt') + expected_result['auth'] = ('CHAP otzL 234Z') self.assertEqual(expected_result, self.target.create_export(ctxt, -- 2.45.2