[libvirt] [dbus PATCH 0/2] Make correct use of setup fixtures & cleanup

Katerina Koukiou (2): tests: {interface, node_device}_create should be used as setup fixtures tests: fix all coding style issues to comply with pep8 tests/libvirttest.py | 1 + tests/test_connect.py | 9 +++++---- tests/test_domain.py | 5 +++-- tests/test_interface.py | 23 +++++++++++++---------- tests/test_network.py | 8 ++++---- tests/test_nodedev.py | 16 ++++++++-------- tests/test_storage.py | 1 + 7 files changed, 35 insertions(+), 28 deletions(-) -- 2.17.1

* node_device_create was a fixture but we were calling it as normal function, thus it got triggered twice. * interface_create was not a fixture. This patch makes sure that the setup work these two functions are doing, will run before the actual test call phase. Signed-off-by: Katerina Koukiou <kkoukiou@redhat.com> --- tests/libvirttest.py | 1 + tests/test_connect.py | 9 +++++---- tests/test_interface.py | 22 ++++++++++++---------- tests/test_nodedev.py | 16 ++++++++-------- 4 files changed, 26 insertions(+), 22 deletions(-) diff --git a/tests/libvirttest.py b/tests/libvirttest.py index 9db6e0a..7a75ff3 100644 --- a/tests/libvirttest.py +++ b/tests/libvirttest.py @@ -71,6 +71,7 @@ class BaseTestClass(): if self.timeout: raise TimeoutError() + @pytest.fixture def interface_create(self): """ Fixture to define dummy interface on the test driver diff --git a/tests/test_connect.py b/tests/test_connect.py index 042c568..bb2d767 100755 --- a/tests/test_connect.py +++ b/tests/test_connect.py @@ -88,14 +88,15 @@ class TestConnect(libvirttest.BaseTestClass): path = self.connect.InterfaceDefineXML(xmldata.minimal_interface_xml, 0) assert isinstance(path, dbus.ObjectPath) + @pytest.mark.usefixtures("interface_create") @pytest.mark.parametrize("lookup_method_name,lookup_item", [ ("InterfaceLookupByName", 'Name'), ("InterfaceLookupByMAC", 'MAC'), ]) - def test_connect_interface_lookup_by_property(self, lookup_method_name, lookup_item): + def test_connect_interface_lookup_by_property(self, lookup_method_name, lookup_item, interface_create): """Parameterized test for all InterfaceLookupBy* API calls of Connect interface """ - original_path,_ = self.interface_create() + original_path,_ = interface_create obj = self.bus.get_object('org.libvirt', original_path) prop = obj.Get('org.libvirt.Interface', lookup_item, dbus_interface=dbus.PROPERTIES_IFACE) path = getattr(self.connect, lookup_method_name)(prop) @@ -164,10 +165,10 @@ class TestConnect(libvirttest.BaseTestClass): @pytest.mark.parametrize("lookup_method_name,lookup_item", [ ("NodeDeviceLookupByName", 'Name'), ]) - def test_connect_node_device_lookup_by_property(self, lookup_method_name, lookup_item): + def test_connect_node_device_lookup_by_property(self, lookup_method_name, lookup_item, node_device_create): """Parameterized test for all NodeDeviceLookupBy* API calls of Connect interface """ - original_path = self.node_device_create() + original_path = node_device_create obj = self.bus.get_object('org.libvirt', original_path) prop = obj.Get('org.libvirt.NodeDevice', lookup_item, dbus_interface=dbus.PROPERTIES_IFACE) path = getattr(self.connect, lookup_method_name)(prop) diff --git a/tests/test_interface.py b/tests/test_interface.py index 9503eef..bcda8d5 100755 --- a/tests/test_interface.py +++ b/tests/test_interface.py @@ -2,33 +2,35 @@ import dbus import libvirttest +import pytest +@pytest.mark.usefixtures("interface_create") class TestInterface(libvirttest.BaseTestClass): """ Tests for methods and properties of the Interface interface """ - def test_interface_undefine(self): - _,interface_obj = self.interface_create() + def test_interface_undefine(self, interface_create): + _,interface_obj = interface_create interface_obj.Destroy(0) interface_obj.Undefine() - def test_interface_destroy(self): - _,interface_obj = self.interface_create() + def test_interface_destroy(self, interface_create): + _,interface_obj = interface_create interface_obj.Destroy(0) - def test_interface_create(self): - _,interface_obj = self.interface_create() + def test_interface_create(self, interface_create): + _,interface_obj = interface_create interface_obj.Destroy(0) interface_obj.Create(0) - def test_interface_get_xml_description(self): - _,interface_obj = self.interface_create() + def test_interface_get_xml_description(self, interface_create): + _,interface_obj = interface_create assert isinstance(interface_obj.GetXMLDesc(0), dbus.String) - def test_interface_properties_type(self): + def test_interface_properties_type(self, interface_create): """ Ensure correct return type for Interface properties """ - test_interface_path,_ = self.interface_create() + test_interface_path,_ = interface_create obj = self.bus.get_object('org.libvirt', test_interface_path) props = obj.GetAll('org.libvirt.Interface', dbus_interface=dbus.PROPERTIES_IFACE) assert isinstance(props['Name'], dbus.String) diff --git a/tests/test_nodedev.py b/tests/test_nodedev.py index 6aa6211..22c8d60 100755 --- a/tests/test_nodedev.py +++ b/tests/test_nodedev.py @@ -10,7 +10,7 @@ class TestNodeDevice(libvirttest.BaseTestClass): """ Tests for methods and properties of the NodeDevice interface """ - def test_node_device_destroy(self): + def test_node_device_destroy(self, node_device_create): def node_device_deleted(path, event, _detail): if event != libvirttest.NodeDeviceEvent.DELETED: return @@ -19,29 +19,29 @@ class TestNodeDevice(libvirttest.BaseTestClass): self.connect.connect_to_signal('NodeDeviceEvent', node_device_deleted) - test_node_device_path = self.node_device_create() + test_node_device_path = node_device_create obj = self.bus.get_object('org.libvirt', test_node_device_path) interface_obj = dbus.Interface(obj, 'org.libvirt.NodeDevice') interface_obj.Destroy() self.main_loop() - def test_node_device_get_xml_description(self): - test_node_device_path = self.node_device_create() + def test_node_device_get_xml_description(self, node_device_create): + test_node_device_path = node_device_create obj = self.bus.get_object('org.libvirt', test_node_device_path) interface_obj = dbus.Interface(obj, 'org.libvirt.NodeDevice') assert isinstance(interface_obj.GetXMLDesc(0), dbus.String) - def test_node_device_list_caps(self): - test_node_device_path = self.node_device_create() + def test_node_device_list_caps(self, node_device_create): + test_node_device_path = node_device_create obj = self.bus.get_object('org.libvirt', test_node_device_path) interface_obj = dbus.Interface(obj, 'org.libvirt.NodeDevice') assert isinstance(interface_obj.ListCaps(), dbus.Array) - def test_node_device_properties_type(self): + def test_node_device_properties_type(self, node_device_create): """ Ensure correct return type for NodeDevice properties """ - test_node_device_path = self.node_device_create() + test_node_device_path = node_device_create obj = self.bus.get_object('org.libvirt', test_node_device_path) props = obj.GetAll('org.libvirt.NodeDevice', dbus_interface=dbus.PROPERTIES_IFACE) assert isinstance(props['Name'], dbus.String) -- 2.17.1

On Thu, Jul 26, 2018 at 02:09:57PM +0200, Katerina Koukiou wrote:
* node_device_create was a fixture but we were calling it as normal function, thus it got triggered twice. * interface_create was not a fixture.
This patch makes sure that the setup work these two functions are doing, will run before the actual test call phase.
Signed-off-by: Katerina Koukiou <kkoukiou@redhat.com> --- tests/libvirttest.py | 1 + tests/test_connect.py | 9 +++++---- tests/test_interface.py | 22 ++++++++++++---------- tests/test_nodedev.py | 16 ++++++++-------- 4 files changed, 26 insertions(+), 22 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

Signed-off-by: Katerina Koukiou <kkoukiou@redhat.com> --- tests/test_connect.py | 2 +- tests/test_domain.py | 5 +++-- tests/test_interface.py | 11 ++++++----- tests/test_network.py | 8 ++++---- tests/test_storage.py | 1 + 5 files changed, 15 insertions(+), 12 deletions(-) diff --git a/tests/test_connect.py b/tests/test_connect.py index bb2d767..f481356 100755 --- a/tests/test_connect.py +++ b/tests/test_connect.py @@ -96,7 +96,7 @@ class TestConnect(libvirttest.BaseTestClass): def test_connect_interface_lookup_by_property(self, lookup_method_name, lookup_item, interface_create): """Parameterized test for all InterfaceLookupBy* API calls of Connect interface """ - original_path,_ = interface_create + original_path, _ = interface_create obj = self.bus.get_object('org.libvirt', original_path) prop = obj.Get('org.libvirt.Interface', lookup_item, dbus_interface=dbus.PROPERTIES_IFACE) path = getattr(self.connect, lookup_method_name)(prop) diff --git a/tests/test_domain.py b/tests/test_domain.py index b9a6d33..96b282c 100755 --- a/tests/test_domain.py +++ b/tests/test_domain.py @@ -5,6 +5,7 @@ import libvirttest DBUS_EXCEPTION_MISSING_FUNCTION = 'this function is not supported by the connection driver' + class TestDomain(libvirttest.BaseTestClass): def test_api(self): obj, domain = self.get_test_domain() @@ -153,8 +154,8 @@ class TestDomain(libvirttest.BaseTestClass): def test_domain_vcpu_pin_info(self): obj, domain = self.get_test_domain() pinInfo_expected = [ - [ True, True, True, True, True, True, True, True ], - [ True, True, True, True, True, True, True, True ] + [True, True, True, True, True, True, True, True], + [True, True, True, True, True, True, True, True] ] pinInfo = domain.GetVcpuPinInfo(0) assert pinInfo == pinInfo_expected diff --git a/tests/test_interface.py b/tests/test_interface.py index bcda8d5..38a1cc1 100755 --- a/tests/test_interface.py +++ b/tests/test_interface.py @@ -4,33 +4,34 @@ import dbus import libvirttest import pytest + @pytest.mark.usefixtures("interface_create") class TestInterface(libvirttest.BaseTestClass): """ Tests for methods and properties of the Interface interface """ def test_interface_undefine(self, interface_create): - _,interface_obj = interface_create + _, interface_obj = interface_create interface_obj.Destroy(0) interface_obj.Undefine() def test_interface_destroy(self, interface_create): - _,interface_obj = interface_create + _, interface_obj = interface_create interface_obj.Destroy(0) def test_interface_create(self, interface_create): - _,interface_obj = interface_create + _, interface_obj = interface_create interface_obj.Destroy(0) interface_obj.Create(0) def test_interface_get_xml_description(self, interface_create): - _,interface_obj = interface_create + _, interface_obj = interface_create assert isinstance(interface_obj.GetXMLDesc(0), dbus.String) def test_interface_properties_type(self, interface_create): """ Ensure correct return type for Interface properties """ - test_interface_path,_ = interface_create + test_interface_path, _ = interface_create obj = self.bus.get_object('org.libvirt', test_interface_path) props = obj.GetAll('org.libvirt.Interface', dbus_interface=dbus.PROPERTIES_IFACE) assert isinstance(props['Name'], dbus.String) diff --git a/tests/test_network.py b/tests/test_network.py index 11418cb..2e3a8f0 100755 --- a/tests/test_network.py +++ b/tests/test_network.py @@ -25,7 +25,7 @@ class TestNetwork(libvirttest.BaseTestClass): assert isinstance(props['UUID'], dbus.String) def test_network_autostart(self): - _,test_network = self.get_test_network() + _, test_network = self.get_test_network() interface_obj = dbus.Interface(test_network, 'org.libvirt.Network') autostart_expected = True interface_obj.Set('org.libvirt.Network', 'Autostart', autostart_expected, dbus_interface=dbus.PROPERTIES_IFACE) @@ -41,7 +41,7 @@ class TestNetwork(libvirttest.BaseTestClass): self.connect.connect_to_signal('NetworkEvent', domain_started) - _,test_network = self.get_test_network() + _, test_network = self.get_test_network() interface_obj = dbus.Interface(test_network, 'org.libvirt.Network') interface_obj.Destroy() interface_obj.Create() @@ -64,7 +64,7 @@ class TestNetwork(libvirttest.BaseTestClass): self.main_loop() def test_network_get_xml_description(self): - _,test_network = self.get_test_network() + _, test_network = self.get_test_network() interface_obj = dbus.Interface(test_network, 'org.libvirt.Network') assert isinstance(interface_obj.GetXMLDesc(0), dbus.String) @@ -77,7 +77,7 @@ class TestNetwork(libvirttest.BaseTestClass): self.connect.connect_to_signal('NetworkEvent', domain_undefined) - _,test_network = self.get_test_network() + _, test_network = self.get_test_network() interface_obj = dbus.Interface(test_network, 'org.libvirt.Network') interface_obj.Destroy() interface_obj.Undefine() diff --git a/tests/test_storage.py b/tests/test_storage.py index 63ecf91..631e107 100755 --- a/tests/test_storage.py +++ b/tests/test_storage.py @@ -4,6 +4,7 @@ import dbus import libvirttest import pytest + class TestStoragePool(libvirttest.BaseTestClass): def test_storage_pool_autostart(self): _, test_storage_pool = self.get_test_storage_pool() -- 2.17.1

On Thu, Jul 26, 2018 at 02:09:58PM +0200, Katerina Koukiou wrote: Is there nothing that could be said here in the commit message? Also, it might be a good time to introduce a 'syntax-check' target to Makefile, to make sure these won't get merged again.
Signed-off-by: Katerina Koukiou <kkoukiou@redhat.com> --- tests/test_connect.py | 2 +- tests/test_domain.py | 5 +++-- tests/test_interface.py | 11 ++++++----- tests/test_network.py | 8 ++++---- tests/test_storage.py | 1 + 5 files changed, 15 insertions(+), 12 deletions(-)
diff --git a/tests/test_domain.py b/tests/test_domain.py index b9a6d33..96b282c 100755 --- a/tests/test_domain.py +++ b/tests/test_domain.py @@ -5,6 +5,7 @@ import libvirttest
DBUS_EXCEPTION_MISSING_FUNCTION = 'this function is not supported by the connection driver'
+ class TestDomain(libvirttest.BaseTestClass): def test_api(self): obj, domain = self.get_test_domain() @@ -153,8 +154,8 @@ class TestDomain(libvirttest.BaseTestClass): def test_domain_vcpu_pin_info(self): obj, domain = self.get_test_domain() pinInfo_expected = [ - [ True, True, True, True, True, True, True, True ], - [ True, True, True, True, True, True, True, True ] + [True, True, True, True, True, True, True, True], + [True, True, True, True, True, True, True, True]
The 'flake8 --ignore=E501' command you proposed months ago [0] still shows some issues, including: ./tests/test_domain.py:157:17: E126 continuation line over-indented for hanging indent [True, True, True, True, True, True, True, True], ^ Jano [0] https://www.redhat.com/archives/libvir-list/2018-March/msg01368.html

On Thu, Jul 26, 2018 at 02:39:37PM +0200, Ján Tomko wrote:
On Thu, Jul 26, 2018 at 02:09:58PM +0200, Katerina Koukiou wrote:
Is there nothing that could be said here in the commit message?
Also, it might be a good time to introduce a 'syntax-check' target to Makefile, to make sure these won't get merged again.
OK, I 'll add syntax check with flake8, with reminders and repost. Katerina
participants (2)
-
Ján Tomko
-
Katerina Koukiou