Re: [libvirt] [dbus PATCH v2 05/17] Introduce Interface Tests & Test Create method for Interface Interface

2018-07-24 Thread Pavel Hrdina
On Tue, Jul 24, 2018 at 11:30:10AM +0200, Katerina Koukiou wrote:
> On Fri, Jul 20, 2018 at 02:34:49PM -0400, Anya Harter wrote:
> > Signed-off-by: Anya Harter 
> > ---
> >  tests/Makefile.am   |  1 +
> >  tests/libvirttest.py| 12 
> >  tests/test_interface.py | 16 
> >  3 files changed, 29 insertions(+)
> >  create mode 100755 tests/test_interface.py
> > 
> > diff --git a/tests/Makefile.am b/tests/Makefile.am
> > index 09c3e2e..cd1fbd7 100644
> > --- a/tests/Makefile.am
> > +++ b/tests/Makefile.am
> > @@ -11,6 +11,7 @@ test_helpers = \
> >  test_programs = \
> > test_connect.py \
> > test_domain.py \
> > +   test_interface.py \
> > test_network.py \
> > test_nodedev.py \
> > test_storage.py \
> > diff --git a/tests/libvirttest.py b/tests/libvirttest.py
> > index 3741abd..2a09182 100644
> > --- a/tests/libvirttest.py
> > +++ b/tests/libvirttest.py
> > @@ -71,6 +71,18 @@ class BaseTestClass():
> >  if self.timeout:
> >  raise TimeoutError()
> >  
> 
> This method is not a fixture unless you put the @pytest.fixture
> decorator.
> 
> This code without the fixture decorator will actually work, but it does
> not do exactly what we want it to do.
> 
> See explanation bellow.
> 
> > +def interface_create(self):
> > +""" Fixture to define dummy interface on the test driver
> > +
> > +This fixture should be used in the setup of every test manipulating
> > +with interfaces.
> > +"""
> > +path = 
> > self.connect.InterfaceDefineXML(xmldata.minimal_interface_xml, 0)
> > +obj = self.bus.get_object('org.libvirt', path)
> > +interface_obj = dbus.Interface(obj, 'org.libvirt.Interface')
> > +interface_obj.Create(0)
> > +return path, interface_obj
> > +
> >  @pytest.fixture
> >  def node_device_create(self):
> >  """ Fixture to create dummy node device on the test driver
> > diff --git a/tests/test_interface.py b/tests/test_interface.py
> > new file mode 100755
> > index 000..88be5dc
> > --- /dev/null
> > +++ b/tests/test_interface.py
> > @@ -0,0 +1,16 @@
> > +#!/usr/bin/python3
> > +
> > +import dbus
> > +import libvirttest
> > +
> > +class TestInterface(libvirttest.BaseTestClass):
> > +""" Tests for methods and properties of the Interface interface
> > +"""
> 
> Here you are calling the interace_create function (! it's not a fixture
> without the decorator). It will create a dummy interface on the test
> driver so that your tests can use it. However, the interface_create
> functionality should not be tested in this test, this interface creation 
> should
> be part of the test "setup".
> 
> What is "setup"?
> pytest defines three test phases for each test, which are "setup", "call" and
> "teardown". The "setup" part should be used to prepare the environment for
> the test to run (for our case define an test Interface), and teardown to clean
> up after the test. Call is the actual test code itself.
> The way to run some code as a test setup is using pytest fuxtures.
> 
> So the fixture definition you need to mark it with @pytest.fixture and
> the functions where the fixture should be applied to with 
> @pytest.mark.usefixtures("interface_create")
> Then instead of calling the fixture we can use it's result by having the
> fixture name in the function arguments. So the following should look
> like:
> 
> @pytest.mark.fixtures("interface_create")
> def test_interface_create(self, test_interface):
> _,interface_obj = test_interface
> interface_obj.Destroy(0)
> interface_obj.Create(0)

Nice, I didn't know that this is possible.  I suggested not to use the
fixture because we need the result as well and existing code was calling
the fixture again as normal function which was failing for interfaces.

Pavel


signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [dbus PATCH v2 05/17] Introduce Interface Tests & Test Create method for Interface Interface

2018-07-24 Thread Katerina Koukiou
On Fri, Jul 20, 2018 at 02:34:49PM -0400, Anya Harter wrote:
> Signed-off-by: Anya Harter 
> ---
>  tests/Makefile.am   |  1 +
>  tests/libvirttest.py| 12 
>  tests/test_interface.py | 16 
>  3 files changed, 29 insertions(+)
>  create mode 100755 tests/test_interface.py
> 
> diff --git a/tests/Makefile.am b/tests/Makefile.am
> index 09c3e2e..cd1fbd7 100644
> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -11,6 +11,7 @@ test_helpers = \
>  test_programs = \
>   test_connect.py \
>   test_domain.py \
> + test_interface.py \
>   test_network.py \
>   test_nodedev.py \
>   test_storage.py \
> diff --git a/tests/libvirttest.py b/tests/libvirttest.py
> index 3741abd..2a09182 100644
> --- a/tests/libvirttest.py
> +++ b/tests/libvirttest.py
> @@ -71,6 +71,18 @@ class BaseTestClass():
>  if self.timeout:
>  raise TimeoutError()
>  

This method is not a fixture unless you put the @pytest.fixture
decorator.

This code without the fixture decorator will actually work, but it does
not do exactly what we want it to do.

See explanation bellow.

> +def interface_create(self):
> +""" Fixture to define dummy interface on the test driver
> +
> +This fixture should be used in the setup of every test manipulating
> +with interfaces.
> +"""
> +path = 
> self.connect.InterfaceDefineXML(xmldata.minimal_interface_xml, 0)
> +obj = self.bus.get_object('org.libvirt', path)
> +interface_obj = dbus.Interface(obj, 'org.libvirt.Interface')
> +interface_obj.Create(0)
> +return path, interface_obj
> +
>  @pytest.fixture
>  def node_device_create(self):
>  """ Fixture to create dummy node device on the test driver
> diff --git a/tests/test_interface.py b/tests/test_interface.py
> new file mode 100755
> index 000..88be5dc
> --- /dev/null
> +++ b/tests/test_interface.py
> @@ -0,0 +1,16 @@
> +#!/usr/bin/python3
> +
> +import dbus
> +import libvirttest
> +
> +class TestInterface(libvirttest.BaseTestClass):
> +""" Tests for methods and properties of the Interface interface
> +"""

Here you are calling the interace_create function (! it's not a fixture
without the decorator). It will create a dummy interface on the test
driver so that your tests can use it. However, the interface_create
functionality should not be tested in this test, this interface creation should
be part of the test "setup".

What is "setup"?
pytest defines three test phases for each test, which are "setup", "call" and
"teardown". The "setup" part should be used to prepare the environment for
the test to run (for our case define an test Interface), and teardown to clean
up after the test. Call is the actual test code itself.
The way to run some code as a test setup is using pytest fuxtures.

So the fixture definition you need to mark it with @pytest.fixture and
the functions where the fixture should be applied to with 
@pytest.mark.usefixtures("interface_create")
Then instead of calling the fixture we can use it's result by having the
fixture name in the function arguments. So the following should look
like:

@pytest.mark.fixtures("interface_create")
def test_interface_create(self, test_interface):
_,interface_obj = test_interface
interface_obj.Destroy(0)
interface_obj.Create(0)

> +
> +def test_interface_create(self):
> +_,interface_obj = self.interface_create()
> +interface_obj.Destroy(0)
> +interface_obj.Create(0)
> +
> +if __name__ == '__main__':
> +libvirttest.run()
> -- 
> 2.17.1
> 
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list

Although the concept is not correct, similar mistake exist in the
node_device tests, so I can fix it together in another patch set.

Reviewed-by: Katerina Koukiou 


Note: 
If you 're still not sure what the setup fixture does see the code
bellow:

$ cat show-setup.py
import pytest

@pytest.mark.usefixtures("foo")
def test_bar(foo):
print("Here starts test CALL")
print(foo)


@pytest.fixture
def foo():
print("Here starts test SETUP");
return "blabla"


$ pytest show-setup.py -s
 
test session starts 

platform linux2 -- Python 2.7.15, pytest-3.4.2, py-1.5.2, pluggy-0.6.0
rootdir: /home/kkoukiou/repos/libvirt-dbus, inifile:
plugins: ordering-0.5, marker-bugzilla-0.7, jira-0.3.6, forked-0.2
collected 1 item

show-setup.py Here starts test SETUP
Here starts test CALL
blabla
.

= 1 
passed in 0.00 seconds 
==


signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir

[libvirt] [dbus PATCH v2 05/17] Introduce Interface Tests & Test Create method for Interface Interface

2018-07-20 Thread Anya Harter
Signed-off-by: Anya Harter 
---
 tests/Makefile.am   |  1 +
 tests/libvirttest.py| 12 
 tests/test_interface.py | 16 
 3 files changed, 29 insertions(+)
 create mode 100755 tests/test_interface.py

diff --git a/tests/Makefile.am b/tests/Makefile.am
index 09c3e2e..cd1fbd7 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -11,6 +11,7 @@ test_helpers = \
 test_programs = \
test_connect.py \
test_domain.py \
+   test_interface.py \
test_network.py \
test_nodedev.py \
test_storage.py \
diff --git a/tests/libvirttest.py b/tests/libvirttest.py
index 3741abd..2a09182 100644
--- a/tests/libvirttest.py
+++ b/tests/libvirttest.py
@@ -71,6 +71,18 @@ class BaseTestClass():
 if self.timeout:
 raise TimeoutError()
 
+def interface_create(self):
+""" Fixture to define dummy interface on the test driver
+
+This fixture should be used in the setup of every test manipulating
+with interfaces.
+"""
+path = self.connect.InterfaceDefineXML(xmldata.minimal_interface_xml, 
0)
+obj = self.bus.get_object('org.libvirt', path)
+interface_obj = dbus.Interface(obj, 'org.libvirt.Interface')
+interface_obj.Create(0)
+return path, interface_obj
+
 @pytest.fixture
 def node_device_create(self):
 """ Fixture to create dummy node device on the test driver
diff --git a/tests/test_interface.py b/tests/test_interface.py
new file mode 100755
index 000..88be5dc
--- /dev/null
+++ b/tests/test_interface.py
@@ -0,0 +1,16 @@
+#!/usr/bin/python3
+
+import dbus
+import libvirttest
+
+class TestInterface(libvirttest.BaseTestClass):
+""" Tests for methods and properties of the Interface interface
+"""
+
+def test_interface_create(self):
+_,interface_obj = self.interface_create()
+interface_obj.Destroy(0)
+interface_obj.Create(0)
+
+if __name__ == '__main__':
+libvirttest.run()
-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list