From 7bfdb6a704855984ae35a1c6ef063782a4f7bf1d Mon Sep 17 00:00:00 2001 From: Roman Podoliaka Date: Wed, 25 Feb 2015 15:38:50 +0200 Subject: [PATCH] Make DBAPI class work with mocks correctly Currently, DBAPI __getattr__() implementation will *always* implicitly wrap all mocked methods of a target class/module using wrap_db_retry(), as retry_on_* attributes will always exist and be true for mock instances. Not only this leads to a situation, when we would retry methods, we were not going to retry, but this also fails with AttributeError on functools.wraps() call, breaking valid unit tests in projects. DBAPI is changed in a way, so that it does dict look ups instead of fetching instance attributes to ensure mocked methods are handled correctly and aren't wrapped when they shouldn't be wrapped. Related issue: http://stackoverflow.com/questions/22204660/python-mock-wrapsf-problems Change-Id: I36550ffa88056c53a431d64e61e84fbb44bbf68d --- oslo_db/api.py | 15 ++++++++------- oslo_db/tests/test_api.py | 8 ++++++++ 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/oslo_db/api.py b/oslo_db/api.py index 98e4d755..616cac50 100644 --- a/oslo_db/api.py +++ b/oslo_db/api.py @@ -56,7 +56,7 @@ def safe_for_db_retry(f): :param f: database api method. :type f: function. """ - f.enable_retry_on_disconnect = True + f.__dict__['enable_retry_on_disconnect'] = True return f @@ -66,7 +66,7 @@ def retry_on_deadlock(f): wrap_db_entry will be applied to all db.api functions marked with this decorator. """ - f.enable_retry_on_deadlock = True + f.__dict__['enable_retry_on_deadlock'] = True return f @@ -76,7 +76,7 @@ def retry_on_request(f): wrap_db_entry will be applied to all db.api functions marked with this decorator. """ - f.enable_retry_on_request = True + f.__dict__['enable_retry_on_request'] = True return f @@ -225,10 +225,11 @@ class DBAPI(object): # NOTE(vsergeyev): If `use_db_reconnect` option is set to True, retry # DB API methods, decorated with @safe_for_db_retry # on disconnect. - retry_on_disconnect = self.use_db_reconnect and getattr( - attr, 'enable_retry_on_disconnect', False) - retry_on_deadlock = getattr(attr, 'enable_retry_on_deadlock', False) - retry_on_request = getattr(attr, 'enable_retry_on_request', False) + retry_on_disconnect = self.use_db_reconnect and attr.__dict__.get( + 'enable_retry_on_disconnect', False) + retry_on_deadlock = attr.__dict__.get('enable_retry_on_deadlock', + False) + retry_on_request = attr.__dict__.get('enable_retry_on_request', False) if retry_on_disconnect or retry_on_deadlock or retry_on_request: attr = wrap_db_retry( diff --git a/oslo_db/tests/test_api.py b/oslo_db/tests/test_api.py index 18dc586f..c784afe1 100644 --- a/oslo_db/tests/test_api.py +++ b/oslo_db/tests/test_api.py @@ -196,3 +196,11 @@ class DBRetryRequestCase(DBAPITestCase): res = {'result': 0} self.assertRaises(ValueError, some_method, res) self.assertEqual(max_retries + 1, res['result']) + + @mock.patch.object(DBAPI, 'api_class_call1') + @mock.patch.object(api, 'wrap_db_retry') + def test_mocked_methods_are_not_wrapped(self, mocked_wrap, mocked_method): + dbapi = api.DBAPI('oslo_db.tests.test_api') + dbapi.api_class_call1() + + self.assertFalse(mocked_wrap.called)