Remove use of contextlib.nested

The contextlib.nested call has been deprecated
in Python 2.7. This causes DeprecationWarning
messages in the unit tests.

There are also known issues with contextlib.nested
that were addressed by the native support for
multiple "with" variables. For instance, if the
first object is created but the second one throws
an exception, the first object's __exit__ is never
called.

Since Cinder no longer supports 2.6 we can remove
the use of these contextlib.nested calls.

Added hacking check to catch if any new instances
are added to the codebase.

Note: line continuation markers (e.g. '\') had to
be used or syntax errors were thrown. While using
parentheses is the preferred way for multiple line
statements it is not a requirement.

Partial-Bug: 1428424
Change-Id: I7bb7d201d31ff239be3402fb64e5f202ede019b0
This commit is contained in:
Sean McGinnis 2015-02-24 09:24:52 -06:00
parent 72b7f3b8b5
commit ddbcdbb2d0
9 changed files with 118 additions and 121 deletions

View File

@ -14,6 +14,7 @@ Cinder Specific Commandments
- [N324] Enforce no use of LOG.audit messages. LOG.info should be used instead.
- [N327] assert_called_once is not a valid Mock method.
- [N333] Ensure that oslo namespaces are used for namespaced libraries.
- [N339] Prevent use of deprecated contextlib.nested.
General

View File

@ -138,6 +138,15 @@ def check_oslo_namespace_imports(logical_line):
yield(0, msg)
def check_no_contextlib_nested(logical_line):
msg = ("N339: contextlib.nested is deprecated. With Python 2.7 and later "
"the with-statement supports multiple nested objects. See https://"
"docs.python.org/2/library/contextlib.html#contextlib.nested "
"for more information.")
if "with contextlib.nested" in logical_line:
yield(0, msg)
def factory(register):
register(no_vi_headers)
register(no_translate_debug_logs)
@ -146,3 +155,4 @@ def factory(register):
register(check_no_log_audit)
register(check_assert_called_once)
register(check_oslo_namespace_imports)
register(check_no_contextlib_nested)

View File

@ -12,8 +12,6 @@
# License for the specific language governing permissions and limitations
# under the License.
import contextlib
import mock
from cinder.compute import nova
@ -102,11 +100,10 @@ class NovaApiTestCase(test.TestCase):
self.ctx = context.get_admin_context()
def test_update_server_volume(self):
with contextlib.nested(
mock.patch.object(nova, 'novaclient'),
with mock.patch.object(nova, 'novaclient') as mock_novaclient, \
mock.patch.object(self.novaclient.volumes,
'update_server_volume')
) as (mock_novaclient, mock_update_server_volume):
'update_server_volume') as \
mock_update_server_volume:
mock_novaclient.return_value = self.novaclient
self.api.update_server_volume(self.ctx, 'server_id',

View File

@ -14,7 +14,6 @@
# under the License.
""" Tests for Ceph backup service."""
import contextlib
import hashlib
import os
import tempfile
@ -481,10 +480,9 @@ class BackupCephTestCase(test.TestCase):
self.mock_rbd.RBD.list = mock.Mock()
self.mock_rbd.RBD.list.return_value = [backup_name]
with contextlib.nested(
mock.patch.object(self.service, 'get_backup_snaps'),
mock.patch.object(self.service, '_rbd_diff_transfer')
) as (_unused, mock_rbd_diff_transfer):
with mock.patch.object(self.service, 'get_backup_snaps'), \
mock.patch.object(self.service, '_rbd_diff_transfer') as \
mock_rbd_diff_transfer:
def mock_rbd_diff_transfer_side_effect(src_name, src_pool,
dest_name, dest_pool,
src_user, src_conf,
@ -495,10 +493,11 @@ class BackupCephTestCase(test.TestCase):
# Raise a pseudo exception.BackupRBDOperationFailed.
mock_rbd_diff_transfer.side_effect \
= mock_rbd_diff_transfer_side_effect
with contextlib.nested(
mock.patch.object(self.service, '_full_backup'),
mock.patch.object(self.service, '_try_delete_base_image')
) as (_unused, mock_try_delete_base_image):
with mock.patch.object(self.service, '_full_backup'), \
mock.patch.object(self.service,
'_try_delete_base_image') as \
mock_try_delete_base_image:
def mock_try_delete_base_image_side_effect(backup_id,
volume_id,
base_name):
@ -556,12 +555,11 @@ class BackupCephTestCase(test.TestCase):
self.mock_rbd.RBD.list = mock.Mock()
self.mock_rbd.RBD.list.return_value = [backup_name]
with contextlib.nested(
mock.patch.object(self.service, 'get_backup_snaps'),
mock.patch.object(self.service, '_rbd_diff_transfer'),
mock.patch.object(self.service, '_full_backup'),
mock.patch.object(self.service, '_backup_metadata')
) as (_unused1, _u2, _u3, mock_backup_metadata):
with mock.patch.object(self.service, 'get_backup_snaps'), \
mock.patch.object(self.service, '_rbd_diff_transfer'), \
mock.patch.object(self.service, '_full_backup'), \
mock.patch.object(self.service, '_backup_metadata') as \
mock_backup_metadata:
def mock_backup_metadata_side_effect(backup):
raise exception.BackupOperationError(_('mock'))

View File

@ -10,7 +10,6 @@
# License for the specific language governing permissions and limitations
# under the License.
import contextlib
import datetime
import StringIO
import sys
@ -706,16 +705,14 @@ class TestCinderRtstoolCmd(test.TestCase):
mock.sentinel.iser_enabled)
def _test_create_rtslib_error_network_portal(self, ip):
with contextlib.nested(
mock.patch('rtslib.NetworkPortal'),
mock.patch('rtslib.LUN'),
mock.patch('rtslib.TPG'),
mock.patch('rtslib.FabricModule'),
mock.patch('rtslib.Target'),
mock.patch('rtslib.BlockStorageObject'),
mock.patch('rtslib.root.RTSRoot')
) as (network_portal, lun, tpg, fabric_module, target,
block_storage_object, rts_root):
with mock.patch('rtslib.NetworkPortal') as network_portal, \
mock.patch('rtslib.LUN') as lun, \
mock.patch('rtslib.TPG') as tpg, \
mock.patch('rtslib.FabricModule') as fabric_module, \
mock.patch('rtslib.Target') as target, \
mock.patch('rtslib.BlockStorageObject') as \
block_storage_object, \
mock.patch('rtslib.root.RTSRoot') as rts_root:
root_new = mock.MagicMock(storage_objects=mock.MagicMock())
rts_root.return_value = root_new
block_storage_object.return_value = mock.sentinel.so_new
@ -766,16 +763,14 @@ class TestCinderRtstoolCmd(test.TestCase):
self._test_create_rtslib_error_network_portal('::0')
def _test_create(self, ip):
with contextlib.nested(
mock.patch('rtslib.NetworkPortal'),
mock.patch('rtslib.LUN'),
mock.patch('rtslib.TPG'),
mock.patch('rtslib.FabricModule'),
mock.patch('rtslib.Target'),
mock.patch('rtslib.BlockStorageObject'),
mock.patch('rtslib.root.RTSRoot')
) as (network_portal, lun, tpg, fabric_module, target,
block_storage_object, rts_root):
with mock.patch('rtslib.NetworkPortal') as network_portal, \
mock.patch('rtslib.LUN') as lun, \
mock.patch('rtslib.TPG') as tpg, \
mock.patch('rtslib.FabricModule') as fabric_module, \
mock.patch('rtslib.Target') as target, \
mock.patch('rtslib.BlockStorageObject') as \
block_storage_object, \
mock.patch('rtslib.root.RTSRoot') as rts_root:
root_new = mock.MagicMock(storage_objects=mock.MagicMock())
rts_root.return_value = root_new
block_storage_object.return_value = mock.sentinel.so_new

View File

@ -163,3 +163,9 @@ class HackingTestCase(test.TestCase):
"from oslo.log import foo"))))
self.assertEqual(0, len(list(checks.check_oslo_namespace_imports(
"from oslo_log import bar"))))
def test_no_contextlib_nested(self):
self.assertEqual(1, len(list(checks.check_no_contextlib_nested(
"with contextlib.nested("))))
self.assertEqual(0, len(list(checks.check_no_contextlib_nested(
"with foo as bar"))))

View File

@ -15,7 +15,6 @@
# under the License.
"""Unit tests for the Quobyte driver module."""
import contextlib
import errno
import os
import StringIO
@ -126,11 +125,9 @@ class QuobyteDriverTestCase(test.TestCase):
drv.local_path(volume))
def test_mount_quobyte_should_mount_correctly(self):
with contextlib.nested(
mock.patch.object(self._driver, '_execute'),
with mock.patch.object(self._driver, '_execute') as mock_execute, \
mock.patch('cinder.volume.drivers.quobyte.QuobyteDriver'
'.read_proc_mount'),
) as (mock_execute, mock_open):
'.read_proc_mount') as mock_open:
# Content of /proc/mount (not mounted yet).
mock_open.return_value = StringIO.StringIO(
"/dev/sda5 / ext4 rw,relatime,data=ordered 0 0")
@ -152,11 +149,9 @@ class QuobyteDriverTestCase(test.TestCase):
[mkdir_call, mount_call, getfattr_call], any_order=False)
def test_mount_quobyte_already_mounted_detected_seen_in_proc_mount(self):
with contextlib.nested(
mock.patch.object(self._driver, '_execute'),
with mock.patch.object(self._driver, '_execute') as mock_execute, \
mock.patch('cinder.volume.drivers.quobyte.QuobyteDriver'
'.read_proc_mount'),
) as (mock_execute, mock_open):
'.read_proc_mount') as mock_open:
# Content of /proc/mount (already mounted).
mock_open.return_value = StringIO.StringIO(
"quobyte@%s %s fuse rw,nosuid,nodev,noatime,user_id=1000"
@ -179,12 +174,10 @@ class QuobyteDriverTestCase(test.TestCase):
Because _mount_quobyte gets called with ensure=True, the error will
be suppressed and logged instead.
"""
with contextlib.nested(
mock.patch.object(self._driver, '_execute'),
with mock.patch.object(self._driver, '_execute') as mock_execute, \
mock.patch('cinder.volume.drivers.quobyte.QuobyteDriver'
'.read_proc_mount'),
mock.patch('cinder.volume.drivers.quobyte.LOG')
) as (mock_execute, mock_open, mock_LOG):
'.read_proc_mount') as mock_open, \
mock.patch('cinder.volume.drivers.quobyte.LOG') as mock_LOG:
# Content of /proc/mount (empty).
mock_open.return_value = StringIO.StringIO()
mock_execute.side_effect = [None, putils.ProcessExecutionError(
@ -209,11 +202,9 @@ class QuobyteDriverTestCase(test.TestCase):
test_mount_quobyte_should_suppress_and_log_already_mounted_error
but with ensure=False.
"""
with contextlib.nested(
mock.patch.object(self._driver, '_execute'),
with mock.patch.object(self._driver, '_execute') as mock_execute, \
mock.patch('cinder.volume.drivers.quobyte.QuobyteDriver'
'.read_proc_mount')
) as (mock_execute, mock_open):
'.read_proc_mount') as mock_open:
mock_open.return_value = StringIO.StringIO()
mock_execute.side_effect = [
None, # mkdir
@ -312,10 +303,10 @@ class QuobyteDriverTestCase(test.TestCase):
def test_ensure_share_mounted(self):
"""_ensure_share_mounted simple use case."""
with contextlib.nested(
mock.patch.object(self._driver, '_get_mount_point_for_share'),
mock.patch.object(self._driver, '_mount_quobyte')
) as (mock_get_mount_point, mock_mount):
with mock.patch.object(self._driver, '_get_mount_point_for_share') as \
mock_get_mount_point, \
mock.patch.object(self._driver, '_mount_quobyte') as \
mock_mount:
drv = self._driver
drv._ensure_share_mounted(self.TEST_QUOBYTE_VOLUME)
@ -578,16 +569,19 @@ class QuobyteDriverTestCase(test.TestCase):
volume_path = '%s/%s' % (self.TEST_MNT_POINT, volume_filename)
info_file = volume_path + '.info'
with contextlib.nested(
mock.patch.object(self._driver, '_ensure_share_mounted'),
mock.patch.object(self._driver, '_local_volume_dir'),
mock.patch.object(self._driver, 'get_active_image_from_info'),
mock.patch.object(self._driver, '_execute'),
mock.patch.object(self._driver, '_local_path_volume'),
mock.patch.object(self._driver, '_local_path_volume_info')
) as (mock_ensure_share_mounted, mock_local_volume_dir,
mock_active_image_from_info, mock_execute,
mock_local_path_volume, mock_local_path_volume_info):
with mock.patch.object(self._driver, '_ensure_share_mounted') as \
mock_ensure_share_mounted, \
mock.patch.object(self._driver, '_local_volume_dir') as \
mock_local_volume_dir, \
mock.patch.object(self._driver,
'get_active_image_from_info') as \
mock_active_image_from_info, \
mock.patch.object(self._driver, '_execute') as \
mock_execute, \
mock.patch.object(self._driver, '_local_path_volume') as \
mock_local_path_volume, \
mock.patch.object(self._driver, '_local_path_volume_info') as \
mock_local_path_volume_info:
mock_local_volume_dir.return_value = self.TEST_MNT_POINT
mock_active_image_from_info.return_value = volume_filename
mock_local_path_volume.return_value = volume_path
@ -812,15 +806,16 @@ class QuobyteDriverTestCase(test.TestCase):
volume_path = '%s/%s' % (self.TEST_MNT_POINT, volume['name'])
image_meta = {'id': '10958016-e196-42e3-9e7f-5d8927ae3099'}
with contextlib.nested(
mock.patch.object(drv, 'get_active_image_from_info'),
mock.patch.object(drv, '_local_volume_dir'),
mock.patch.object(image_utils, 'qemu_img_info'),
mock.patch.object(image_utils, 'upload_volume'),
mock.patch.object(image_utils, 'create_temporary_file')
) as (mock_get_active_image_from_info, mock_local_volume_dir,
mock_qemu_img_info, mock_upload_volume,
mock_create_temporary_file):
with mock.patch.object(drv, 'get_active_image_from_info') as \
mock_get_active_image_from_info, \
mock.patch.object(drv, '_local_volume_dir') as \
mock_local_volume_dir, \
mock.patch.object(image_utils, 'qemu_img_info') as \
mock_qemu_img_info, \
mock.patch.object(image_utils, 'upload_volume') as \
mock_upload_volume, \
mock.patch.object(image_utils, 'create_temporary_file') as \
mock_create_temporary_file:
mock_get_active_image_from_info.return_value = volume['name']
mock_local_volume_dir.return_value = self.TEST_MNT_POINT
@ -854,16 +849,18 @@ class QuobyteDriverTestCase(test.TestCase):
volume_path = '%s/%s' % (self.TEST_MNT_POINT, volume['name'])
image_meta = {'id': '10958016-e196-42e3-9e7f-5d8927ae3099'}
with contextlib.nested(
mock.patch.object(drv, 'get_active_image_from_info'),
mock.patch.object(drv, '_local_volume_dir'),
mock.patch.object(image_utils, 'qemu_img_info'),
mock.patch.object(image_utils, 'convert_image'),
mock.patch.object(image_utils, 'upload_volume'),
mock.patch.object(image_utils, 'create_temporary_file')
) as (mock_get_active_image_from_info, mock_local_volume_dir,
mock_qemu_img_info, mock_convert_image, mock_upload_volume,
mock_create_temporary_file):
with mock.patch.object(drv, 'get_active_image_from_info') as \
mock_get_active_image_from_info, \
mock.patch.object(drv, '_local_volume_dir') as \
mock_local_volume_dir, \
mock.patch.object(image_utils, 'qemu_img_info') as \
mock_qemu_img_info, \
mock.patch.object(image_utils, 'convert_image') as \
mock_convert_image, \
mock.patch.object(image_utils, 'upload_volume') as \
mock_upload_volume, \
mock.patch.object(image_utils, 'create_temporary_file') as \
mock_create_temporary_file:
mock_get_active_image_from_info.return_value = volume['name']
mock_local_volume_dir.return_value = self.TEST_MNT_POINT
@ -900,16 +897,18 @@ class QuobyteDriverTestCase(test.TestCase):
volume_filename = 'volume-%s' % self.VOLUME_UUID
image_meta = {'id': '10958016-e196-42e3-9e7f-5d8927ae3099'}
with contextlib.nested(
mock.patch.object(drv, 'get_active_image_from_info'),
mock.patch.object(drv, '_local_volume_dir'),
mock.patch.object(image_utils, 'qemu_img_info'),
mock.patch.object(image_utils, 'convert_image'),
mock.patch.object(image_utils, 'upload_volume'),
mock.patch.object(image_utils, 'create_temporary_file')
) as (mock_get_active_image_from_info, mock_local_volume_dir,
mock_qemu_img_info, mock_convert_image, mock_upload_volume,
mock_create_temporary_file):
with mock.patch.object(drv, 'get_active_image_from_info') as \
mock_get_active_image_from_info, \
mock.patch.object(drv, '_local_volume_dir') as \
mock_local_volume_dir, \
mock.patch.object(image_utils, 'qemu_img_info') as \
mock_qemu_img_info, \
mock.patch.object(image_utils, 'convert_image') as \
mock_convert_image, \
mock.patch.object(image_utils, 'upload_volume') as \
mock_upload_volume, \
mock.patch.object(image_utils, 'create_temporary_file') as \
mock_create_temporary_file:
mock_get_active_image_from_info.return_value = volume['name']
mock_local_volume_dir.return_value = self.TEST_MNT_POINT

View File

@ -12,7 +12,6 @@
# License for the specific language governing permissions and limitations
# under the License.
import contextlib
import copy
import os
@ -321,10 +320,9 @@ class SmbFsTestCase(test.TestCase):
return_value=mock.Mock(file_format=image_format))
drv._delete = mock.Mock()
with contextlib.nested(
mock.patch.object(image_utils, 'resize_image'),
mock.patch.object(image_utils, 'convert_image')) as (
fake_resize, fake_convert):
with mock.patch.object(image_utils, 'resize_image') as fake_resize, \
mock.patch.object(image_utils, 'convert_image') as \
fake_convert:
if extend_failed:
self.assertRaises(exception.ExtendVolumeError,
drv._extend_volume,
@ -473,13 +471,9 @@ class SmbFsTestCase(test.TestCase):
mock.sentinel.block_size)
exc = None
with contextlib.nested(
mock.patch.object(image_utils,
'fetch_to_volume_format'),
mock.patch.object(image_utils,
'qemu_img_info')) as (
fake_fetch,
fake_qemu_img_info):
with mock.patch.object(image_utils, 'fetch_to_volume_format') as \
fake_fetch, mock.patch.object(image_utils, 'qemu_img_info') as \
fake_qemu_img_info:
if wrong_size_after_fetch:
exc = exception.ImageUnacceptable

View File

@ -18,7 +18,6 @@ Tests for Volume Code.
"""
import contextlib
import datetime
import os
import shutil
@ -1618,12 +1617,10 @@ class VolumeTestCase(BaseVolumeTestCase):
'key2': 'value2'}
}
with contextlib.nested(
mock.patch.object(cinder.volume.volume_types,
'get_volume_type_qos_specs'),
with mock.patch.object(cinder.volume.volume_types,
'get_volume_type_qos_specs') as type_qos, \
mock.patch.object(cinder.tests.fake_driver.FakeISCSIDriver,
'initialize_connection')
) as (type_qos, driver_init):
'initialize_connection') as driver_init:
type_qos.return_value = dict(qos_specs=qos_values)
driver_init.return_value = {'data': {}}
qos_specs_expected = {'key1': 'value1',