Change in vdsm[master]: vdsm: refactored new functional tests

Yoav Kleinberger has uploaded a new change for review.

Change subject: vdsm: refactored new functional tests
......................................................................

vdsm: refactored new functional tests

Seprarated the storage backend and verifier base classes into two
separate modules.

Change-Id: Ie9dbe640943f86fcb2415c25498f663304653b3a
Signed-off-by: Yoav Kleinberger <ykleinbe(a)redhat.com&gt;
---
D tests/functional/testlib/storagecontexts/base.py
A tests/functional/testlib/storagecontexts/base/__init__.py
A tests/functional/testlib/storagecontexts/base/storagebackend.py
A tests/functional/testlib/storagecontexts/base/verify.py
M tests/functional/testlib/storagecontexts/filebased.py
M tests/functional/testlib/storagecontexts/iscsi.py
6 files changed, 150 insertions(+), 147 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/42/34242/1

diff --git a/tests/functional/testlib/storagecontexts/base.py
b/tests/functional/testlib/storagecontexts/base.py
deleted file mode 100644
index 2e308ad..0000000
--- a/tests/functional/testlib/storagecontexts/base.py
+++ /dev/null
@@ -1,140 +0,0 @@
-import logging
-import os
-import random
-import time
-import uuid
-from functional.testlib import vdsmcaller
-
-
-class Verify(object):
-    def __init__(self, vdsm):
-        self.vdsm = vdsm
-
-    def sleepWhileVDSMCompletesTask(self, duration):
-        time.sleep(duration)
-
-    def storagePoolCreated(self, poolID, masterDomainID):
-        self.sleepWhileVDSMCompletesTask(duration=1)
-        linkToDomain =\
-            os.path.join('/rhev/data-center', poolID, masterDomainID)
-        linkToMasterDomain =\
-            os.path.join('/rhev/data-center', poolID, 'mastersd')
-        assert os.path.lexists(linkToDomain)
-        assert os.path.lexists(linkToMasterDomain)
-
-    def waitFor(self, seconds, description, predicate, *args, **kwargs):
-        logging.info('waiting for "%s"' % description)
-        start = time.time()
-        for _ in xrange(seconds):
-            if predicate(*args, **kwargs):
-                logging.info('it took %0.3f seconds' % (time.time() - start))
-                return
-            time.sleep(1)
-
-        MESSAGE = 'waited %s seconds for "%s" but it did not happen'
%\
-                  (timeout, description)
-        assert False, MESSAGE
-
-    def spmStarted(self, poolID):
-        self.sleepWhileVDSMCompletesTask(duration=1)
-        masterDomainDirectory = '/rhev/data-center/%s/mastersd' % poolID
-        master = os.path.join(masterDomainDirectory, 'master')
-        tasks = os.path.join(master, 'tasks')
-        vms = os.path.join(master, 'vms')
-        self.waitFor(
-            60,
-            'SPM related subdirectories exist',
-            self._allExist,
-            [master, tasks, vms])
-
-    def _allExist(self, paths):
-        result = True
-        for path in paths:
-            result = result and os.path.exists(path)
-        return result
-
-    def waitUntilVDSMTaskFinished(self, taskID, timeout):
-        self.waitFor(
-            timeout,
-            'vdsm task to be finished',
-            self._taskFinished,
-            taskID)
-        taskStatus = self._taskStatus(taskID)
-        assert taskStatus['code'] == 0, taskStatus['message']
-
-    def _taskFinished(self, taskID):
-        return self._taskStatus(taskID)['taskState'] == 'finished'
-
-    def _taskStatus(self, taskID):
-        result = self.vdsm().getTaskStatus(taskID)
-        return result['taskStatus']
-
-
-class StorageBackend(object):
-    def __init__(self):
-        self._vdsmCaller = vdsmcaller.VDSMCaller()
-        self._domainID = self._newUUID()
-        self._poolID = self._newUUID()
-        self._imageID = self._newUUID()
-        self._volumeID = self._newUUID()
-        self._connectionID = self._newUUID()
-
-    def connectionID(self):
-        return self._connectionID
-
-    def volumeID(self):
-        return self._volumeID
-
-    def imageID(self):
-        return self._imageID
-
-    def poolID(self):
-        return self._poolID
-
-    def domainID(self):
-        return self._domainID
-
-    def vdsm(self):
-        return self._vdsmCaller
-
-    def _newUUID(self):
-        return str(uuid.uuid4())
-
-    def randomName(self, base):
-        return "%s_%04d" % (base, random.randint(1, 10000))
-
-    def createStoragePool(self):
-        POOL_TYPE_DEPRECATED = 0
-        self.vdsm().createStoragePool(
-            POOL_TYPE_DEPRECATED,
-            self.poolID(),
-            self.randomName('pool'),
-            self.domainID(),
-            [self.domainID()],
-            1)
-        return self.poolID()
-
-    def connectStoragePool(self, poolID, masterDomainID):
-        SCSI_KEY_DEPRECATED = 0
-        self.vdsm().connectStoragePool(
-            poolID,
-            1,
-            SCSI_KEY_DEPRECATED,
-            masterDomainID,
-            1)
-
-    def spmStart(self, poolID):
-        RECOVERY_MODE_DEPRECATED = 0
-        SCSI_FENCING_DEPRECATED = 0
-        self.vdsm().spmStart(
-            poolID,
-            -1,
-            '-1',
-            SCSI_FENCING_DEPRECATED,
-            RECOVERY_MODE_DEPRECATED)
-
-    def activateStorageDomain(self, domainID, poolID):
-        self.vdsm().activateStorageDomain(domainID, poolID)
-
-    def largeIntegerXMLRPCWorkaround(self, number):
-        return str(number)
diff --git a/tests/functional/testlib/storagecontexts/base/__init__.py
b/tests/functional/testlib/storagecontexts/base/__init__.py
new file mode 100644
index 0000000..e69de29
--- /dev/null
+++ b/tests/functional/testlib/storagecontexts/base/__init__.py
diff --git a/tests/functional/testlib/storagecontexts/base/storagebackend.py
b/tests/functional/testlib/storagecontexts/base/storagebackend.py
new file mode 100644
index 0000000..9fad2e9
--- /dev/null
+++ b/tests/functional/testlib/storagecontexts/base/storagebackend.py
@@ -0,0 +1,73 @@
+import random
+import uuid
+from functional.testlib import vdsmcaller
+
+
+class StorageBackend(object):
+    def __init__(self):
+        self._vdsmCaller = vdsmcaller.VDSMCaller()
+        self._domainID = self._newUUID()
+        self._poolID = self._newUUID()
+        self._imageID = self._newUUID()
+        self._volumeID = self._newUUID()
+        self._connectionID = self._newUUID()
+
+    def connectionID(self):
+        return self._connectionID
+
+    def volumeID(self):
+        return self._volumeID
+
+    def imageID(self):
+        return self._imageID
+
+    def poolID(self):
+        return self._poolID
+
+    def domainID(self):
+        return self._domainID
+
+    def vdsm(self):
+        return self._vdsmCaller
+
+    def _newUUID(self):
+        return str(uuid.uuid4())
+
+    def randomName(self, base):
+        return "%s_%04d" % (base, random.randint(1, 10000))
+
+    def createStoragePool(self):
+        POOL_TYPE_DEPRECATED = 0
+        self.vdsm().createStoragePool(
+            POOL_TYPE_DEPRECATED,
+            self.poolID(),
+            self.randomName('pool'),
+            self.domainID(),
+            [self.domainID()],
+            1)
+        return self.poolID()
+
+    def connectStoragePool(self, poolID, masterDomainID):
+        SCSI_KEY_DEPRECATED = 0
+        self.vdsm().connectStoragePool(
+            poolID,
+            1,
+            SCSI_KEY_DEPRECATED,
+            masterDomainID,
+            1)
+
+    def spmStart(self, poolID):
+        RECOVERY_MODE_DEPRECATED = 0
+        SCSI_FENCING_DEPRECATED = 0
+        self.vdsm().spmStart(
+            poolID,
+            -1,
+            '-1',
+            SCSI_FENCING_DEPRECATED,
+            RECOVERY_MODE_DEPRECATED)
+
+    def activateStorageDomain(self, domainID, poolID):
+        self.vdsm().activateStorageDomain(domainID, poolID)
+
+    def largeIntegerXMLRPCWorkaround(self, number):
+        return str(number)
diff --git a/tests/functional/testlib/storagecontexts/base/verify.py
b/tests/functional/testlib/storagecontexts/base/verify.py
new file mode 100644
index 0000000..0ecc956
--- /dev/null
+++ b/tests/functional/testlib/storagecontexts/base/verify.py
@@ -0,0 +1,67 @@
+import logging
+import os
+import time
+
+
+class Verify(object):
+    def __init__(self, vdsm):
+        self.vdsm = vdsm
+
+    def sleepWhileVDSMCompletesTask(self, duration):
+        time.sleep(duration)
+
+    def storagePoolCreated(self, poolID, masterDomainID):
+        self.sleepWhileVDSMCompletesTask(duration=1)
+        linkToDomain =\
+            os.path.join('/rhev/data-center', poolID, masterDomainID)
+        linkToMasterDomain =\
+            os.path.join('/rhev/data-center', poolID, 'mastersd')
+        assert os.path.lexists(linkToDomain)
+        assert os.path.lexists(linkToMasterDomain)
+
+    def waitFor(self, seconds, description, predicate, *args, **kwargs):
+        logging.info('waiting for "%s"' % description)
+        start = time.time()
+        for _ in xrange(seconds):
+            if predicate(*args, **kwargs):
+                logging.info('it took %0.3f seconds' % (time.time() - start))
+                return
+            time.sleep(1)
+
+        MESSAGE = 'waited %s seconds for "%s" but it did not happen'
%\
+                  (seconds, description)
+        assert False, MESSAGE
+
+    def spmStarted(self, poolID):
+        self.sleepWhileVDSMCompletesTask(duration=1)
+        masterDomainDirectory = '/rhev/data-center/%s/mastersd' % poolID
+        master = os.path.join(masterDomainDirectory, 'master')
+        tasks = os.path.join(master, 'tasks')
+        vms = os.path.join(master, 'vms')
+        self.waitFor(
+            60,
+            'SPM related subdirectories exist',
+            self._allExist,
+            [master, tasks, vms])
+
+    def _allExist(self, paths):
+        result = True
+        for path in paths:
+            result = result and os.path.exists(path)
+        return result
+
+    def waitUntilVDSMTaskFinished(self, taskID, timeout):
+        self.waitFor(
+            timeout,
+            'vdsm task to be finished',
+            self._taskFinished,
+            taskID)
+        taskStatus = self._taskStatus(taskID)
+        assert taskStatus['code'] == 0, taskStatus['message']
+
+    def _taskFinished(self, taskID):
+        return self._taskStatus(taskID)['taskState'] == 'finished'
+
+    def _taskStatus(self, taskID):
+        result = self.vdsm().getTaskStatus(taskID)
+        return result['taskStatus']
diff --git a/tests/functional/testlib/storagecontexts/filebased.py
b/tests/functional/testlib/storagecontexts/filebased.py
index bdb15d7..ba7ab10 100644
--- a/tests/functional/testlib/storagecontexts/filebased.py
+++ b/tests/functional/testlib/storagecontexts/filebased.py
@@ -2,10 +2,12 @@
 import storage.volume
 import storage.image
 from . import base
+from functional.testlib.storagecontexts.base import storagebackend
+from functional.testlib.storagecontexts.base import verify
 import logging
 
 
-class Verify(base.Verify):
+class Verify(verify.Verify):
 
     def rhevMountPoint(self):
         raise Exception('you must override this function')
@@ -52,7 +54,7 @@
             assert os.path.exists(path)
 
 
-class FileBased(base.StorageBackend):
+class FileBased(storagebackend.StorageBackend):
     def createVolume(self, size):
         PREALLOCATE = 1
         result = self.vdsm().createVolume(
diff --git a/tests/functional/testlib/storagecontexts/iscsi.py
b/tests/functional/testlib/storagecontexts/iscsi.py
index eb19715..640ec14 100644
--- a/tests/functional/testlib/storagecontexts/iscsi.py
+++ b/tests/functional/testlib/storagecontexts/iscsi.py
@@ -8,12 +8,13 @@
 import storage.sd
 import storage.volume
 import storage.image
-from . import base
+from functional.testlib.storagecontexts.base import storagebackend
+from functional.testlib.storagecontexts.base import verify
 
 
-class Verify(base.Verify):
+class Verify(verify.Verify):
     def __init__(self, iqn, volumeGroup, vdsm, volumeID):
-        base.Verify.__init__(self, vdsm)
+        verify.Verify.__init__(self, vdsm)
         self._iqn = iqn
         self._volumeGroup = volumeGroup
         self._volumeID = volumeID
@@ -53,11 +54,11 @@
         assert result == 0, "did not find logical volume in volume group"
 
 
-class ISCSI(base.StorageBackend):
+class ISCSI(storagebackend.StorageBackend):
     _NULL_UUID = '00000000-0000-0000-0000-000000000000'
 
     def __init__(self):
-        base.StorageBackend.__init__(self)
+        storagebackend.StorageBackend.__init__(self)
         self._iqn = 'iqn.1970-01.functional.test:%04d' %\
             random.randint(1, 10000)
         self._volumeGroup = {'uuid': self.domainID(), 'vgs_uuid': None}


-- 
To view, visit http://gerrit.ovirt.org/34242
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie9dbe640943f86fcb2415c25498f663304653b3a
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yoav Kleinberger <ykleinbe(a)redhat.com&gt;

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: vdsm: refactored new functional tests
......................................................................


Patch Set 1:

Build Failed 

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/12994/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/12836/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/12045/ : FAILURE

-- 
To view, visit http://gerrit.ovirt.org/34242
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie9dbe640943f86fcb2415c25498f663304653b3a
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yoav Kleinberger <ykleinbe(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

[Allon Mureinik]

Allon Mureinik has posted comments on this change.

Change subject: vdsm: refactored new functional tests
......................................................................


Patch Set 1:

Fede? Nir? thoughts?

-- 
To view, visit http://gerrit.ovirt.org/34242
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie9dbe640943f86fcb2415c25498f663304653b3a
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yoav Kleinberger <ykleinbe(a)redhat.com&gt;
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: vdsm: refactored new functional tests
......................................................................


Patch Set 1:

(4 comments)

Moving the verifier and backend to their own module can be nice, but creating unneeded
packages is not.

I think we should have *all* the files in the storage testlib in the same directory. Keep
it simple and flat:

    testlib/
        backend.py
        verify.py
        iscsi.py
        local.py
        nfs.py
        fc.py

http://gerrit.ovirt.org/#/c/34242/1/tests/functional/testlib/storageconte...
File tests/functional/testlib/storagecontexts/filebased.py:

Line 1: import os
Why is this called filebased.py, and iscsi module is called iscsi.py?

Name should be consistent and module names should be short. file.py or local.py are better
names.
Line 2: import storage.volume
Line 3: import storage.image
Line 4: from . import base
Line 5: from functional.testlib.storagecontexts.base import storagebackend


http://gerrit.ovirt.org/#/c/34242/1/tests/functional/testlib/storageconte...
File tests/functional/testlib/storagecontexts/iscsi.py:

Line 52
Line 53
Line 54
Line 55
Line 56
This is nice, clear that this is a base class...


Line 8: import storage.sd
Line 9: import storage.volume
Line 10: import storage.image
Line 11: from functional.testlib.storagecontexts.base import storagebackend
Line 12: from functional.testlib.storagecontexts.base import verify
Too wordy compared with the old code.
Line 13: 
Line 14: 
Line 15: class Verify(verify.Verify):
Line 16:     def __init__(self, iqn, volumeGroup, vdsm, volumeID):


Line 53:         result = subprocess.call(COMMAND, shell=True)
Line 54:         assert result == 0, "did not find logical volume in volume
group"
Line 55: 
Line 56: 
Line 57: class ISCSI(storagebackend.StorageBackend):
This is repetitive and does not tell that this is a base class.
Line 58:     _NULL_UUID = '00000000-0000-0000-0000-000000000000'
Line 59: 
Line 60:     def __init__(self):
Line 61:         storagebackend.StorageBackend.__init__(self)


-- 
To view, visit http://gerrit.ovirt.org/34242
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie9dbe640943f86fcb2415c25498f663304653b3a
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yoav Kleinberger <ykleinbe(a)redhat.com&gt;
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes

Yoav Kleinberger has posted comments on this change.

Change subject: vdsm: refactored new functional tests
......................................................................


Patch Set 1:

there's some merit to Nir's comments. Maybe this refactoring should be reverted.

-- 
To view, visit http://gerrit.ovirt.org/34242
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie9dbe640943f86fcb2415c25498f663304653b3a
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yoav Kleinberger <ykleinbe(a)redhat.com&gt;
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Yoav Kleinberger <ykleinbe(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No