Re: [PATCH v3 1/5] tests/acceptance: Extract QemuBaseTest from Test

[Wainer dos Santos Moschetta]

Hi,

On 3/15/21 8:08 PM, Philippe Mathieu-Daudé wrote:
> The Avocado Test::fetch_asset() is handy to download artifacts
> before running tests. The current class is named Test but only
> tests system emulation. As we want to test user emulation,
> refactor the common code as QemuBaseTest.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>   tests/acceptance/avocado_qemu/__init__.py | 23 ++++++++++++++++++++---
>   1 file changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/tests/acceptance/avocado_qemu/__init__.py 
> b/tests/acceptance/avocado_qemu/__init__.py
> index df167b142cc..4f814047176 100644
> --- a/tests/acceptance/avocado_qemu/__init__.py
> +++ b/tests/acceptance/avocado_qemu/__init__.py
> @@ -155,7 +155,7 @@ def exec_command_and_wait_for_pattern(test, command,
>       """
>       _console_interaction(test, success_message, failure_message, command + 
> '\r')
>   
> -class Test(avocado.Test):
> +class QemuBaseTest(avocado.Test):

The QemuBaseTest class still defines require_accelerator() which is only 
used by qemu-system tests (thus, it should rather live on the Test 
class). Same thing for self.machine, unless that property is used on 
qemu-user tests.

>       def _get_unique_tag_val(self, tag_name):
>           """
>           Gets a tag value, if unique for a key
> @@ -188,8 +188,6 @@ def require_accelerator(self, accelerator):
>                           "available" % accelerator)
>   
>       def setUp(self):
> -        self._vms = {}
> -
>           self.arch = self.params.get('arch',
>                                       default=self._get_unique_tag_val('arch'))
>   
> @@ -202,6 +200,25 @@ def setUp(self):
>           if self.qemu_bin is None:
>               self.cancel("No QEMU binary defined or found in the build tree")
>   
> +
> +    def fetch_asset(self, name,
> +                    asset_hash=None, algorithm=None,
> +                    locations=None, expire=None,
> +                    find_only=False, cancel_on_missing=True):
> +        return super(QemuBaseTest, self).fetch_asset(name,
> +                        asset_hash=asset_hash,
> +                        algorithm=algorithm,
> +                        locations=locations,
> +                        expire=expire,
> +                        find_only=find_only,
> +                        cancel_on_missing=cancel_on_missing)
Do you overwrite this fetch_asset() on class Test on purpose? I didn't 
get why fetch_asset() is defined on the classes inherited from QemuBaseTest.
> +
> +# a.k.a. QemuSystemTest for system emulation...
Above comment could become the class docstring.
> +class Test(QemuBaseTest):
> +    def setUp(self):
> +        self._vms = {}
> +        super(Test, self).setUp()
> +
>       def _new_vm(self, *args):
>           self._sd = tempfile.TemporaryDirectory(prefix="avo_qemu_sock_")
>           vm = QEMUMachine(self.qemu_bin, sock_dir=self._sd.name)