diff mbox series

[5/8] VNC Acceptance test: use UNIX domain sockets to avoid port collisions

Message ID 20190607152223.9467-6-crosa@redhat.com
State New
Headers show
Series Miscellaneous acceptance test and Travis CI improvements | expand

Commit Message

Cleber Rosa June 7, 2019, 3:22 p.m. UTC
While running in parallel, the VNC tests that use a TCP port easily
collide.  There's a number of possibilities to reduce the probability
of collisions, but none that completely prevents it from happening.

So, to avoid those collisions, and given that the scope of the tests
are really not related to nature of the socket type, let's switch to
UNIX domain sockets created in temporary directories.

Note: the amount of boiler plate code is far from the ideal, but it's
related to the fact that a test "workdir"[1] attribute can not be used
here, because of the 108 bytes limitation of the UNIX socket path (see
ad9579aaa16). There's a fair assumption here that the temporary
directory returned by Python's tempfile.mkdtemp() won't be anywhere
close to 100 bytes.

[1] https://avocado-framework.readthedocs.io/en/68.0/api/test/avocado.html#avocado.Test.workdir

Signed-off-by: Cleber Rosa <crosa@redhat.com>
---
 tests/acceptance/vnc.py | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

Comments

Wainer dos Santos Moschetta June 10, 2019, 8:27 p.m. UTC | #1
On 06/07/2019 12:22 PM, Cleber Rosa wrote:
> While running in parallel, the VNC tests that use a TCP port easily
> collide.  There's a number of possibilities to reduce the probability
> of collisions, but none that completely prevents it from happening.
>
> So, to avoid those collisions, and given that the scope of the tests
> are really not related to nature of the socket type, let's switch to
> UNIX domain sockets created in temporary directories.
>
> Note: the amount of boiler plate code is far from the ideal, but it's
> related to the fact that a test "workdir"[1] attribute can not be used
> here, because of the 108 bytes limitation of the UNIX socket path (see
> ad9579aaa16). There's a fair assumption here that the temporary
> directory returned by Python's tempfile.mkdtemp() won't be anywhere
> close to 100 bytes.
>
> [1] https://avocado-framework.readthedocs.io/en/68.0/api/test/avocado.html#avocado.Test.workdir
>
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>   tests/acceptance/vnc.py | 20 ++++++++++++++++++--
>   1 file changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/tests/acceptance/vnc.py b/tests/acceptance/vnc.py
> index 064ceabcc1..675fd507ed 100644
> --- a/tests/acceptance/vnc.py
> +++ b/tests/acceptance/vnc.py
> @@ -8,6 +8,10 @@
>   # This work is licensed under the terms of the GNU GPL, version 2 or
>   # later.  See the COPYING file in the top-level directory.
>   
> +import os
> +import tempfile
> +import shutil
> +
>   from avocado_qemu import Test
>   
>   
> @@ -34,8 +38,16 @@ class Vnc(Test):
>           self.assertEqual(set_password_response['error']['desc'],
>                            'Could not set password')
>   
> +class VncUnixSocket(Test):
> +
> +    def setUp(self):
> +        super(VncUnixSocket, self).setUp()
> +        self.socket_dir = tempfile.mkdtemp()
> +        self.socket_path = os.path.join(self.socket_dir, 'vnc-socket')
> +
>       def test_vnc_change_password_requires_a_password(self):
> -        self.vm.add_args('-nodefaults', '-S', '-vnc', ':0')
> +        self.vm.add_args('-nodefaults', '-S',
> +                         '-vnc', 'unix:%s' % self.socket_path)
>           self.vm.launch()
>           self.assertTrue(self.vm.qmp('query-vnc')['return']['enabled'])
>           set_password_response = self.vm.qmp('change',
> @@ -49,7 +61,8 @@ class Vnc(Test):
>                            'Could not set password')
>   
>       def test_vnc_change_password(self):
> -        self.vm.add_args('-nodefaults', '-S', '-vnc', ':0,password')
> +        self.vm.add_args('-nodefaults', '-S',
> +                         '-vnc', 'unix:%s,password' % self.socket_path)
>           self.vm.launch()
>           self.assertTrue(self.vm.qmp('query-vnc')['return']['enabled'])
>           set_password_response = self.vm.qmp('change',
> @@ -57,3 +70,6 @@ class Vnc(Test):
>                                               target='password',
>                                               arg='new_password')
>           self.assertEqual(set_password_response['return'], {})
> +
> +    def tearDown(self):
> +        shutil.rmtree(self.socket_dir)

You missed to call super's tearDown in order to gently shutdown all VM 
created in by the tests. Other than that, it looks good to me.

- Wainer
diff mbox series

Patch

diff --git a/tests/acceptance/vnc.py b/tests/acceptance/vnc.py
index 064ceabcc1..675fd507ed 100644
--- a/tests/acceptance/vnc.py
+++ b/tests/acceptance/vnc.py
@@ -8,6 +8,10 @@ 
 # This work is licensed under the terms of the GNU GPL, version 2 or
 # later.  See the COPYING file in the top-level directory.
 
+import os
+import tempfile
+import shutil
+
 from avocado_qemu import Test
 
 
@@ -34,8 +38,16 @@  class Vnc(Test):
         self.assertEqual(set_password_response['error']['desc'],
                          'Could not set password')
 
+class VncUnixSocket(Test):
+
+    def setUp(self):
+        super(VncUnixSocket, self).setUp()
+        self.socket_dir = tempfile.mkdtemp()
+        self.socket_path = os.path.join(self.socket_dir, 'vnc-socket')
+
     def test_vnc_change_password_requires_a_password(self):
-        self.vm.add_args('-nodefaults', '-S', '-vnc', ':0')
+        self.vm.add_args('-nodefaults', '-S',
+                         '-vnc', 'unix:%s' % self.socket_path)
         self.vm.launch()
         self.assertTrue(self.vm.qmp('query-vnc')['return']['enabled'])
         set_password_response = self.vm.qmp('change',
@@ -49,7 +61,8 @@  class Vnc(Test):
                          'Could not set password')
 
     def test_vnc_change_password(self):
-        self.vm.add_args('-nodefaults', '-S', '-vnc', ':0,password')
+        self.vm.add_args('-nodefaults', '-S',
+                         '-vnc', 'unix:%s,password' % self.socket_path)
         self.vm.launch()
         self.assertTrue(self.vm.qmp('query-vnc')['return']['enabled'])
         set_password_response = self.vm.qmp('change',
@@ -57,3 +70,6 @@  class Vnc(Test):
                                             target='password',
                                             arg='new_password')
         self.assertEqual(set_password_response['return'], {})
+
+    def tearDown(self):
+        shutil.rmtree(self.socket_dir)