Re: [PATCH 06/26] tests/acceptance: Rename avocado_qemu.Test as MachineTest

From: Wainer dos Santos Moschetta <wainersm@redhat.com>
To: "Philippe Mathieu-Daudé" <philmd@redhat.com>, qemu-devel@nongnu.org
Cc: "Fam Zheng" <fam@euphon.net>,
	"Eduardo Habkost" <ehabkost@redhat.com>,
	"KONRAD Frederic" <frederic.konrad@adacore.com>,
	"Fabien Chouteau" <chouteau@adacore.com>,
	"Kamil Rytarowski" <kamil@netbsd.org>,
	"Cleber Rosa" <crosa@redhat.com>,
	"Aleksandar Rikalo" <aleksandar.rikalo@rt-rk.com>,
	"Alex Bennée" <alex.bennee@linaro.org>,
	"Aurelien Jarno" <aurelien@aurel32.net>
Subject: Re: [PATCH 06/26] tests/acceptance: Rename avocado_qemu.Test as MachineTest
Date: Tue, 19 Nov 2019 11:52:14 -0200	[thread overview]
Message-ID: <91987b1a-c20f-c5ae-1240-1743c16a8d71@redhat.com> (raw)
In-Reply-To: <20191028073441.6448-7-philmd@redhat.com>

Hi Philippe,

On 10/28/19 4:34 AM, Philippe Mathieu-Daudé wrote:
> This class is used to test QEMU machines, rename it as MachineTest.
> This will allow us to add a UserTest class for qemu-user tests.

My concern with this rename is that usually "Machine" refers to machine 
type implementation. What if we follow QEMU's terminology of User vs 
System? So it would be renamed to something like SystemTest (vs 
LinuxUserTest as you propose on Patch 08).

Thanks,

Wainer

>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>   docs/devel/testing.rst                    | 8 ++++----
>   tests/acceptance/avocado_qemu/__init__.py | 7 ++++++-
>   tests/acceptance/boot_linux_console.py    | 4 ++--
>   tests/acceptance/cpu_queries.py           | 4 ++--
>   tests/acceptance/empty_cpu_model.py       | 4 ++--
>   tests/acceptance/linux_initrd.py          | 4 ++--
>   tests/acceptance/linux_ssh_mips_malta.py  | 4 ++--
>   tests/acceptance/migration.py             | 4 ++--
>   tests/acceptance/version.py               | 4 ++--
>   tests/acceptance/virtio_version.py        | 4 ++--
>   tests/acceptance/vnc.py                   | 4 ++--
>   11 files changed, 28 insertions(+), 23 deletions(-)
>
> diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst
> index 8e981e062d..d9fab83458 100644
> --- a/docs/devel/testing.rst
> +++ b/docs/devel/testing.rst
> @@ -601,7 +601,7 @@ class.  Here's a simple usage example:
>     from avocado_qemu import Test
>   
>   
> -  class Version(Test):
> +  class Version(MachineTest):
>         """
>         :avocado: tags=quick
>         """
> @@ -625,7 +625,7 @@ in the current directory, tagged as "quick", run:
>   
>     avocado run -t quick .
>   
> -The ``avocado_qemu.Test`` base test class
> +The ``avocado_qemu.MachineTest`` base test class
>   -----------------------------------------
>   
>   The ``avocado_qemu.Test`` class has a number of characteristics that
> @@ -646,10 +646,10 @@ and hypothetical example follows:
>   
>   .. code::
>   
> -  from avocado_qemu import Test
> +  from avocado_qemu import MachineTest
>   
>   
> -  class MultipleMachines(Test):
> +  class MultipleMachines(MachineTest):
>         """
>         :avocado: enable
>         """
> diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py
> index bdece76723..a2cc3d689b 100644
> --- a/tests/acceptance/avocado_qemu/__init__.py
> +++ b/tests/acceptance/avocado_qemu/__init__.py
> @@ -100,7 +100,6 @@ def exec_command_and_wait_for_pattern(test, command,
>   
>   class Test(avocado.Test):
>       def setUp(self):
> -        self._vms = {}
>           arches = self.tags.get('arch', [])
>           if len(arches) == 1:
>               arch = arches.pop()
> @@ -113,6 +112,12 @@ class Test(avocado.Test):
>           if self.qemu_bin is None:
>               self.cancel("No QEMU binary defined or found in the source tree")
>   
> +
> +class MachineTest(Test):
> +    def setUp(self):
> +        self._vms = {}
> +        super().setUp()
> +
>       def _new_vm(self, *args):
>           vm = QEMUMachine(self.qemu_bin)
>           if args:
> diff --git a/tests/acceptance/boot_linux_console.py b/tests/acceptance/boot_linux_console.py
> index 4b419b0559..f94dc4bbca 100644
> --- a/tests/acceptance/boot_linux_console.py
> +++ b/tests/acceptance/boot_linux_console.py
> @@ -13,14 +13,14 @@ import lzma
>   import gzip
>   import shutil
>   
> -from avocado_qemu import Test
> +from avocado_qemu import MachineTest
>   from avocado_qemu import exec_command_and_wait_for_pattern
>   from avocado_qemu import wait_for_console_pattern
>   from avocado.utils import process
>   from avocado.utils import archive
>   
>   
> -class BootLinuxConsole(Test):
> +class BootLinuxConsole(MachineTest):
>       """
>       Boots a Linux kernel and checks that the console is operational and the
>       kernel command line is properly passed from QEMU to the kernel
> diff --git a/tests/acceptance/cpu_queries.py b/tests/acceptance/cpu_queries.py
> index af47d2795a..40df8264cf 100644
> --- a/tests/acceptance/cpu_queries.py
> +++ b/tests/acceptance/cpu_queries.py
> @@ -10,9 +10,9 @@
>   
>   import logging
>   
> -from avocado_qemu import Test
> +from avocado_qemu import MachineTest
>   
> -class QueryCPUModelExpansion(Test):
> +class QueryCPUModelExpansion(MachineTest):
>       """
>       Run query-cpu-model-expansion for each CPU model, and validate results
>       """
> diff --git a/tests/acceptance/empty_cpu_model.py b/tests/acceptance/empty_cpu_model.py
> index 3f4f663582..a4e9cc62f8 100644
> --- a/tests/acceptance/empty_cpu_model.py
> +++ b/tests/acceptance/empty_cpu_model.py
> @@ -8,9 +8,9 @@
>   # 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 subprocess
> -from avocado_qemu import Test
> +from avocado_qemu import MachineTest
>   
> -class EmptyCPUModel(Test):
> +class EmptyCPUModel(MachineTest):
>       def test(self):
>           cmd = [self.qemu_bin, '-S', '-display', 'none', '-machine', 'none', '-cpu', '']
>           r = subprocess.run(cmd, stderr=subprocess.PIPE, stdout=subprocess.PIPE)
> diff --git a/tests/acceptance/linux_initrd.py b/tests/acceptance/linux_initrd.py
> index c61d9826a4..e8afb007d0 100644
> --- a/tests/acceptance/linux_initrd.py
> +++ b/tests/acceptance/linux_initrd.py
> @@ -12,10 +12,10 @@ import logging
>   import tempfile
>   from avocado.utils.process import run
>   
> -from avocado_qemu import Test
> +from avocado_qemu import MachineTest
>   
>   
> -class LinuxInitrd(Test):
> +class LinuxInitrd(MachineTest):
>       """
>       Checks QEMU evaluates correctly the initrd file passed as -initrd option.
>   
> diff --git a/tests/acceptance/linux_ssh_mips_malta.py b/tests/acceptance/linux_ssh_mips_malta.py
> index fc13f9e4d4..5db64affda 100644
> --- a/tests/acceptance/linux_ssh_mips_malta.py
> +++ b/tests/acceptance/linux_ssh_mips_malta.py
> @@ -12,14 +12,14 @@ import logging
>   import time
>   
>   from avocado import skipUnless
> -from avocado_qemu import Test
> +from avocado_qemu import MachineTest
>   from avocado_qemu import wait_for_console_pattern
>   from avocado.utils import process
>   from avocado.utils import archive
>   from avocado.utils import ssh
>   
>   
> -class LinuxSSH(Test):
> +class LinuxSSH(MachineTest):
>   
>       timeout = 150 # Not for 'configure --enable-debug --enable-debug-tcg'
>   
> diff --git a/tests/acceptance/migration.py b/tests/acceptance/migration.py
> index a44c1ae58f..3a823c9cc7 100644
> --- a/tests/acceptance/migration.py
> +++ b/tests/acceptance/migration.py
> @@ -10,13 +10,13 @@
>   # later.  See the COPYING file in the top-level directory.
>   
>   
> -from avocado_qemu import Test
> +from avocado_qemu import MachineTest
>   
>   from avocado.utils import network
>   from avocado.utils import wait
>   
>   
> -class Migration(Test):
> +class Migration(MachineTest):
>   
>       timeout = 10
>   
> diff --git a/tests/acceptance/version.py b/tests/acceptance/version.py
> index 67c2192c93..e11661f780 100644
> --- a/tests/acceptance/version.py
> +++ b/tests/acceptance/version.py
> @@ -9,10 +9,10 @@
>   # later.  See the COPYING file in the top-level directory.
>   
>   
> -from avocado_qemu import Test
> +from avocado_qemu import MachineTest
>   
>   
> -class Version(Test):
> +class Version(MachineTest):
>       """
>       :avocado: tags=quick
>       """
> diff --git a/tests/acceptance/virtio_version.py b/tests/acceptance/virtio_version.py
> index 33593c29dd..ea33363b0f 100644
> --- a/tests/acceptance/virtio_version.py
> +++ b/tests/acceptance/virtio_version.py
> @@ -13,7 +13,7 @@ import os
>   
>   sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
>   from qemu.machine import QEMUMachine
> -from avocado_qemu import Test
> +from avocado_qemu import MachineTest
>   
>   # Virtio Device IDs:
>   VIRTIO_NET = 1
> @@ -55,7 +55,7 @@ def get_pci_interfaces(vm, devtype):
>       interfaces = ('pci-express-device', 'conventional-pci-device')
>       return [i for i in interfaces if devtype_implements(vm, devtype, i)]
>   
> -class VirtioVersionCheck(Test):
> +class VirtioVersionCheck(MachineTest):
>       """
>       Check if virtio-version-specific device types result in the
>       same device tree created by `disable-modern` and
> diff --git a/tests/acceptance/vnc.py b/tests/acceptance/vnc.py
> index 3f40bc2be1..c2a364d23e 100644
> --- a/tests/acceptance/vnc.py
> +++ b/tests/acceptance/vnc.py
> @@ -8,10 +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.
>   
> -from avocado_qemu import Test
> +from avocado_qemu import MachineTest
>   
>   
> -class Vnc(Test):
> +class Vnc(MachineTest):
>       """
>       :avocado: tags=vnc,quick
>       """