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