Change in vdsm[ovirt-3.5]: tests: Fix RescanTimeoutsTests

ahino at redhat.com ahino at redhat.com
Tue Apr 7 13:38:25 UTC 2015


Hello Nir Soffer, Dan Kenigsberg, Francesco Romani,

I'd like you to do a code review.  Please visit

    https://gerrit.ovirt.org/39609

to review the following change.

Change subject: tests: Fix RescanTimeoutsTests
......................................................................

tests: Fix RescanTimeoutsTests

In commit 4c10d3d10c (multipath: Wait for udev events after rescan) I
removed unneeded and badly implemented minimal timeout, and the unneeded
optional arguments in iscsi.rescan() but forgot to update the tests.

In this patch:

- remove the now unneeded and broken test for minimal timeout
  (the test was passing while the code was broken)
- simplify the testing infrastructure
- instead of passing arguments to rescan, monkey-patch the config.
- use safe monkey-patching.
- use monotonic_time
- use real process so we get SIGCHLD when the scan is finished;
  otherwise, the tests may pass while the code is broken
- add test for normal operation

make_config and monotonic_time defined locally in the test because they
don't exist on ovirt 3.5 branch.

Change-Id: I308c73eb7059053aa91bf15957cf9029234b37d1
Signed-off-by: Nir Soffer <nsoffer at redhat.com>
Signed-off-by: Ala Hino <ahino at redhat.com>
Reviewed-on: https://gerrit.ovirt.org/39422
Reviewed-by: Francesco Romani <fromani at redhat.com>
Reviewed-by: Dan Kenigsberg <danken at redhat.com>
---
M tests/iscsiTests.py
1 file changed, 30 insertions(+), 56 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/09/39609/1

diff --git a/tests/iscsiTests.py b/tests/iscsiTests.py
index 6f384b4..27e57bb 100644
--- a/tests/iscsiTests.py
+++ b/tests/iscsiTests.py
@@ -1,84 +1,58 @@
+import ConfigParser
 import os
-import threading
-import time
 from contextlib import contextmanager
 
+from monkeypatch import MonkeyPatch
 from testrunner import VdsmTestCase as TestCaseBase
 
+from vdsm import config
+from vdsm import utils
+
 from storage import iscsi
+from storage import iscsiadm
 
 
-class AsyncStubOperation(object):
-    def __init__(self, timeout):
-        self._evt = threading.Event()
-        if timeout == 0:
-            self._evt.set()
-        else:
-            threading.Timer(timeout, self._evt.set)
+def make_config(timeout='30'):
+    cfg = ConfigParser.ConfigParser()
+    config.set_defaults(cfg)
+    cfg.set('irs', 'scsi_rescan_maximal_timeout', timeout)
+    return cfg
 
-    def wait(self, timeout=None, cond=None):
-        if cond is not None:
-            raise Exception("TODO!!!")
 
-        self._evt.wait(timeout)
+def fake_rescan(timeout):
+    def func():
+        proc = utils.execCmd(["sleep", str(timeout)], sync=False)
+        return utils.AsyncProcessOperation(proc)
+    return func
 
-    def stop(self):
-        self._evt.set()
 
-    def result(self):
-        if self._evt.is_set():
-            return (None, None)
-        else:
-            return None
+def monotonic_time():
+    return os.times()[4]
 
 
 class RescanTimeoutTests(TestCaseBase):
-    def setUp(self):
-        self._iscsiadm_rescan_async = \
-            iscsi.iscsiadm.session_rescan_async
-        iscsi.iscsiadm.session_rescan_async = self._iscsi_stub
-        self._timeout = 0
-
-    def tearDown(self):
-        iscsi.iscsiadm.session_rescan_async = \
-            self._iscsiadm_rescan_async
-
-    def _iscsi_stub(self):
-        return AsyncStubOperation(self._timeout)
 
     @contextmanager
     def assertMaxDuration(self, maxtime):
-        start = time.time()
+        start = monotonic_time()
         try:
             yield
         finally:
-            end = time.time()
-            elapsed = end - start
+            elapsed = monotonic_time() - start
             if maxtime < elapsed:
-                self.fail("Operation was too slow %fs > %fs" %
+                self.fail("Operation was too slow %.2fs > %.2fs" %
                           (elapsed, maxtime))
 
-    @contextmanager
-    def assertMinDuration(self, mintime):
-        start = time.time()
-        try:
-            yield
-        finally:
-            end = time.time()
-            elapsed = end - start
-            if mintime > elapsed:
-                self.fail("Operation was too fast %fs > %fs" %
-                          (elapsed, mintime))
+    @MonkeyPatch(iscsiadm, 'session_rescan_async', fake_rescan(0.1))
+    def testWait(self):
+        with self.assertMaxDuration(0.3):
+            iscsi.rescan()
 
-    def testFast(self):
-        self._timeout = 0
-        with self.assertMinDuration(2):
-            iscsi.rescan(2, 4)
-
-    def testSlow(self):
-        self._timeout = 60
-        with self.assertMaxDuration(3):
-            iscsi.rescan(1, 2)
+    @MonkeyPatch(iscsiadm, 'session_rescan_async', fake_rescan(2))
+    @MonkeyPatch(iscsi, 'config', make_config(timeout="1"))
+    def testTimeout(self):
+        with self.assertMaxDuration(1.2):
+            iscsi.rescan()
 
 
 class IscsiAdmTests(TestCaseBase):


-- 
To view, visit https://gerrit.ovirt.org/39609
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I308c73eb7059053aa91bf15957cf9029234b37d1
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.5
Gerrit-Owner: Ala Hino <ahino at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>


More information about the vdsm-patches mailing list