Re: [PATCH 1/2] tests/avocado: Invert parameter vs. tag precedence during setUp

[Daniel Henrique Barboza]

On 1/20/23 19:14, Fabiano Rosas wrote:
> We currently never pass parameters to the avocado process via
> Makefile. To start doing so we need to invert the precedence between
> command line parameters and tags, otherwise a command line parameter
> would override values for all the tests, which is unlikely to be a
> common use-case.
>
> A more likely use-case is to force certain values for the tests that
> have no tags. For instance, if a test has no 'arch' tags and therefore
> can run for all targets, one could possibly force it to run on a
> certain target with an arch=foo parameter.
>
> This applies to the variables set during setUp(): arch, machine, cpu,
> distro_name, distro_version. Parameters used directly in tests or read
> via self.params.get are left unchanged.
>
> Signed-off-by: Fabiano Rosas <farosas@suse.de>
> ---

Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>

>   tests/avocado/avocado_qemu/__init__.py | 32 +++++++++++++++-----------
>   1 file changed, 19 insertions(+), 13 deletions(-)
>
> diff --git a/tests/avocado/avocado_qemu/__init__.py 
> b/tests/avocado/avocado_qemu/__init__.py
> index 910f3ba1ea..a181cac383 100644
> --- a/tests/avocado/avocado_qemu/__init__.py
> +++ b/tests/avocado/avocado_qemu/__init__.py
> @@ -240,12 +240,23 @@ def _get_unique_tag_val(self, tag_name):
>               return vals.pop()
>           return None
>   
> +    def _get_prop(self, name):
> +        """
> +        Infer test properties based on tags. If no tag is present,
> +        look for a command line parameter of the same name.
> +        """
> +        val = self._get_unique_tag_val(name)
> +        if not val:
> +            # If there's no tag, look for a command line
> +            # parameter. This allows the user to override any defaults
> +            # the caller of this function would choose if we were to
> +            # return None.
> +            val = self.params.get(name)
> +        return val
> +
>       def setUp(self, bin_prefix):
> -        self.arch = self.params.get('arch',
> -                                    default=self._get_unique_tag_val('arch'))
> -
> -        self.cpu = self.params.get('cpu',
> -                                   default=self._get_unique_tag_val('cpu'))
> +        self.arch = self._get_prop('arch')
> +        self.cpu = self._get_prop('cpu')
>   
>           default_qemu_bin = pick_default_qemu_bin(bin_prefix, arch=self.arch)
>           self.qemu_bin = self.params.get('qemu_bin',
> @@ -274,8 +285,7 @@ def setUp(self):
>   
>           super().setUp('qemu-system-')
>   
> -        self.machine = self.params.get('machine',
> -                                       
> default=self._get_unique_tag_val('machine'))
> +        self.machine = self._get_prop('machine')
>   
>       def require_accelerator(self, accelerator):
>           """
> @@ -529,15 +539,11 @@ class LinuxTest(LinuxSSHMixIn, QemuSystemTest):
>       memory = '1024'
>   
>       def _set_distro(self):
> -        distro_name = self.params.get(
> -            'distro',
> -            default=self._get_unique_tag_val('distro'))
> +        distro_name = self._get_prop('distro')
>           if not distro_name:
>               distro_name = 'fedora'
>   
> -        distro_version = self.params.get(
> -            'distro_version',
> -            default=self._get_unique_tag_val('distro_version'))
> +        distro_version = self._get_prop('distro_version')
>           if not distro_version:
>               distro_version = '31'
>