Re: [Qemu-devel] [PATCH 3/5] Acceptance tests: add quick VNC tests

[Cleber Rosa]

On 05/25/2018 01:37 AM, Fam Zheng wrote:
> On Thu, 05/24 20:58, Cleber Rosa wrote:
>> This patch adds a few simple behavior tests for VNC.  These tests
>> introduce manipulation of the QEMUMachine arguments, by setting
>> the arguments, instead of adding to the existing ones.
> 
> I'm confused by this. The code uses 'add_args', so it does add to the 
> arguments,
> no?
> 

And you should be.  I changed the code to just add args, and forgot to
update the commit message.  If a better example comes up that requires
setting arguments, I'll get back to this.

>>
>> Signed-off-by: Cleber Rosa <address@hidden>
>> ---
>>  tests/acceptance/test_vnc.py | 50 ++++++++++++++++++++++++++++++++++++
>>  1 file changed, 50 insertions(+)
>>  create mode 100644 tests/acceptance/test_vnc.py
>>
>> diff --git a/tests/acceptance/test_vnc.py b/tests/acceptance/test_vnc.py
>> new file mode 100644
>> index 0000000000..9d9a35cf55
>> --- /dev/null
>> +++ b/tests/acceptance/test_vnc.py
>> @@ -0,0 +1,50 @@
> 
> Copyright header is missing here too.
> 

Indeed.

> Fam
> 
>> +from avocado_qemu import Test
>> +
>> +
>> +class Vnc(Test):
> 
> Should VncTest be a better class name?
> 

I'm favoring simpler names.  If you think about the complete test names,
it's already too verbose IMO: "test_vnc.Vnc.test_no_vnc".

That's actually an interesting point: how would you feel about dropping
the "test_" prefix from the file names?

- Cleber.

>> +    """
>> +    :avocado: enable
>> +    :avocado: tags=vnc,quick
>> +    """
>> +    def test_no_vnc(self):
>> +        self.vm.add_args('-nodefaults', '-S')
>> +        self.vm.launch()
>> +        self.assertFalse(self.vm.qmp('query-vnc')['return']['enabled'])
>> +
>> +    def test_no_vnc_change_password(self):
>> +        self.vm.add_args('-nodefaults', '-S')
>> +        self.vm.launch()
>> +        self.assertFalse(self.vm.qmp('query-vnc')['return']['enabled'])
>> +        set_password_response = self.vm.qmp('change',
>> +                                            device='vnc',
>> +                                            target='password',
>> +                                            arg='new_password')
>> +        self.assertIn('error', set_password_response)
>> +        self.assertEqual(set_password_response['error']['class'],
>> +                         'GenericError')
>> +        self.assertEqual(set_password_response['error']['desc'],
>> +                         'Could not set password')
>> +
>> +    def test_vnc_change_password_requires_a_password(self):
>> +        self.vm.add_args('-nodefaults', '-S', '-vnc', ':0')
>> +        self.vm.launch()
>> +        self.assertTrue(self.vm.qmp('query-vnc')['return']['enabled'])
>> +        set_password_response = self.vm.qmp('change',
>> +                                            device='vnc',
>> +                                            target='password',
>> +                                            arg='new_password')
>> +        self.assertIn('error', set_password_response)
>> +        self.assertEqual(set_password_response['error']['class'],
>> +                         'GenericError')
>> +        self.assertEqual(set_password_response['error']['desc'],
>> +                         'Could not set password')
>> +
>> +    def test_vnc_change_password(self):
>> +        self.vm.add_args('-nodefaults', '-S', '-vnc', ':0,password')
>> +        self.vm.launch()
>> +        self.assertTrue(self.vm.qmp('query-vnc')['return']['enabled'])
>> +        set_password_response = self.vm.qmp('change',
>> +                                            device='vnc',
>> +                                            target='password',
>> +                                            arg='new_password')
>> +        self.assertEqual(set_password_response['return'], {})
>> -- 
>> 2.17.0
>>
> 
> Fam
>