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>
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes