Re: [Qemu-devel] [PATCH v2 1/2] tests.acceptance: adds multi vm capability for acceptance tests

[Wainer dos Santos Moschetta]

On 01/28/2019 03:47 PM, Caio Carrara wrote:
> This change adds the possibility to write acceptance tests with multi
> virtual machine support. It's done keeping the virtual machines objects
> stored in a test attribute (dictionary). This dictionary shouldn't be
> accessed directly but through the new method added `get_vm`. This new
> method accept a list of args (that will be added as virtual machine
> arguments) and an optional name argument. The name is the key that
> identify a single virtual machine along the test machines available. If
> a name without a machine is informed a new machine will be instantiated.
>
> The current usage of vm in tests will not be broken by this change since
> it keeps a property called vm in the base test class. This property only
> calls the new method `get_vm` with default parameters (no args and
> 'default' as machine name).

I've checked that current tests does not break by this change. I also 
checked the example you provided on docs/devel/testing.rst works too.

So Tested-by: Wainer dos Santos Moschetta <address@hidden>

> Signed-off-by: Caio Carrara <address@hidden>
> ---
>   docs/devel/testing.rst                    | 40 ++++++++++++++++++++++-
>   tests/acceptance/avocado_qemu/__init__.py | 25 +++++++++++---
>   2 files changed, 60 insertions(+), 5 deletions(-)
>
> diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst
> index 18e2c0868a..b97c0368bc 100644
> --- a/docs/devel/testing.rst
> +++ b/docs/devel/testing.rst
> @@ -634,7 +634,45 @@ instance, available at ``self.vm``.  Because many tests 
> will tweak the
>   QEMU command line, launching the QEMUMachine (by using ``self.vm.launch()``)
>   is left to the test writer.
>   
> -At test "tear down", ``avocado_qemu.Test`` handles the QEMUMachine
> +The base test class has also support for tests with more than one
> +QEMUMachine. The way to get machines is through the ``self.get_vm()``
> +method which will return a QEMUMachine instance. The ``self.get_vm()``
> +method also accepts an optional `name` attribute so you can identify a
> +specific machine and get it more than once through the tests methods. A
> +simple and hypothetical example follows:

Since you explain the self.get_vm() optional name attribute, you also 
could mention it accepts arguments to be passed to the newly created VM.

> +
> +.. code::
> +
> +  from avocado_qemu import Test
> +
> +
> +  class MultipleMachines(Test):
> +      """
> +      :avocado: enable
> +      """
> +      def test_multiple_machines(self):
> +          first_machine = self.get_vm()
> +          second_machine = self.get_vm()
> +          self.get_vm(name='third_machine').launch()
> +
> +          first_machine.launch()
> +          second_machine.launch()
> +
> +          first_res = first_machine.command(
> +              'human-monitor-command',
> +              command_line='info version')
> +
> +          second_res = second_machine.command(
> +              'human-monitor-command',
> +              command_line='info version')
> +
> +          third_res = self.get_vm(name='third_machine').command(
> +              'human-monitor-command',
> +              command_line='info version')
> +
> +          self.assertEquals(first_res, second_res, third_res)
> +
> +At test "tear down", ``avocado_qemu.Test`` handles all the QEMUMachines
>   shutdown.
>   
>   QEMUMachine
> diff --git a/tests/acceptance/avocado_qemu/__init__.py 
> b/tests/acceptance/avocado_qemu/__init__.py
> index 1e54fd5932..4c9e27feda 100644
> --- a/tests/acceptance/avocado_qemu/__init__.py
> +++ b/tests/acceptance/avocado_qemu/__init__.py
> @@ -10,6 +10,7 @@
>   
>   import os
>   import sys
> +import uuid
>   
>   import avocado
>   
> @@ -42,13 +43,29 @@ def pick_default_qemu_bin():
>   
>   class Test(avocado.Test):
>       def setUp(self):
> -        self.vm = None
> +        self._vms = {}
>           self.qemu_bin = self.params.get('qemu_bin',
>                                           default=pick_default_qemu_bin())
>           if self.qemu_bin is None:
>               self.cancel("No QEMU binary defined or found in the source tree")
> -        self.vm = QEMUMachine(self.qemu_bin)
> +
> +    def _new_vm(self, *args):
> +        vm = QEMUMachine(self.qemu_bin)
> +        if args:
> +            vm.add_args(*args)
> +        return vm
> +
> +    @property
> +    def vm(self):
> +        return self.get_vm(name='default')
> +
> +    def get_vm(self, *args, name=None):
> +        if not name:
> +            name = str(uuid.uuid4())

Beware that if you don't give a name to the VM, the only way to access 
it later is to keep the reference returned by get_vm(). Do you think it 
is something we should care about? or assume the test writer handle this 
(unlikely?) case somehow?

- Wainer

> +        if self._vms.get(name) is None:
> +            self._vms[name] = self._new_vm(*args)
> +        return self._vms[name]
>   
>       def tearDown(self):
> -        if self.vm is not None:
> -            self.vm.shutdown()
> +        for vm in self._vms.values():
> +            vm.shutdown()