On Thu, Sep 19, 2013 at 6:21 PM, Klaus Aehlig <[email protected]> wrote:
> Even in the tests, real time is used. While, generally, the assumptions > about execution time are pretty safe, in some rare circumstances, e.g., > on machines with extremely heavy load they do not hold true, thus rendering > the tests flaky. Fix this, by mocking time. > > Signed-off-by: Klaus Aehlig <[email protected]> > --- > test/py/ganeti.utils.retry_unittest.py | 54 > ++++++++++++++++++++++++---------- > 1 file changed, 38 insertions(+), 16 deletions(-) > > diff --git a/test/py/ganeti.utils.retry_unittest.py b/test/py/ > ganeti.utils.retry_unittest.py > index 42e43b4..f9d45a4 100755 > --- a/test/py/ganeti.utils.retry_unittest.py > +++ b/test/py/ganeti.utils.retry_unittest.py > @@ -35,6 +35,13 @@ class TestRetry(testutils.GanetiTestCase): > testutils.GanetiTestCase.setUp(self) > self.retries = 0 > self.called = 0 > + self.time = 1379601882.0 > + > + def _time_fn(self): > + return self.time > + > + def _wait_fn(self, delay): > + self.time += delay > Only advancing time if _wait_fn is called is an oversimplification. Actually, it's a corner case that two consecutive invocations of _time_fn return the same value, but here that's the default. Calling _time_fn should also increase self.time, and there should be a test introduced which tests for the special case that the time resolution between two calls of _time_fn is too small to return a different value. Additionally, it would be possible to make those test (and the production code) cleaner by using Python Mock. With that you could just replace the implementation of time.time() and time.sleep(), thus avoiding to have the two additional named parameters _wait_fn and _time_fn (see ganeti.storage.drbd_unittest.py:429 for a related example). > > @staticmethod > def _RaiseRetryAgain(): > @@ -64,28 +71,37 @@ class TestRetry(testutils.GanetiTestCase): > > def testRaiseTimeout(self): > self.failUnlessRaises(utils.RetryTimeout, utils.Retry, > - self._RaiseRetryAgain, 0.01, 0.02) > + self._RaiseRetryAgain, 0.01, 0.02, > + wait_fn = self._wait_fn, _time_fn = > self._time_fn) > self.failUnlessRaises(utils.RetryTimeout, utils.Retry, > - self._RetryAndSucceed, 0.01, 0, args=[1]) > + self._RetryAndSucceed, 0.01, 0, args=[1], > + wait_fn = self._wait_fn, _time_fn = > self._time_fn) > self.failUnlessEqual(self.retries, 1) > > def testComplete(self): > - self.failUnlessEqual(utils.Retry(lambda: True, 0, 1), True) > - self.failUnlessEqual(utils.Retry(self._RetryAndSucceed, 0, 1, > args=[2]), > + self.failUnlessEqual(utils.Retry(lambda: True, 0, 1, > + wait_fn = self._wait_fn, > + _time_fn = self._time_fn), > + True) > + self.failUnlessEqual(utils.Retry(self._RetryAndSucceed, 0, 1, > args=[2], > + wait_fn = self._wait_fn, > + _time_fn = self._time_fn), > True) > self.failUnlessEqual(self.retries, 2) > > def testNestedLoop(self): > try: > self.failUnlessRaises(errors.ProgrammerError, utils.Retry, > - self._WrongNestedLoop, 0, 1) > + self._WrongNestedLoop, 0, 1, > + wait_fn = self._wait_fn, _time_fn = > self._time_fn) > except utils.RetryTimeout: > self.fail("Didn't detect inner loop's exception") > > def testTimeoutArgument(self): > retry_arg="my_important_debugging_message" > try: > - utils.Retry(self._RaiseRetryAgainWithArg, 0.01, 0.02, > args=[[retry_arg]]) > + utils.Retry(self._RaiseRetryAgainWithArg, 0.01, 0.02, > args=[[retry_arg]], > + wait_fn = self._wait_fn, _time_fn = self._time_fn) > except utils.RetryTimeout, err: > self.failUnlessEqual(err.args, (retry_arg, )) > else: > @@ -96,7 +112,8 @@ class TestRetry(testutils.GanetiTestCase): > try: > try: > utils.Retry(self._RaiseRetryAgainWithArg, 0.01, 0.02, > - args=[[errors.GenericError(retry_arg, retry_arg)]]) > + args=[[errors.GenericError(retry_arg, retry_arg)]], > + wait_fn = self._wait_fn, _time_fn = self._time_fn) > except utils.RetryTimeout, err: > err.RaiseInner() > else: > @@ -111,7 +128,8 @@ class TestRetry(testutils.GanetiTestCase): > try: > try: > utils.Retry(self._RaiseRetryAgainWithArg, 0.01, 0.02, > - args=[[retry_arg, retry_arg]]) > + args=[[retry_arg, retry_arg]], > + wait_fn = self._wait_fn, _time_fn = self._time_fn) > except utils.RetryTimeout, err: > err.RaiseInner() > else: > @@ -122,17 +140,21 @@ class TestRetry(testutils.GanetiTestCase): > self.fail("Expected RetryTimeout didn't happen") > > def testSimpleRetry(self): > - self.assertFalse(utils.SimpleRetry(True, lambda: False, 0.01, 0.02)) > - self.assertFalse(utils.SimpleRetry(lambda x: x, lambda: False, 0.01, > 0.02)) > - self.assertTrue(utils.SimpleRetry(True, lambda: True, 0, 1)) > - self.assertTrue(utils.SimpleRetry(lambda x: x, lambda: True, 0, 1)) > - self.assertTrue(utils.SimpleRetry(True, self._SimpleRetryAndSucceed, > - 0, 1, args=[1])) > + self.assertFalse(utils.SimpleRetry(True, lambda: False, 0.01, 0.02, > + wait_fn = self._wait_fn, _time_fn > = self._time_fn)) > + self.assertFalse(utils.SimpleRetry(lambda x: x, lambda: False, 0.01, > 0.02, > + wait_fn = self._wait_fn, _time_fn > = self._time_fn)) > + self.assertTrue(utils.SimpleRetry(True, lambda: True, 0, 1, > + wait_fn = self._wait_fn, _time_fn = > self._time_fn)) > + self.assertTrue(utils.SimpleRetry(lambda x: x, lambda: True, 0, 1, > + wait_fn = self._wait_fn, _time_fn = > self._time_fn)) > + self.assertTrue(utils.SimpleRetry(True, self._SimpleRetryAndSucceed, > 0, 1, args=[1], > + wait_fn = self._wait_fn, _time_fn = > self._time_fn)) > self.assertEqual(self.retries, 1) > self.assertEqual(self.called, 2) > self.called = self.retries = 0 > - self.assertTrue(utils.SimpleRetry(True, self._SimpleRetryAndSucceed, > - 0, 1, args=[2])) > + self.assertTrue(utils.SimpleRetry(True, self._SimpleRetryAndSucceed, > 0, 1, args=[2], > + wait_fn = self._wait_fn, _time_fn = > self._time_fn)) > self.assertEqual(self.called, 3) > > > -- > 1.8.4 > > -- Thomas Thrainer | Software Engineer | [email protected] | Google Germany GmbH Dienerstr. 12 80331 München Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg Geschäftsführer: Graham Law, Christine Elizabeth Flores
