From 0b6c975fe5c9e1cb2c3289c876410bcf3eaf6ef7 Mon Sep 17 00:00:00 2001 From: Federico Ressi Date: Tue, 4 Aug 2020 11:22:24 +0200 Subject: [PATCH] Improve retry APIs - add retry_test_case decorator - make typing more explicit - add support for default retry parameters Change-Id: I0a5feb9f826416e45cc51bebf467e576c91ebb29 --- tobiko/__init__.py | 1 + tobiko/common/_retry.py | 131 ++++++++++++++++++++++++------ tobiko/openstack/nova/_service.py | 12 +-- tobiko/tests/unit/test_retry.py | 111 +++++++++++++++++++++++++ 4 files changed, 227 insertions(+), 28 deletions(-) diff --git a/tobiko/__init__.py b/tobiko/__init__.py index 99473422e..a73bc5150 100644 --- a/tobiko/__init__.py +++ b/tobiko/__init__.py @@ -87,6 +87,7 @@ retry = _retry.retry Retry = _retry.Retry retry_attempt = _retry.retry_attempt retry_on_exception = _retry.retry_on_exception +retry_test_case = _retry.retry_test_case RetryAttempt = _retry.RetryAttempt RetryCountLimitError = _retry.RetryCountLimitError RetryLimitError = _retry.RetryLimitError diff --git a/tobiko/common/_retry.py b/tobiko/common/_retry.py index 6d8589600..a4855a82f 100644 --- a/tobiko/common/_retry.py +++ b/tobiko/common/_retry.py @@ -13,13 +13,13 @@ # under the License. from __future__ import absolute_import -import collections import functools import itertools import typing from oslo_log import log +from tobiko.common import _asserts from tobiko.common import _exception from tobiko.common import _time @@ -43,10 +43,33 @@ class RetryTimeLimitError(RetryLimitError): message = ("Retry time limit exceeded ({attempt.details})") -class RetryAttempt( - collections.namedtuple('RetryAttempt', ['number', 'count', - 'start_time', 'elapsed_time', - 'timeout', 'interval'])): +class RetryAttempt(object): + + def __init__(self, + number: int, + start_time: float, + elapsed_time: float, + count: typing.Optional[int] = None, + timeout: _time.Seconds = None, + interval: _time.Seconds = None): + self.number = number + self.start_time = start_time + self.elapsed_time = elapsed_time + self.count = count + self.timeout = _time.to_seconds(timeout) + self.interval = _time.to_seconds(interval) + + def __eq__(self, other): + return (other.number == self.number and + other.start_time == self.start_time and + other.elapsed_time == self.elapsed_time and + other.count == self.count and + other.timeout == self.timeout and + other.interval == self.interval) + + def __hash__(self): + raise NotImplementedError + @property def count_left(self) -> typing.Optional[int]: if self.count is None: @@ -109,10 +132,6 @@ def retry_attempt(number: int, class Retry(object): - count: typing.Optional[int] = None - timeout: _time.Seconds = None - interval: _time.Seconds = None - def __init__(self, count: typing.Optional[int] = None, timeout: _time.Seconds = None, @@ -121,6 +140,14 @@ class Retry(object): self.timeout = _time.to_seconds(timeout) self.interval = _time.to_seconds(interval) + def __eq__(self, other): + return (other.count == self.count and + other.timeout == self.timeout and + other.interval == self.interval) + + def __hash__(self): + raise NotImplementedError + def __iter__(self) -> typing.Iterator[RetryAttempt]: start_time = _time.time() elapsed_time = 0. @@ -149,42 +176,100 @@ class Retry(object): _time.sleep(sleep_time) elapsed_time = _time.time() - start_time + @property + def details(self) -> str: + details = [] + if self.count is not None: + details.append(f"count={self.count}") + if self.timeout is not None: + details.append(f"timeout={self.timeout}") + if self.interval is not None: + details.append(f"interval={self.interval}") + return ', '.join(details) -def retry(other: typing.Optional[Retry] = None, + def __repr__(self): + return f"retry({self.details})" + + +def retry(other_retry: typing.Optional[Retry] = None, count: typing.Optional[int] = None, timeout: _time.Seconds = None, - interval: _time.Seconds = None) -> Retry: - if other is not None: - _exception.check_valid_type(other, Retry) - count = count or other.count - timeout = timeout or other.timeout - interval = interval or other.interval + interval: _time.Seconds = None, + default_count: typing.Optional[int] = None, + default_timeout: _time.Seconds = None, + default_interval: _time.Seconds = None) -> Retry: + + if other_retry is not None: + # Apply default values from the other Retry object + _exception.check_valid_type(other_retry, Retry) + count = count or other_retry.count + timeout = timeout or other_retry.timeout + interval = interval or other_retry.interval + + # Apply default values + count = count or default_count + timeout = timeout or default_timeout + interval = interval or default_interval return Retry(count=count, timeout=timeout, interval=interval) def retry_on_exception(exception: Exception, - *exceptions: typing.Tuple[Exception], + *exceptions: Exception, + other_retry: typing.Optional[Retry] = None, count: typing.Optional[int] = None, timeout: _time.Seconds = None, - interval: _time.Seconds = None): + interval: _time.Seconds = None, + default_count: typing.Optional[int] = None, + default_timeout: _time.Seconds = None, + default_interval: _time.Seconds = None) -> \ + typing.Callable[[typing.Callable], typing.Callable]: - failures = (exception,) + exceptions + retry_object = retry(other_retry=other_retry, + count=count, + timeout=timeout, + interval=interval, + default_count=default_count, + default_timeout=default_timeout, + default_interval=default_interval) + exceptions = (exception,) + exceptions def decorator(func): if typing.TYPE_CHECKING: + # Don't neet to wrap the function when going to check argument + # types return func @functools.wraps(func) def wrapper(*args, **kwargs): # pylint: disable=catching-non-exception - for attempt in retry(count=count, - timeout=timeout, - interval=interval): + for attempt in retry_object: try: return func(*args, **kwargs) - except failures: + except exceptions: attempt.check_limits() return wrapper return decorator + + +def retry_test_case(*exceptions: Exception, + other_retry: typing.Optional[Retry] = None, + count: typing.Optional[int] = None, + timeout: _time.Seconds = None, + interval: _time.Seconds = None, + default_count: typing.Optional[int] = None, + default_timeout: _time.Seconds = None, + default_interval: _time.Seconds = None) \ + -> typing.Callable[[typing.Callable], typing.Callable]: + """Re-run test case method in case it fails + """ + exceptions = exceptions or (_asserts.FailureException,) + return retry_on_exception(*exceptions, + other_retry=other_retry, + count=count, + timeout=timeout, + interval=interval, + default_count=default_count or 3, + default_timeout=default_timeout or 30., + default_interval=default_interval) diff --git a/tobiko/openstack/nova/_service.py b/tobiko/openstack/nova/_service.py index 1d5eb8a9e..70db2c1c6 100644 --- a/tobiko/openstack/nova/_service.py +++ b/tobiko/openstack/nova/_service.py @@ -44,14 +44,16 @@ def services_details(services: typing.List): def wait_for_services_up(retry: typing.Optional[tobiko.Retry] = None, - **kwargs): - retry = retry or tobiko.retry(timeout=30., interval=5.) - for attempt in retry: - services = _client.list_services(**kwargs) + **list_services_params): + for attempt in tobiko.retry(other_retry=retry, + default_timeout=30., + default_interval=5.): + services = _client.list_services(**list_services_params) LOG.debug(f"Found {len(services)} Nova services") try: if not services: - raise NovaServicesNotfound(attributes=json.dumps(kwargs)) + raise NovaServicesNotfound( + attributes=json.dumps(list_services_params)) heathy_services = services.with_attributes(state='up') LOG.debug(f"Found {len(heathy_services)} healthy Nova services") diff --git a/tobiko/tests/unit/test_retry.py b/tobiko/tests/unit/test_retry.py index 326276209..321a8152c 100644 --- a/tobiko/tests/unit/test_retry.py +++ b/tobiko/tests/unit/test_retry.py @@ -16,6 +16,7 @@ from __future__ import absolute_import import itertools import mock +import testtools import tobiko from tobiko.tests import unit @@ -247,3 +248,113 @@ class RetryTest(unit.TobikoUnitTest): mock_time.sleep.assert_has_calls([mock.call(4.), mock.call(3.), mock.call(3.)]) + + def test_retry_test_case_when_succeed(self): + + class MyTest(testtools.TestCase): + + @tobiko.retry_test_case() + def test_success(self): + pass + + result = testtools.TestResult() + test_case = MyTest('test_success') + test_case.run(result) + + self.assertEqual(1, result.testsRun) + self.assertEqual([], result.failures) + self.assertEqual([], result.errors) + self.assertEqual({}, result.skip_reasons) + + def test_retry_test_case_when_fails(self): + + class MyTest(testtools.TestCase): + + @tobiko.retry_test_case() + def test_failure(self): + try: + self.fail("this is failing") + except tobiko.FailureException as ex: + failures.append(ex) + raise + + failures = [] + result = testtools.TestResult() + test_case = MyTest('test_failure') + test_case.run(result) + + self.assertEqual(1, result.testsRun) + self.assertEqual([], result.errors) + self.assertEqual({}, result.skip_reasons) + + self.assertEqual(3, len(failures)) + self.assertEqual(1, len(result.failures)) + failed_test_case, traceback = result.failures[0] + self.assertIs(test_case, failed_test_case) + self.assertIn(str(failures[-1]), traceback) + + def test_retry_test_case_when_fails_once(self): + + class MyTest(testtools.TestCase): + + @tobiko.retry_test_case() + def test_one_failure(self): + count = next(count_calls) + self.assertNotEqual(0, count) + + count_calls = itertools.count() + result = testtools.TestResult() + test_case = MyTest('test_one_failure') + test_case.run(result) + + self.assertEqual(2, next(count_calls)) + self.assertEqual(1, result.testsRun) + self.assertEqual([], result.failures) + self.assertEqual([], result.errors) + self.assertEqual({}, result.skip_reasons) + + def test_retry_test_case_when_raises_errors(self): + + class MyTest(testtools.TestCase): + + @tobiko.retry_test_case() + def test_errors(self): + ex = ValueError('pippo') + errors.append(ex) + raise ex + + errors = [] + result = testtools.TestResult() + test_case = MyTest('test_errors') + test_case.run(result) + + self.assertEqual(1, result.testsRun) + self.assertEqual([], result.failures) + self.assertEqual({}, result.skip_reasons) + + self.assertEqual(1, len(errors)) + self.assertEqual(1, len(result.errors)) + failed_test_case, traceback = result.errors[0] + self.assertIs(test_case, failed_test_case) + self.assertIn(str(errors[-1]), traceback) + + def test_retry_test_case_when_skip(self): + + class MyTest(testtools.TestCase): + + @tobiko.retry_test_case() + def test_skip(self): + next(count_calls) + self.skip("Not the right day!") + + count_calls = itertools.count() + result = testtools.TestResult() + test_case = MyTest('test_skip') + test_case.run(result) + + self.assertEqual(1, next(count_calls)) + self.assertEqual(1, result.testsRun) + self.assertEqual([], result.failures) + self.assertEqual([], result.errors) + self.assertEqual({"Not the right day!": [test_case]}, + result.skip_reasons)