Martin Polednik has uploaded a new change for review.
Change subject: virt: remove duplicate connection in hostdevTests ......................................................................
virt: remove duplicate connection in hostdevTests
hostdevTests included a fake Connection, similar to the one in vmfakelib. This patch creates new decorator, that is able to inject devices without needing another Connection and without having to hack the vmfakelib connection.
Change-Id: I936766c8fd4c87175dcc3987d7ae5c64ff5675ea Signed-off-by: Martin Polednik mpolednik@redhat.com --- M tests/hostdevTests.py M tests/vmfakelib.py 2 files changed, 33 insertions(+), 17 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/46/40946/1
diff --git a/tests/hostdevTests.py b/tests/hostdevTests.py index 6cf7b78..94e06cd 100644 --- a/tests/hostdevTests.py +++ b/tests/hostdevTests.py @@ -204,22 +204,6 @@ {'params': DEVICES_PARSED['usb_1_1']}}}
-class Connection(fake.Connection): - - def __init__(self, *args): - self._virNodeDevices = [] - - for device in _PCI_DEVICES + _USB_DEVICES + _SCSI_DEVICES: - self._virNodeDevices.append( - fake.Connection.nodeDeviceLookupByName(device)) - - def listAllDevices(self, flags=0): - return self._virNodeDevices - - def nodeDeviceLookupByName(self, device_xml_filename): - return fake.Connection.nodeDeviceLookupByName(device_xml_filename) - - def _fake_totalvfs(device_name): if device_name == 'pci_0000_05_00_1': return 7 @@ -228,7 +212,7 @@
@expandPermutations -@MonkeyClass(libvirtconnection, 'get', Connection) +@MonkeyClass(libvirtconnection, 'get', fake.Connection) @MonkeyClass(hostdev, '_sriov_totalvfs', _fake_totalvfs) @MonkeyClass(hooks, 'after_hostdev_list_by_caps', lambda json: json) class HostdevTests(TestCaseBase): @@ -265,6 +249,7 @@
self.assertEquals(_NET_DEVICE_PARSED, deviceXML)
+ @fake.set_device_pool(_PCI_DEVICES + _USB_DEVICES + _SCSI_DEVICES) def testGetDevicesFromLibvirt(self): libvirt_devices = hostdev._get_devices_from_libvirt()
@@ -274,6 +259,7 @@ len(_USB_DEVICES) + len(_SCSI_DEVICES))
+ @fake.set_device_pool(_PCI_DEVICES + _USB_DEVICES + _SCSI_DEVICES) @permutations([[''], [('pci',)], [('usb_device',)], [('pci', 'usb_device')]]) def testListByCaps(self, caps): diff --git a/tests/vmfakelib.py b/tests/vmfakelib.py index a77d15e..9c15325 100644 --- a/tests/vmfakelib.py +++ b/tests/vmfakelib.py @@ -20,6 +20,7 @@ #
from contextlib import contextmanager +from functools import wraps import logging import os import threading @@ -49,6 +50,9 @@ def listAllNetworks(self, *args): return []
+ def listAllDevices(self, flags=0): + pass + @staticmethod def nodeDeviceLookupByName(name): """ @@ -71,6 +75,32 @@ return VirNodeDeviceStub(device_xml)
+def set_device_pool(devices): + """ + Decorator to inject new listAllDevices method, that will return + VirNodeDeviceStubs from device names specified in the argument. + """ + def decorator(func): + @wraps(func) + def wrapper(*args, **kwargs): + vir_node_devices = [] + + for device in devices: + vir_node_devices.append( + Connection.nodeDeviceLookupByName(device) + ) + + def listAllDevices(_, flags=0): + return vir_node_devices + + with MonkeyPatchScope( + [(libvirtconnection.get, 'listAllDevices', listAllDevices)] + ): + return func(*args, **kwargs) + return wrapper + return decorator + + class ClientIF(clientIF.clientIF): def __init__(self): # the bare minimum initialization for our test needs.
automation@ovirt.org has posted comments on this change.
Change subject: virt: remove duplicate connection in hostdevTests ......................................................................
Patch Set 1:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
Francesco Romani has posted comments on this change.
Change subject: virt: remove duplicate connection in hostdevTests ......................................................................
Patch Set 1: Code-Review+1
cool, I like the concept.
Please note that I'm *not* religiously against duplicating fakes outside vmfakelib. I just need we need clear and strong cases for such duplication, and try hard to be DRY in the common case.
Ido Barkan has posted comments on this change.
Change subject: virt: remove duplicate connection in hostdevTests ......................................................................
Patch Set 1: Code-Review+1
automation@ovirt.org has posted comments on this change.
Change subject: virt: remove duplicate connection in hostdevTests ......................................................................
Patch Set 2:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
Martin Polednik has posted comments on this change.
Change subject: virt: remove duplicate connection in hostdevTests ......................................................................
Patch Set 2: Verified+1
Francesco Romani has posted comments on this change.
Change subject: virt: remove duplicate connection in hostdevTests ......................................................................
Patch Set 2:
(2 comments)
https://gerrit.ovirt.org/#/c/40946/2/tests/vmfakelib.py File tests/vmfakelib.py:
Line 87: Line 88: def listAllDevices(self, flags=0): Line 89: pass Line 90: Line 91: @staticmethod related? Line 92: def nodeDeviceLookupByName(name): Line 93: """ Line 94: This is a method that allows us to access hostdev XML in a test. Line 95: Normally, libvirt holds the device XML but in case of unit testing,
Line 170: def listAllDevices(_, flags=0): Line 171: return vir_node_devices Line 172: Line 173: with MonkeyPatchScope( Line 174: [(libvirtconnection.get, 'listAllDevices', listAllDevices)] not sure this works as intended. I'm suspicious of the first argoment. Shouldn't wrap a Connection (fake or real) object? Line 175: ): Line 176: return func(*args, **kwargs) Line 177: return wrapper Line 178: return decorator
automation@ovirt.org has posted comments on this change.
Change subject: virt: remove duplicate connection in hostdevTests ......................................................................
Patch Set 3:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
Martin Polednik has posted comments on this change.
Change subject: virt: remove duplicate connection in hostdevTests ......................................................................
Patch Set 3: Verified+1
Works as expected now: in order to keep the semantics of method belonging to the class the Connection class is copied (via inheritance) and the method is added and filled with required devices. The class is only available in patched function/method as expected.
automation@ovirt.org has posted comments on this change.
Change subject: virt: remove duplicate connection in hostdevTests ......................................................................
Patch Set 4:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
automation@ovirt.org has posted comments on this change.
Change subject: virt: remove duplicate connection in hostdevTests ......................................................................
Patch Set 5:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
vdsm-patches@lists.fedorahosted.org