[2/3] Acceptance tests: introduce utility method for tags unique vals
diff mbox series

Message ID 20190924194501.9303-3-crosa@redhat.com
State New
Headers show
Series
  • Acceptance tests: make better use of machine tags
Related show

Commit Message

Cleber Rosa Sept. 24, 2019, 7:45 p.m. UTC
Currently a test can describe the target architecture binary that it
should primarily be run with, be setting a single tag value.

The same approach is expected to be done with other QEMU aspects to be
tested, for instance, the machine type and accelerator, so let's
generalize the logic into a utility method.

Signed-off-by: Cleber Rosa <crosa@redhat.com>
---
 tests/acceptance/avocado_qemu/__init__.py | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

Comments

Wainer dos Santos Moschetta Oct. 24, 2019, 9:12 p.m. UTC | #1
Hi Cleber,

On 9/24/19 4:45 PM, Cleber Rosa wrote:
> Currently a test can describe the target architecture binary that it
> should primarily be run with, be setting a single tag value.
>
> The same approach is expected to be done with other QEMU aspects to be
> tested, for instance, the machine type and accelerator, so let's
> generalize the logic into a utility method.
>
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>   tests/acceptance/avocado_qemu/__init__.py | 19 +++++++++++++------
>   1 file changed, 13 insertions(+), 6 deletions(-)
>
> diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py
> index bd41e0443c..02775bafcf 100644
> --- a/tests/acceptance/avocado_qemu/__init__.py
> +++ b/tests/acceptance/avocado_qemu/__init__.py
> @@ -54,14 +54,21 @@ def pick_default_qemu_bin(arch=None):
>   
>   
>   class Test(avocado.Test):
> +    def _get_unique_tag_val(self, tag_name):
> +        """
> +        Gets a tag value, if unique for a key
> +        """
> +        vals = self.tags.get(tag_name, [])
> +        if len(vals) == 1:


An small optimization:

if vals:

   return vals.pop()


> +            return vals.pop()
> +        return None

Does it allows to express a scenario like "I want my test method to run 
on x86_64 and aarch64" using tags? If so, _get_unique_tag_val logic 
returns None for multi-value tags (e.g. 'tags=arch:x86_64,arch:aarch64').

Thanks,

Wainer

> +
>       def setUp(self):
>           self._vms = {}
> -        arches = self.tags.get('arch', [])
> -        if len(arches) == 1:
> -            arch = arches.pop()
> -        else:
> -            arch = None
> -        self.arch = self.params.get('arch', default=arch)
> +
> +        self.arch = self.params.get('arch',
> +                                    default=self._get_unique_tag_val('arch'))
> +
>           default_qemu_bin = pick_default_qemu_bin(arch=self.arch)
>           self.qemu_bin = self.params.get('qemu_bin',
>                                           default=default_qemu_bin)
Cleber Rosa Oct. 28, 2019, 11:02 p.m. UTC | #2
On Thu, Oct 24, 2019 at 06:12:25PM -0300, Wainer dos Santos Moschetta wrote:
> Hi Cleber,
> 
> On 9/24/19 4:45 PM, Cleber Rosa wrote:
> > Currently a test can describe the target architecture binary that it
> > should primarily be run with, be setting a single tag value.
> > 
> > The same approach is expected to be done with other QEMU aspects to be
> > tested, for instance, the machine type and accelerator, so let's
> > generalize the logic into a utility method.
> > 
> > Signed-off-by: Cleber Rosa <crosa@redhat.com>
> > ---
> >   tests/acceptance/avocado_qemu/__init__.py | 19 +++++++++++++------
> >   1 file changed, 13 insertions(+), 6 deletions(-)
> > 
> > diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py
> > index bd41e0443c..02775bafcf 100644
> > --- a/tests/acceptance/avocado_qemu/__init__.py
> > +++ b/tests/acceptance/avocado_qemu/__init__.py
> > @@ -54,14 +54,21 @@ def pick_default_qemu_bin(arch=None):
> >   class Test(avocado.Test):
> > +    def _get_unique_tag_val(self, tag_name):
> > +        """
> > +        Gets a tag value, if unique for a key
> > +        """
> > +        vals = self.tags.get(tag_name, [])
> > +        if len(vals) == 1:
> 
> 
> An small optimization:
> 
> if vals:
> 
>   return vals.pop()
>

IIUC, this would break the idea of uniqueness that this method, for
now, has.  Read on.

> 
> > +            return vals.pop()
> > +        return None
> 
> Does it allows to express a scenario like "I want my test method to run on
> x86_64 and aarch64" using tags? If so, _get_unique_tag_val logic returns
> None for multi-value tags (e.g. 'tags=arch:x86_64,arch:aarch64').
>

I thought that initially we should attempt to pick a default arch or
machine type only of len(vals) == 1.  Not because what you describe
can't be done, but because I would like to go through the tests and
make sure we run them in all the given tagged arches when we allow
that.

Thanks,
- Cleber.

> Thanks,
> 
> Wainer
> 
> > +
> >       def setUp(self):
> >           self._vms = {}
> > -        arches = self.tags.get('arch', [])
> > -        if len(arches) == 1:
> > -            arch = arches.pop()
> > -        else:
> > -            arch = None
> > -        self.arch = self.params.get('arch', default=arch)
> > +
> > +        self.arch = self.params.get('arch',
> > +                                    default=self._get_unique_tag_val('arch'))
> > +
> >           default_qemu_bin = pick_default_qemu_bin(arch=self.arch)
> >           self.qemu_bin = self.params.get('qemu_bin',
> >                                           default=default_qemu_bin)
Wainer dos Santos Moschetta Nov. 7, 2019, 6:29 p.m. UTC | #3
On 10/28/19 8:02 PM, Cleber Rosa wrote:
> On Thu, Oct 24, 2019 at 06:12:25PM -0300, Wainer dos Santos Moschetta wrote:
>> Hi Cleber,
>>
>> On 9/24/19 4:45 PM, Cleber Rosa wrote:
>>> Currently a test can describe the target architecture binary that it
>>> should primarily be run with, be setting a single tag value.
>>>
>>> The same approach is expected to be done with other QEMU aspects to be
>>> tested, for instance, the machine type and accelerator, so let's
>>> generalize the logic into a utility method.
>>>
>>> Signed-off-by: Cleber Rosa <crosa@redhat.com>
>>> ---
>>>    tests/acceptance/avocado_qemu/__init__.py | 19 +++++++++++++------
>>>    1 file changed, 13 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py
>>> index bd41e0443c..02775bafcf 100644
>>> --- a/tests/acceptance/avocado_qemu/__init__.py
>>> +++ b/tests/acceptance/avocado_qemu/__init__.py
>>> @@ -54,14 +54,21 @@ def pick_default_qemu_bin(arch=None):
>>>    class Test(avocado.Test):
>>> +    def _get_unique_tag_val(self, tag_name):
>>> +        """
>>> +        Gets a tag value, if unique for a key
>>> +        """
>>> +        vals = self.tags.get(tag_name, [])
>>> +        if len(vals) == 1:
>>
>> An small optimization:
>>
>> if vals:
>>
>>    return vals.pop()
>>
> IIUC, this would break the idea of uniqueness that this method, for
> now, has.  Read on.
>
>>> +            return vals.pop()
>>> +        return None
>> Does it allows to express a scenario like "I want my test method to run on
>> x86_64 and aarch64" using tags? If so, _get_unique_tag_val logic returns
>> None for multi-value tags (e.g. 'tags=arch:x86_64,arch:aarch64').
>>
> I thought that initially we should attempt to pick a default arch or
> machine type only of len(vals) == 1.  Not because what you describe
> can't be done, but because I would like to go through the tests and
> make sure we run them in all the given tagged arches when we allow
> that.

Ok, understood the rationale now.

Reviewed-by: Wainer dos Santos Moschetta <wainersm@redhat.com>

>
> Thanks,
> - Cleber.
>
>> Thanks,
>>
>> Wainer
>>
>>> +
>>>        def setUp(self):
>>>            self._vms = {}
>>> -        arches = self.tags.get('arch', [])
>>> -        if len(arches) == 1:
>>> -            arch = arches.pop()
>>> -        else:
>>> -            arch = None
>>> -        self.arch = self.params.get('arch', default=arch)
>>> +
>>> +        self.arch = self.params.get('arch',
>>> +                                    default=self._get_unique_tag_val('arch'))
>>> +
>>>            default_qemu_bin = pick_default_qemu_bin(arch=self.arch)
>>>            self.qemu_bin = self.params.get('qemu_bin',
>>>                                            default=default_qemu_bin)
>

Patch
diff mbox series

diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py
index bd41e0443c..02775bafcf 100644
--- a/tests/acceptance/avocado_qemu/__init__.py
+++ b/tests/acceptance/avocado_qemu/__init__.py
@@ -54,14 +54,21 @@  def pick_default_qemu_bin(arch=None):
 
 
 class Test(avocado.Test):
+    def _get_unique_tag_val(self, tag_name):
+        """
+        Gets a tag value, if unique for a key
+        """
+        vals = self.tags.get(tag_name, [])
+        if len(vals) == 1:
+            return vals.pop()
+        return None
+
     def setUp(self):
         self._vms = {}
-        arches = self.tags.get('arch', [])
-        if len(arches) == 1:
-            arch = arches.pop()
-        else:
-            arch = None
-        self.arch = self.params.get('arch', default=arch)
+
+        self.arch = self.params.get('arch',
+                                    default=self._get_unique_tag_val('arch'))
+
         default_qemu_bin = pick_default_qemu_bin(arch=self.arch)
         self.qemu_bin = self.params.get('qemu_bin',
                                         default=default_qemu_bin)