openstack/cinder
Code Issues Proposed changes

tests: Don't use mocks as requests

Browse Source 
These have weird side-effects (like returning true for all microversion
checks) and don't behave like real request objects.

This process highlights a couple of issues with the tests:

- In one test we were trying to filter by 'id', which is not a valid
  filter. We now use 'availability_zone' instead ('name' doesn't work
  since we don't use ast.literal_eval on this field, which means the
  list operation doesn't work).
- In multiple tests we were trying to filter by 'display_name' but API
  3.0 used 'name'. We update these references.
- In multiple tests we were using integers (0 or 1) to filter on boolean
  fields but this is only supported by API microversion 3.2. We use
  stringified booleans instead.

Change-Id: I275c85e223f925b54d5941ba02edec725080a357
Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
This commit is contained in:
Stephen Finucane
2024-02-19 13:14:29 +00:00
parent 1cde7143a8
commit c5b9496c4b
1 changed files with 46 additions and 59 deletions
Download Patch File Download Diff File Expand all files Collapse all files

105
cinder/tests/unit/api/v2/test_volumes.py
View File

@@ -1496,14 +1496,13 @@ class VolumeApiTest(test.TestCase):
self._create_volume_bad_request(body=body) self._create_volume_bad_request(body=body)
def _test_get_volumes_by_name(self, get_all, display_name): def _test_get_volumes_by_name(self, get_all, display_name):
req = mock.MagicMock() qs = urllib.parse.urlencode({'name': display_name})
context = mock.Mock() req = fakes.HTTPRequest.blank('/v3/volumes' + '?' + qs)
req.environ = {'cinder.context': context} ctxt = req.environ['cinder.context']
req.params = {'display_name': display_name}
self.controller._view_builder.detail_list = mock.Mock() self.controller._view_builder.detail_list = mock.Mock()
self.controller._get_volumes(req, True) self.controller._get_volumes(req, True)
get_all.assert_called_once_with( get_all.assert_called_once_with(
context, None, CONF.osapi_max_limit, ctxt, None, CONF.osapi_max_limit,
sort_keys=['created_at'], sort_dirs=['desc'], sort_keys=['created_at'], sort_dirs=['desc'],
filters={'display_name': display_name}, filters={'display_name': display_name},
viewable_admin_meta=True, offset=0) viewable_admin_meta=True, offset=0)
@@ -1537,68 +1536,67 @@ class VolumeApiTest(test.TestCase):
@mock.patch('cinder.volume.api.API.get_all') @mock.patch('cinder.volume.api.API.get_all')
def test_get_volumes_filter_with_true(self, get_all): def test_get_volumes_filter_with_true(self, get_all):
req = mock.MagicMock() qs = urllib.parse.urlencode(
context = mock.Mock() {'name': 'Volume-573108026', 'bootable': 'true'}
req.environ = {'cinder.context': context} )
req.params = {'display_name': 'Volume-573108026', 'bootable': 1} req = fakes.HTTPRequest.blank('/v3/volumes' + '?' + qs)
ctxt = req.environ['cinder.context']
self.controller._view_builder.detail_list = mock.Mock() self.controller._view_builder.detail_list = mock.Mock()
self.controller._get_volumes(req, True) self.controller._get_volumes(req, True)
get_all.assert_called_once_with( get_all.assert_called_once_with(
context, None, CONF.osapi_max_limit, ctxt, None, CONF.osapi_max_limit,
sort_keys=['created_at'], sort_dirs=['desc'], sort_keys=['created_at'], sort_dirs=['desc'],
filters={'display_name': 'Volume-573108026', 'bootable': True}, filters={'display_name': 'Volume-573108026', 'bootable': True},
viewable_admin_meta=True, offset=0) viewable_admin_meta=True, offset=0)
@mock.patch('cinder.volume.api.API.get_all') @mock.patch('cinder.volume.api.API.get_all')
def test_get_volumes_filter_with_false(self, get_all): def test_get_volumes_filter_with_false(self, get_all):
req = mock.MagicMock() qs = urllib.parse.urlencode(
context = mock.Mock() {'name': 'Volume-573108026', 'bootable': 'false'}
req.environ = {'cinder.context': context} )
req.params = {'display_name': 'Volume-573108026', 'bootable': 0} req = fakes.HTTPRequest.blank('/v3/volumes' + '?' + qs)
ctxt = req.environ['cinder.context']
self.controller._view_builder.detail_list = mock.Mock() self.controller._view_builder.detail_list = mock.Mock()
self.controller._get_volumes(req, True) self.controller._get_volumes(req, True)
get_all.assert_called_once_with( get_all.assert_called_once_with(
context, None, CONF.osapi_max_limit, ctxt, None, CONF.osapi_max_limit,
sort_keys=['created_at'], sort_dirs=['desc'], sort_keys=['created_at'], sort_dirs=['desc'],
filters={'display_name': 'Volume-573108026', 'bootable': False}, filters={'display_name': 'Volume-573108026', 'bootable': False},
viewable_admin_meta=True, offset=0) viewable_admin_meta=True, offset=0)
@mock.patch('cinder.volume.api.API.get_all') @mock.patch('cinder.volume.api.API.get_all')
def test_get_volumes_filter_with_list(self, get_all): def test_get_volumes_filter_with_list(self, get_all):
req = mock.MagicMock() qs = urllib.parse.urlencode({
context = mock.Mock() 'availability_zone': "['az0', 'az1', 'az2']",
req.environ = {'cinder.context': context} })
req.params = {'id': "['%s', '%s', '%s']" % ( req = fakes.HTTPRequest.blank('/v3/volumes' + '?' + qs)
fake.VOLUME_ID, fake.VOLUME2_ID, fake.VOLUME3_ID)} ctxt = req.environ['cinder.context']
self.controller._view_builder.detail_list = mock.Mock() self.controller._view_builder.detail_list = mock.Mock()
self.controller._get_volumes(req, True) self.controller._get_volumes(req, True)
get_all.assert_called_once_with( get_all.assert_called_once_with(
context, None, CONF.osapi_max_limit, ctxt, None, CONF.osapi_max_limit,
sort_keys=['created_at'], sort_dirs=['desc'], sort_keys=['created_at'], sort_dirs=['desc'],
filters={'id': [fake.VOLUME_ID, fake.VOLUME2_ID, fake.VOLUME3_ID]}, filters={'availability_zone': ['az0', 'az1', 'az2']},
viewable_admin_meta=True, viewable_admin_meta=True,
offset=0) offset=0)
@mock.patch('cinder.volume.api.API.get_all') @mock.patch('cinder.volume.api.API.get_all')
def test_get_volumes_filter_with_expression(self, get_all): def test_get_volumes_filter_with_expression(self, get_all):
req = mock.MagicMock() qs = urllib.parse.urlencode({'name': "d-"})
context = mock.Mock() req = fakes.HTTPRequest.blank('/v3/volumes' + '?' + qs)
req.environ = {'cinder.context': context} ctxt = req.environ['cinder.context']
req.params = {'name': "d-"}
self.controller._view_builder.detail_list = mock.Mock() self.controller._view_builder.detail_list = mock.Mock()
self.controller._get_volumes(req, True) self.controller._get_volumes(req, True)
get_all.assert_called_once_with( get_all.assert_called_once_with(
context, None, CONF.osapi_max_limit, ctxt, None, CONF.osapi_max_limit,
sort_keys=['created_at'], sort_dirs=['desc'], sort_keys=['created_at'], sort_dirs=['desc'],
filters={'display_name': 'd-'}, viewable_admin_meta=True, offset=0) filters={'display_name': 'd-'}, viewable_admin_meta=True, offset=0)
@mock.patch('cinder.volume.api.API.get_all') @mock.patch('cinder.volume.api.API.get_all')
def test_get_volumes_filter_with_status(self, get_all): def test_get_volumes_filter_with_status(self, get_all):
req = mock.MagicMock() qs = urllib.parse.urlencode({'status': 'available'})
ctxt = context.RequestContext( req = fakes.HTTPRequest.blank('/v3/volumes' + '?' + qs)
fake.USER_ID, fake.PROJECT_ID, auth_token=True) ctxt = req.environ['cinder.context']
req.environ = {'cinder.context': ctxt}
req.params = {'status': 'available'}
self.controller._view_builder.detail_list = mock.Mock() self.controller._view_builder.detail_list = mock.Mock()
self.controller._get_volumes(req, True) self.controller._get_volumes(req, True)
get_all.assert_called_once_with( get_all.assert_called_once_with(
@@ -1609,11 +1607,9 @@ class VolumeApiTest(test.TestCase):
@mock.patch('cinder.volume.api.API.get_all') @mock.patch('cinder.volume.api.API.get_all')
def test_get_volumes_filter_with_metadata(self, get_all): def test_get_volumes_filter_with_metadata(self, get_all):
req = mock.MagicMock() qs = urllib.parse.urlencode({'metadata': "{'fake_key': 'fake_value'}"})
ctxt = context.RequestContext( req = fakes.HTTPRequest.blank('/v3/volumes' + '?' + qs)
fake.USER_ID, fake.PROJECT_ID, auth_token=True) ctxt = req.environ['cinder.context']
req.environ = {'cinder.context': ctxt}
req.params = {'metadata': "{'fake_key': 'fake_value'}"}
self.controller._view_builder.detail_list = mock.Mock() self.controller._view_builder.detail_list = mock.Mock()
self.controller._get_volumes(req, True) self.controller._get_volumes(req, True)
get_all.assert_called_once_with( get_all.assert_called_once_with(
@@ -1624,11 +1620,9 @@ class VolumeApiTest(test.TestCase):
@mock.patch('cinder.volume.api.API.get_all') @mock.patch('cinder.volume.api.API.get_all')
def test_get_volumes_filter_with_availability_zone(self, get_all): def test_get_volumes_filter_with_availability_zone(self, get_all):
req = mock.MagicMock() qs = urllib.parse.urlencode({'availability_zone': 'nova'})
ctxt = context.RequestContext( req = fakes.HTTPRequest.blank('/v3/volumes' + '?' + qs)
fake.USER_ID, fake.PROJECT_ID, auth_token=True) ctxt = req.environ['cinder.context']
req.environ = {'cinder.context': ctxt}
req.params = {'availability_zone': 'nova'}
self.controller._view_builder.detail_list = mock.Mock() self.controller._view_builder.detail_list = mock.Mock()
self.controller._get_volumes(req, True) self.controller._get_volumes(req, True)
get_all.assert_called_once_with( get_all.assert_called_once_with(
@@ -1639,11 +1633,9 @@ class VolumeApiTest(test.TestCase):
@mock.patch('cinder.volume.api.API.get_all') @mock.patch('cinder.volume.api.API.get_all')
def test_get_volumes_filter_with_bootable(self, get_all): def test_get_volumes_filter_with_bootable(self, get_all):
req = mock.MagicMock() qs = urllib.parse.urlencode({'bootable': 'true'})
ctxt = context.RequestContext( req = fakes.HTTPRequest.blank('/v3/volumes' + '?' + qs)
fake.USER_ID, fake.PROJECT_ID, auth_token=True) ctxt = req.environ['cinder.context']
req.environ = {'cinder.context': ctxt}
req.params = {'bootable': 1}
self.controller._view_builder.detail_list = mock.Mock() self.controller._view_builder.detail_list = mock.Mock()
self.controller._get_volumes(req, True) self.controller._get_volumes(req, True)
get_all.assert_called_once_with( get_all.assert_called_once_with(
@@ -1654,12 +1646,10 @@ class VolumeApiTest(test.TestCase):
@mock.patch('cinder.volume.api.API.get_all') @mock.patch('cinder.volume.api.API.get_all')
def test_get_volumes_filter_with_invalid_filter(self, get_all): def test_get_volumes_filter_with_invalid_filter(self, get_all):
req = mock.MagicMock() qs = urllib.parse.urlencode(
ctxt = context.RequestContext( {'availability_zone': 'nova', 'invalid_filter': 'invalid'})
fake.USER_ID, fake.PROJECT_ID, auth_token=True) req = fakes.HTTPRequest.blank('/v3/volumes' + '?' + qs)
req.environ = {'cinder.context': ctxt} ctxt = req.environ['cinder.context']
req.params = {'invalid_filter': 'invalid',
'availability_zone': 'nova'}
self.controller._view_builder.detail_list = mock.Mock() self.controller._view_builder.detail_list = mock.Mock()
self.controller._get_volumes(req, True) self.controller._get_volumes(req, True)
get_all.assert_called_once_with( get_all.assert_called_once_with(
@@ -1671,12 +1661,9 @@ class VolumeApiTest(test.TestCase):
@mock.patch('cinder.volume.api.API.get_all') @mock.patch('cinder.volume.api.API.get_all')
def test_get_volumes_sort_by_name(self, get_all): def test_get_volumes_sort_by_name(self, get_all):
"""Name in client means display_name in database.""" """Name in client means display_name in database."""
qs = urllib.parse.urlencode({'sort': 'name'})
req = mock.MagicMock() req = fakes.HTTPRequest.blank('/v3/volumes' + '?' + qs)
ctxt = context.RequestContext( ctxt = req.environ['cinder.context']
fake.USER_ID, fake.PROJECT_ID, auth_token=True)
req.environ = {'cinder.context': ctxt}
req.params = {'sort': 'name'}
self.controller._view_builder.detail_list = mock.Mock() self.controller._view_builder.detail_list = mock.Mock()
self.controller._get_volumes(req, True) self.controller._get_volumes(req, True)
get_all.assert_called_once_with( get_all.assert_called_once_with(