From aa1ca195f94f7b4b86e2a77253e5ddb777192f48 Mon Sep 17 00:00:00 2001 From: Akihiro Motoki Date: Sat, 18 Aug 2018 16:56:02 +0900 Subject: [PATCH] Drop use_mox from horizon test helpers We announced in the Rocky cycle that use_mox will be dropped in the beginning of Stein cycle. This commit drops use_mox and cleans up mox related stuffs in horizon and openstack_dashboard codes. Note that openstack_auth is under the mock migration, so we cannot mox3 from test-requirements.txt. blueprint mock-framework-in-unit-tests Change-Id: I189daf2d678bef0fa8219acaab5cf45bbe54178f --- horizon/tables/base.py | 4 +- horizon/test/helpers.py | 25 +- .../dashboards/project/instances/tests.py | 2 +- openstack_dashboard/test/helpers.py | 215 +----------------- .../test/unit/api/test_keystone.py | 9 +- 5 files changed, 15 insertions(+), 240 deletions(-) diff --git a/horizon/tables/base.py b/horizon/tables/base.py index b9c1a8e307..364ab549ca 100644 --- a/horizon/tables/base.py +++ b/horizon/tables/base.py @@ -1322,7 +1322,8 @@ class DataTable(object): # This function should be using django.utils.functional.cached_property # decorator, but unfortunately due to bug in Django # https://code.djangoproject.com/ticket/19872 it would make it fail - # when being mocked by mox in tests. + # when being mocked in tests. + # TODO(amotoki): Check if this trick is still required. if not hasattr(self, '_filtered_data'): self._filtered_data = self.data if self._meta.filter and self._meta._filter_action: @@ -1391,6 +1392,7 @@ class DataTable(object): except AssertionError: # don't trap mox exceptions (which subclass AssertionError) # when testing! + # TODO(amotoki): Check if this trick is still required. raise except Exception: LOG.exception("Error while checking action permissions.") diff --git a/horizon/test/helpers.py b/horizon/test/helpers.py index cb47ae284f..2123426d02 100644 --- a/horizon/test/helpers.py +++ b/horizon/test/helpers.py @@ -63,17 +63,7 @@ except ImportError as e: LOG.warning("%s, force WITH_SELENIUM=False", e) os.environ['WITH_SELENIUM'] = '' -# As of Rocky, we are in the process of removing mox usage. -# To allow mox-free horizon plugins to consume the test helper, -# mox import is now optional. If tests depends on mox, -# mox (or mox3) must be declared in test-requirements.txt. -try: - from mox3 import mox -except ImportError: - pass - - -# Makes output of failing mox tests much easier to read. +# Makes output of failing tests much easier to read. wsgi.WSGIRequest.__repr__ = lambda self: "" @@ -129,23 +119,13 @@ class RequestFactoryWithMessages(RequestFactory): class TestCase(django_test.TestCase): """Base test case class for Horizon with numerous additional features. - * The ``mox`` mocking framework via ``self.mox`` - if ``use_mox`` attribute is set to True. - Note that ``use_mox`` defaults to False. * A ``RequestFactory`` class which supports Django's ``contrib.messages`` framework via ``self.factory``. * A ready-to-go request object via ``self.request``. """ - use_mox = False - def setUp(self): super(TestCase, self).setUp() - if self.use_mox: - LOG.warning("'use_mox' will be dropped at the beginning of " - "'Stein' release. If you still depend on mox, " - "you must prepare mox environment in your test case.") - self.mox = mox.Mox() self._setup_test_data() self._setup_factory() self._setup_user() @@ -174,9 +154,6 @@ class TestCase(django_test.TestCase): def tearDown(self): super(TestCase, self).tearDown() - if self.use_mox: - self.mox.UnsetStubs() - self.mox.VerifyAll() del os.environ["HORIZON_TEST_RUN"] def set_permissions(self, permissions=None): diff --git a/openstack_dashboard/dashboards/project/instances/tests.py b/openstack_dashboard/dashboards/project/instances/tests.py index 2ab3ab13c6..0893513cb3 100644 --- a/openstack_dashboard/dashboards/project/instances/tests.py +++ b/openstack_dashboard/dashboards/project/instances/tests.py @@ -5058,7 +5058,7 @@ class InstanceAjaxTests(helpers.TestCase, InstanceTestHelperMixin): class ConsoleManagerTests(helpers.ResetImageAPIVersionMixin, helpers.TestCase): def setup_consoles(self): - # Need to refresh with mocks or will fail since mox do not detect + # Need to refresh with mocks or will fail since mock do not detect # the api_call() as mocked. console.CONSOLES = collections.OrderedDict([ ('VNC', api.nova.server_vnc_console), diff --git a/openstack_dashboard/test/helpers.py b/openstack_dashboard/test/helpers.py index be73f67c44..5a83e6616d 100644 --- a/openstack_dashboard/test/helpers.py +++ b/openstack_dashboard/test/helpers.py @@ -30,27 +30,12 @@ from django.test import tag from django import urls from django.utils import http -from cinderclient import client as cinder_client -import glanceclient -from keystoneclient.v2_0 import client as keystone_client -# As of Rocky, we are in the process of removing mox usage. -# To allow mox-free horizon plugins to consume the test helper, -# mox import is now optional. If tests depends on mox, -# mox (or mox3) must be declared in test-requirements.txt. import mock -try: - from mox3 import mox -except ImportError: - pass -from neutronclient.v2_0 import client as neutron_client -from novaclient import api_versions as nova_api_versions -from novaclient.v2 import client as nova_client from openstack_auth import user from openstack_auth import utils from requests.packages.urllib3.connection import HTTPConnection import six from six import moves -from swiftclient import client as swift_client from horizon import base from horizon import conf @@ -62,7 +47,7 @@ from openstack_dashboard.test.test_data import utils as test_utils LOG = logging.getLogger(__name__) -# Makes output of failing mox tests much easier to read. +# Makes output of failing tests much easier to read. wsgi.WSGIRequest.__repr__ = lambda self: "" # Shortcuts to avoid importing horizon_helpers and for backward compatibility. @@ -71,61 +56,6 @@ IsA = horizon_helpers.IsA IsHttpRequest = horizon_helpers.IsHttpRequest -def create_stubs(stubs_to_create=None): - """decorator to simplify setting up multiple stubs at once via mox - - :param stubs_to_create: methods to stub in one or more modules - :type stubs_to_create: dict - - The keys are python paths to the module containing the methods to mock. - - To mock a method in openstack_dashboard/api/nova.py, the key is:: - - api.nova - - The values are either a tuple or list of methods to mock in the module - indicated by the key. - - For example:: - - ('server_list',) - -or- - ('flavor_list', 'server_list',) - -or- - ['flavor_list', 'server_list'] - - Additionally, multiple modules can be mocked at once:: - - { - api.nova: ('flavor_list', 'server_list'), - api.glance: ('image_list_detailed',), - } - - """ - if stubs_to_create is None: - stubs_to_create = {} - if not isinstance(stubs_to_create, dict): - raise TypeError("create_stub must be passed a dict, but a %s was " - "given." % type(stubs_to_create).__name__) - - def inner_stub_out(fn): - @wraps(fn) - def instance_stub_out(self, *args, **kwargs): - for key in stubs_to_create: - if not (isinstance(stubs_to_create[key], tuple) or - isinstance(stubs_to_create[key], list)): - raise TypeError("The values of the create_stub " - "dict must be lists or tuples, but " - "is a %s." - % type(stubs_to_create[key]).__name__) - - for value in stubs_to_create[key]: - self.mox.StubOutWithMock(key, value) - return fn(self, *args, **kwargs) - return instance_stub_out - return inner_stub_out - - def create_mocks(target_methods): """decorator to simplify setting up multiple mocks at once @@ -245,8 +175,6 @@ class TestCase(horizon_helpers.TestCase): docs for :class:`~openstack_dashboard.test.test_data.utils.TestData` for more information. - * The ``mox`` mocking framework via ``self.mox``. - if ``use_mox`` attribute is set to True. * A set of request context data via ``self.context``. * A ``RequestFactory`` class which supports Django's ``contrib.messages`` framework via ``self.factory``. @@ -481,146 +409,17 @@ class BaseAdminViewTests(TestCase): class APITestCase(TestCase): - """Testing APIs. - - For use with tests which deal with the underlying clients rather than - stubbing out the openstack_dashboard.api.* methods. - """ - - # NOTE: This test class depends on mox but does not declare use_mox = True - # to notify mox is no longer recommended. - # If a consumer would like to use this class, declare use_mox = True. - def setUp(self): super(APITestCase, self).setUp() - LOG.warning("APITestCase has been deprecated in favor of mock usage " - "and will be removed at the beginning of 'Stein' release. " - "Please convert your to use APIMockTestCase instead.") utils.patch_middleware_get_user() - def fake_keystoneclient(request, admin=False): - """Returns the stub keystoneclient. - Only necessary because the function takes too many arguments to - conveniently be a lambda. - """ - return self.stub_keystoneclient() - - def fake_glanceclient(request, version='1'): - """Returns the stub glanceclient. - - Only necessary because the function takes too many arguments to - conveniently be a lambda. - """ - return self.stub_glanceclient() - - def fake_novaclient(request, version=None): - return self.stub_novaclient() - - # Store the original clients - self._original_glanceclient = api.glance.glanceclient - self._original_keystoneclient = api.keystone.keystoneclient - self._original_novaclient = api.nova.novaclient - self._original_neutronclient = api.neutron.neutronclient - self._original_cinderclient = api.cinder.cinderclient - - # Replace the clients with our stubs. - api.glance.glanceclient = fake_glanceclient - api.keystone.keystoneclient = fake_keystoneclient - api.nova.novaclient = fake_novaclient - api.neutron.neutronclient = lambda request: self.stub_neutronclient() - api.cinder.cinderclient = lambda request: self.stub_cinderclient() - - def tearDown(self): - super(APITestCase, self).tearDown() - api.glance.glanceclient = self._original_glanceclient - api.nova.novaclient = self._original_novaclient - api.keystone.keystoneclient = self._original_keystoneclient - api.neutron.neutronclient = self._original_neutronclient - api.cinder.cinderclient = self._original_cinderclient - - def _warn_client(self, service, removal_version): - LOG.warning( - "APITestCase has been deprecated for %(service)s-related " - "tests and will be removerd in '%(removal_version)s' release. " - "Please convert your to use APIMockTestCase instead.", - {'service': service, 'removal_version': removal_version} - ) - - def stub_novaclient(self): - self._warn_client('nova', 'Stein') - if not hasattr(self, "novaclient"): - self.mox.StubOutWithMock(nova_client, 'Client') - # mock the api_version since MockObject.__init__ ignores it. - # The preferred version in the api.nova code is 2 but that's - # equivalent to 2.1 now and is the base version that's backward - # compatible to 2.0 anyway. - api_version = nova_api_versions.APIVersion('2.1') - nova_client.Client.api_version = api_version - nova_client.Client.projectid = 'fake_project' - nova_client.Client.tenant_id = 'fake_tenant' - self.novaclient = self.mox.CreateMock(nova_client.Client) - return self.novaclient - - def stub_cinderclient(self): - self._warn_client('cinder', 'Stein') - if not hasattr(self, "cinderclient"): - self.mox.StubOutWithMock(cinder_client, 'Client') - self.cinderclient = self.mox.CreateMock(cinder_client.Client) - return self.cinderclient - - def stub_keystoneclient(self): - self._warn_client('keystone', 'Stein') - if not hasattr(self, "keystoneclient"): - self.mox.StubOutWithMock(keystone_client, 'Client') - # NOTE(saschpe): Mock properties, MockObject.__init__ ignores them: - keystone_client.Client.auth_token = 'foo' - keystone_client.Client.service_catalog = None - keystone_client.Client.tenant_id = '1' - keystone_client.Client.tenant_name = 'tenant_1' - keystone_client.Client.management_url = "" - keystone_client.Client.__dir__ = lambda: [] - self.keystoneclient = self.mox.CreateMock(keystone_client.Client) - return self.keystoneclient - - def stub_glanceclient(self): - self._warn_client('glance', 'Stein') - if not hasattr(self, "glanceclient"): - self.mox.StubOutWithMock(glanceclient, 'Client') - self.glanceclient = self.mox.CreateMock(glanceclient.Client) - return self.glanceclient - - def stub_neutronclient(self): - self._warn_client('neutron', 'Stein') - if not hasattr(self, "neutronclient"): - self.mox.StubOutWithMock(neutron_client, 'Client') - self.neutronclient = self.mox.CreateMock(neutron_client.Client) - return self.neutronclient - - def stub_swiftclient(self, expected_calls=1): - self._warn_client('swift', 'Stein') - if not hasattr(self, "swiftclient"): - self.mox.StubOutWithMock(swift_client, 'Connection') - self.swiftclient = self.mox.CreateMock(swift_client.Connection) - while expected_calls: - swift_client.Connection(None, - mox.IgnoreArg(), - None, - preauthtoken=mox.IgnoreArg(), - preauthurl=mox.IgnoreArg(), - cacert=None, - insecure=False, - auth_version="2.0") \ - .AndReturn(self.swiftclient) - expected_calls -= 1 - return self.swiftclient - - -class APIMockTestCase(TestCase): - - def setUp(self): - super(APIMockTestCase, self).setUp() - utils.patch_middleware_get_user() +# APIMockTestCase was introduced to support mox to mock migration smoothly +# but it turns we have still users of APITestCase. +# We keep both for a while. +# Looking at the usage of these classes, it seems better to drop this one. +# TODO(amotoki): Clean up APIMockTestCase usage in horizon plugins. +APIMockTestCase = APITestCase # Need this to test both Glance API V1 and V2 versions diff --git a/openstack_dashboard/test/unit/api/test_keystone.py b/openstack_dashboard/test/unit/api/test_keystone.py index 91e41935b6..65f58c5ef2 100644 --- a/openstack_dashboard/test/unit/api/test_keystone.py +++ b/openstack_dashboard/test/unit/api/test_keystone.py @@ -37,9 +37,6 @@ class RoleAPITests(test.APIMockTestCase): Verifies that remove_tenant_user is called with the right arguments after iterating the user's roles. - - There are no assertions in this test because the checking is handled - by mox in the VerifyAll() call in tearDown(). """ keystoneclient = mock_keystoneclient.return_value tenant = self.tenants.first() @@ -67,10 +64,10 @@ class RoleAPITests(test.APIMockTestCase): role = api.keystone.get_default_role(self.request) self.assertEqual(self.role, role) - # Verify that a second call doesn't hit the API again, - # (it would show up in mox as an unexpected method call) - role = api.keystone.get_default_role(self.request) + # Verify that a second call doesn't hit the API again, + # so we use assert_called_once_with() here. + role = api.keystone.get_default_role(self.request) keystoneclient.roles.list.assert_called_once_with()