| Message ID | 20190607152223.9467-6-crosa@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Miscellaneous acceptance test and Travis CI improvements | expand |
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 --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)
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(-)