Browse Source

test: Ensure to stop mock when create_mocks decorator exits

During reviewing https://review.opendev.org/c/openstack/horizon/+/772603,
we noticed that a method decorated by create_mocks is called multiple time
in a single test. Previously create_mocks decorator does not stop mocking,
so this means that multiple active mock can exist for one method.
In general, it is not a good idea to mock a method multiple times at the
same time.

To cope with this situation, this commit ensures for create_mocks decorator
to stop active mocks when exiting the decorator.

This works for most cases, but it does not work only when mocking and
assertions are handled by separate methods and test logic is placed between
them. To cope with this, "stop_mock" optional argument is introduced.

FYI: Details on why "stop_mock" is needed.
I explored various ways but could not find a good way so far.
create_mocks needs to be a decorator as it needs to refer an object
reference (self). On the other hand, if we would like to merge the logic of
mocking and assertions (for example, _stub_api_calls() and _check_api_calls()
in openstack_dashboard/dashboards/project/overview/tests.py), a context
manager would be good as we would like to call it inside a class.
However, we cannot mix a decorator and a context manger as a decorator is
executed when a context manager is iniitialized and stopping mock in
the create_mocks decorator is done during the initialization of the
context manager and methods are not mocked when test code is run.

Change-Id: I9e37dc1eaa08adf36d11975fed6f5a0a90cdde52
changes/49/777749/1
Akihiro Motoki 9 months ago
parent
commit
1dd12d8deb
  1. 5
      openstack_dashboard/dashboards/project/instances/tests.py
  2. 9
      openstack_dashboard/dashboards/project/overview/tests.py
  3. 13
      openstack_dashboard/test/helpers.py

5
openstack_dashboard/dashboards/project/instances/tests.py

@ -194,7 +194,10 @@ class InstanceTableTests(InstanceTestBase, InstanceTableTestMixin):
'servers_update_addresses',
),
api.cinder: ('volume_list',),
})
}, stop_mock=False)
# NOTE: _get_index() and _check_get_index() are used as pair
# and the test logic will be placed between these calls,
# so we cannot stop mocking when exiting this method.
def _get_index(self, use_servers_update_address=True):
servers = self.servers.list()
self.mock_is_feature_available.return_value = True

9
openstack_dashboard/dashboards/project/overview/tests.py

@ -35,8 +35,12 @@ class UsageViewTests(test.TestCase):
@test.create_mocks({
api.nova: ('usage_get',),
api.neutron: ('is_quotas_extension_supported',)
})
api.neutron: ('is_quotas_extension_supported',),
usage.quotas: ('tenant_quota_usages',),
}, stop_mock=False)
# NOTE: _stub_api_calls() and _check_api_calls() are used as pair
# and the test logic will be placed between these calls,
# so we cannot stop mocking when exiting this method.
def _stub_api_calls(self, nova_stu_enabled=True,
stu_exception=False, overview_days_range=1,
quota_usage_overrides=None,
@ -70,7 +74,6 @@ class UsageViewTests(test.TestCase):
usages.add_quota(api.base.Quota(k, quota))
usages.tally(k, quota_usages[k]['used'])
@test.create_mocks({usage.quotas: ('tenant_quota_usages',)})
def _stub_tenant_quota_usages(self, overrides):
usages_data = usage.quotas.QuotaUsage()
self._add_quota_usages(usages_data, self.quota_usages.first(),

13
openstack_dashboard/test/helpers.py

@ -54,7 +54,7 @@ IsA = horizon_helpers.IsA
IsHttpRequest = horizon_helpers.IsHttpRequest
def create_mocks(target_methods):
def create_mocks(target_methods, stop_mock=True):
"""decorator to simplify setting up multiple mocks at once
:param target_methods: a dict to define methods to be patched using mock.
@ -109,10 +109,14 @@ def create_mocks(target_methods):
self.mock_cinder_tenant_absolute_limits.return_value = ...
...
:param stop_mock: If True (default), mocks started in this decorator will
be stopped. Set this to False only if you cannot stop mocks when exiting
this decorator. The default value, True, should work for most cases.
"""
def wrapper(function):
@wraps(function)
def wrapped(inst, *args, **kwargs):
patchers = []
for target, methods in target_methods.items():
for method in methods:
if isinstance(method, str):
@ -122,8 +126,13 @@ def create_mocks(target_methods):
method_mocked = method[0]
attr_name = method[1]
m = mock.patch.object(target, method_mocked)
patchers.append(m)
setattr(inst, 'mock_%s' % attr_name, m.start())
return function(inst, *args, **kwargs)
retval = function(inst, *args, **kwargs)
if stop_mock:
for m in patchers:
m.stop()
return retval
return wrapped
return wrapper

Loading…
Cancel
Save