openstack/nova
Code Issues Proposed changes

Fix test_age_and_verify_swap_images

Browse Source 
This test mocked os.remove, but the faked implementation recursively
called os.remove if given a path outside the base directory. This
resulted in infinite recursion. We fix this.

The test did not ensure that info files were created under the same
fake base directory as cache entries. This did not cause a failure in
the current code, but a subsequent change trips this.

We update the faking of exists to only return True for files in
expected_exist, which is also tripped in a subsequent change.

We also make a couple of trivial cleanups in related code.

Change-Id: I4f3685bb525ef7b601be3bb7a28983dd4085e68f
This commit is contained in:
Matthew Booth 2016-05-26 13:26:04 +01:00
parent c859509f8c
commit f124b85fa4
1 changed files with 15 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

27
nova/tests/unit/virt/libvirt/test_imagecache.py
View File

@ -850,11 +850,15 @@ class ImageCacheManagerTestCase(test.NoDBTestCase):
@mock.patch.object(lockutils, 'external_lock')
@mock.patch.object(libvirt_utils, 'update_mtime')
@mock.patch('os.path.exists', return_value=True)
@mock.patch('os.path.exists')
@mock.patch('os.path.getmtime')
@mock.patch('os.remove')
def test_age_and_verify_swap_images(self, mock_remove, mock_getmtime,
mock_exist, mock_mtime, mock_lock):
base_dir = '/tmp_age_test'
self.flags(image_info_filename_pattern=base_dir + '/%(image)s.info',
group='libvirt')
image_cache_manager = imagecache.ImageCacheManager()
expected_remove = set()
expected_exist = set(['swap_128', 'swap_256'])
@ -864,26 +868,25 @@ class ImageCacheManagerTestCase(test.NoDBTestCase):
image_cache_manager.used_swap_images.add('swap_128')
def getmtime(path):
return time.time() - 1000000
mock_getmtime.side_effect = lambda path: time.time() - 1000000
mock_getmtime.side_effect = getmtime
mock_exist.side_effect = \
lambda path: os.path.dirname(path) == base_dir and \
os.path.basename(path) in expected_exist
def removefile(path):
if not path.startswith('/tmp_age_test'):
return os.remove(path)
self.assertEqual(base_dir, os.path.dirname(path),
'Attempt to remove unexpected path')
fn = os.path.split(path)[-1]
fn = os.path.basename(path)
expected_remove.add(fn)
expected_exist.remove(fn)
mock_remove.side_effect = removefile
image_cache_manager._age_and_verify_swap_images(None, '/tmp_age_test')
self.assertEqual(1, len(expected_exist))
self.assertEqual(1, len(expected_remove))
self.assertIn('swap_128', expected_exist)
self.assertIn('swap_256', expected_remove)
image_cache_manager._age_and_verify_swap_images(None, base_dir)
self.assertEqual(set(['swap_128']), expected_exist)
self.assertEqual(set(['swap_256']), expected_remove)
@mock.patch.object(utils, 'synchronized')
@mock.patch.object(imagecache.ImageCacheManager, '_get_age_of_file',