Tests: Disallow use of assertTrue(str)

self.assertTrue(string) is fairly dangerous because
things like self.assertTrue('False') will pass.

Require a more specific assertion when passing a
string to assertTrue, which should help prevent mistakes.

Fix up tests that do this currently, some of which
appear to not be testing what they intended.

Change-Id: I0cd29b9f3247ae5b7b3c478b96970b73d69a72c0
This commit is contained in:
Eric Harney 2017-06-07 09:58:10 -04:00
parent ad54657026
commit 7a84172cb2
8 changed files with 22 additions and 17 deletions

View File

@ -346,6 +346,13 @@ class TestCase(testtools.TestCase):
self.assertEqual(call[0], posargs[0])
self.assertEqual(call[1], posargs[2])
def assertTrue(self, x, *args, **kwargs):
if isinstance(x, six.string_types):
raise AssertionError("%s (%s) is a string. Use a more "
"specific assertion such as assertEqual." %
(x, type(x)))
super(TestCase, self).assertTrue(x, *args, **kwargs)
class ModelsObjectComparatorMixin(object):
def _dict_from_object(self, obj, ignored_keys):

View File

@ -39,5 +39,5 @@ class DefinedMessagesTest(test.TestCase):
for attr_name in dir(defined_messages.EventIds):
if not attr_name.startswith('_'):
value = getattr(defined_messages.EventIds, attr_name)
self.assertTrue(defined_messages.event_id_message_map.get(
value))
msg = defined_messages.event_id_message_map.get(value)
self.assertGreater(len(msg), 1)

View File

@ -2525,7 +2525,7 @@ class DBAPIBackupTestCase(BaseTest):
def test_backup_create(self):
values = self._get_values()
for i, backup in enumerate(self.created):
self.assertTrue(backup['id'])
self.assertEqual(36, len(backup['id'])) # dynamic UUID
self._assertEqualObjects(values[i], backup, self._ignored_keys)
def test_backup_get(self):

View File

@ -39,5 +39,4 @@ class TestUtilsTestCase(test.TestCase):
self.assertEqual([], ctxt.service_catalog)
self.assertEqual('no', ctxt.read_deleted)
self.assertTrue(ctxt.read_deleted)
self.assertTrue(ctxt.is_admin)

View File

@ -536,7 +536,7 @@ class IBMStorageVolumeDriverTest(test.TestCase):
self.driver.create_volume(volume)
ret = self.driver.retype(ctxt, volume, new_type, diff, host)
self.assertTrue(ret)
self.assertTrue(volume['easytier'])
self.assertEqual('1', volume['easytier'])
def test_retype_fail_on_exception(self):
"""Test that retype fails on exception."""

View File

@ -444,7 +444,7 @@ class EMCCoprHDISCSIDriverTest(test.TestCase):
def test_get_volume_stats(self):
vol_stats = self.driver.get_volume_stats(True)
self.assertTrue(vol_stats['free_capacity_gb'], 'unknown')
self.assertEqual('unknown', vol_stats['free_capacity_gb'])
def test_create_volume_clone(self):
src_volume_data = get_test_volume_data(self.volume_type_id)
@ -610,7 +610,7 @@ class EMCCoprHDFCDriverTest(test.TestCase):
def test_get_volume_stats(self):
vol_stats = self.driver.get_volume_stats(True)
self.assertTrue(vol_stats['free_capacity_gb'], 'unknown')
self.assertEqual('unknown', vol_stats['free_capacity_gb'])
def test_create_volume_clone(self):
@ -827,7 +827,7 @@ class EMCCoprHDScaleIODriverTest(test.TestCase):
def test_get_volume_stats(self):
vol_stats = self.driver.get_volume_stats(True)
self.assertTrue(vol_stats['free_capacity_gb'], 'unknown')
self.assertEqual('unknown', vol_stats['free_capacity_gb'])
def test_create_volume_clone(self):

View File

@ -245,8 +245,8 @@ class GPFSDriverTestCase(test.TestCase):
'yes '
' 10485760 9953792 ( 95%) '
'10485760 9954560 ( 95%)', '')
self.assertTrue(self.driver._gpfs_device,
self.driver._verify_gpfs_pool('/dev/gpfs'))
self.assertEqual('/dev/gpfs', self.driver._gpfs_device)
self.assertTrue(self.driver._verify_gpfs_pool('/dev/gpfs'))
@mock.patch('cinder.utils.execute')
def test_verify_gpfs_pool_fail_pool(self, mock_exec):
@ -260,8 +260,8 @@ class GPFSDriverTestCase(test.TestCase):
'yes'
' 10485760 9953792 ( 95%)'
' 10485760 9954560 ( 95%)', '')
self.assertTrue(self.driver._gpfs_device,
self.driver._verify_gpfs_pool('/dev/gpfs'))
self.assertEqual('/dev/gpfs', self.driver._gpfs_device)
self.assertTrue(self.driver._verify_gpfs_pool('/dev/gpfs'))
@mock.patch('cinder.utils.execute')
def test_verify_gpfs_pool_fail_raise(self, mock_exec):
@ -1142,11 +1142,11 @@ class GPFSDriverTestCase(test.TestCase):
stats = self.driver.get_volume_stats()
self.assertEqual('GPFS', stats['volume_backend_name'])
self.assertEqual('file', stats['storage_protocol'])
self.assertTrue(stats['gpfs_encryption_rest'])
self.assertEqual('True', stats['gpfs_encryption_rest'])
stats = self.driver.get_volume_stats(True)
self.assertEqual('GPFS', stats['volume_backend_name'])
self.assertEqual('file', stats['storage_protocol'])
self.assertTrue(stats['gpfs_encryption_rest'])
self.assertEqual('True', stats['gpfs_encryption_rest'])
@mock.patch('cinder.utils.execute')
def test_get_gpfs_encryption_status_true(self, mock_exec):

View File

@ -389,9 +389,8 @@ class SnapshotTestCase(base.BaseVolumeTestCase):
vol_glance_meta = db.volume_glance_metadata_get(ctxt, volume_id)
self.assertTrue(vol_glance_meta)
snap = create_snapshot(volume_id)
snap_stat = snap.status
self.assertTrue(snap.id)
self.assertTrue(snap_stat)
self.assertEqual(36, len(snap.id)) # dynamically-generated UUID
self.assertEqual('creating', snap.status)
# set to return DB exception
with mock.patch.object(db, 'volume_glance_metadata_copy_to_snapshot')\