diff mbox series

[2/7] Acceptance Tests: introduce arch parameter and attribute

Message ID 20181004151429.7232-3-crosa@redhat.com
State New
Headers show
Series Acceptance Tests: basic architecture support | expand

Commit Message

Cleber Rosa Oct. 4, 2018, 3:14 p.m. UTC
On a number of different scenarios, such as when choosing a QEMU
binary to be used on tests (or a image to use to boot a test VM), it's
useful to define the architecture that should be used.

This introduces both a test parameter and a test instance attribute,
that will contain such a value.

The selection of the default QEMU binary, if one is not given
explicitly, has also been changed to look at the arch attribute.

Signed-off-by: Cleber Rosa <crosa@redhat.com>
---
 docs/devel/testing.rst                    | 18 ++++++++++++++++++
 tests/acceptance/avocado_qemu/__init__.py | 13 ++++++++++---
 2 files changed, 28 insertions(+), 3 deletions(-)

Comments

Murilo Opsfelder Araújo Oct. 4, 2018, 11:56 p.m. UTC | #1
Hi, Cleber.

On Thu, Oct 04, 2018 at 11:14:24AM -0400, Cleber Rosa wrote:
> On a number of different scenarios, such as when choosing a QEMU
> binary to be used on tests (or a image to use to boot a test VM), it's
> useful to define the architecture that should be used.
>
> This introduces both a test parameter and a test instance attribute,
> that will contain such a value.
>
> The selection of the default QEMU binary, if one is not given
> explicitly, has also been changed to look at the arch attribute.
>
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>  docs/devel/testing.rst                    | 18 ++++++++++++++++++
>  tests/acceptance/avocado_qemu/__init__.py | 13 ++++++++++---
>  2 files changed, 28 insertions(+), 3 deletions(-)
>
> diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst
> index 727c4019b5..4178ae5022 100644
> --- a/docs/devel/testing.rst
> +++ b/docs/devel/testing.rst
> @@ -659,6 +659,17 @@ vm
>  A QEMUMachine instance, initially configured according to the given
>  ``qemu_bin`` parameter.
>
> +arch
> +~~~~
> +
> +The architecture that will be used on a number of different
> +scenarios.  For instance, when a QEMU binary is not explicitly given,
> +the one selected will depend on this attribute.
> +
> +The ``arch`` attribute will be set to the test parameter of the same
> +name, and if one is not given explicitly, it will default to the
> +system architecture (as given by ``uname``).
> +
>  qemu_bin
>  ~~~~~~~~
>
> @@ -681,6 +692,13 @@ like the following:
>
>    PARAMS (key=qemu_bin, path=*, default=x86_64-softmmu/qemu-system-x86_64) => 'x86_64-softmmu/qemu-system-x86_64
>
> +arch
> +~~~~
> +
> +The architecture that will be used on a number of different scenarios.
> +This parameter has a direct relation with the ``arch`` attribute.  If
> +not given, it will default to None.
> +
>  qemu_bin
>  ~~~~~~~~
>
> diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py
> index d8d5b48dac..9329d9b9ec 100644
> --- a/tests/acceptance/avocado_qemu/__init__.py
> +++ b/tests/acceptance/avocado_qemu/__init__.py
> @@ -23,16 +23,22 @@ def is_readable_executable_file(path):
>      return os.path.isfile(path) and os.access(path, os.R_OK | os.X_OK)
>
>
> -def pick_default_qemu_bin():
> +def pick_default_qemu_bin(arch=None):
>      """
>      Picks the path of a QEMU binary, starting either in the current working
>      directory or in the source tree root directory.
>
> +    :param arch: the arch to use when looking for a QEMU binary (the target
> +                 will match the arch given).  If None (the default) arch
> +                 will be the current host system arch (as given by
> +                 :func:`os.uname`).
> +    :param arch: str

Do you mean

    :type arch: str

?

>      :returns: the path to the default QEMU binary or None if one could not
>                be found
>      :rtype: str or None
>      """
> -    arch = os.uname()[4]
> +    if arch is None:
> +        arch = os.uname()[4]
>      qemu_bin_relative_path = os.path.join("%s-softmmu" % arch,
>                                            "qemu-system-%s" % arch)

On ppc64le systems, this can lead to an unexisting path.

For instance, os.uname() returns:

    >>> os.uname()
    ('Linux', 'localhost', '4.15.0-34-generic', '#37-Ubuntu SMP Mon Aug 27 15:18:58 UTC 2018', 'ppc64le')

Then, qemu_bin_relative_path would be set to:

    ppc64le-softmmu/qemu-system-ppc64le

However, the correct path is:

    ppc64-softmmu/qemu-system-ppc64

I'm not sure if ppc64le is the only case where the target name is different from
the OS architecture.

If you decide not to handle this now, at least add a FIXME, if possible.

>      if is_readable_executable_file(qemu_bin_relative_path):
> @@ -47,8 +53,9 @@ def pick_default_qemu_bin():
>  class Test(avocado.Test):
>      def setUp(self):
>          self.vm = None
> +        self.arch = self.params.get('arch', default=os.uname()[4])
>          self.qemu_bin = self.params.get('qemu_bin',
> -                                        default=pick_default_qemu_bin())
> +                                        default=pick_default_qemu_bin(self.arch))
>          if self.qemu_bin is None:
>              self.cancel("No QEMU binary defined or found in the source tree")
>          self.vm = QEMUMachine(self.qemu_bin)
> --
> 2.17.1
>
>

--
Murilo
Cleber Rosa Oct. 10, 2018, 1:16 p.m. UTC | #2
On 10/4/18 7:56 PM, Murilo Opsfelder Araujo wrote:
> Hi, Cleber.
> 
> On Thu, Oct 04, 2018 at 11:14:24AM -0400, Cleber Rosa wrote:
>> On a number of different scenarios, such as when choosing a QEMU
>> binary to be used on tests (or a image to use to boot a test VM), it's
>> useful to define the architecture that should be used.
>>
>> This introduces both a test parameter and a test instance attribute,
>> that will contain such a value.
>>
>> The selection of the default QEMU binary, if one is not given
>> explicitly, has also been changed to look at the arch attribute.
>>
>> Signed-off-by: Cleber Rosa <crosa@redhat.com>
>> ---
>>  docs/devel/testing.rst                    | 18 ++++++++++++++++++
>>  tests/acceptance/avocado_qemu/__init__.py | 13 ++++++++++---
>>  2 files changed, 28 insertions(+), 3 deletions(-)
>>
>> diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst
>> index 727c4019b5..4178ae5022 100644
>> --- a/docs/devel/testing.rst
>> +++ b/docs/devel/testing.rst
>> @@ -659,6 +659,17 @@ vm
>>  A QEMUMachine instance, initially configured according to the given
>>  ``qemu_bin`` parameter.
>>
>> +arch
>> +~~~~
>> +
>> +The architecture that will be used on a number of different
>> +scenarios.  For instance, when a QEMU binary is not explicitly given,
>> +the one selected will depend on this attribute.
>> +
>> +The ``arch`` attribute will be set to the test parameter of the same
>> +name, and if one is not given explicitly, it will default to the
>> +system architecture (as given by ``uname``).
>> +
>>  qemu_bin
>>  ~~~~~~~~
>>
>> @@ -681,6 +692,13 @@ like the following:
>>
>>    PARAMS (key=qemu_bin, path=*, default=x86_64-softmmu/qemu-system-x86_64) => 'x86_64-softmmu/qemu-system-x86_64
>>
>> +arch
>> +~~~~
>> +
>> +The architecture that will be used on a number of different scenarios.
>> +This parameter has a direct relation with the ``arch`` attribute.  If
>> +not given, it will default to None.
>> +
>>  qemu_bin
>>  ~~~~~~~~
>>
>> diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py
>> index d8d5b48dac..9329d9b9ec 100644
>> --- a/tests/acceptance/avocado_qemu/__init__.py
>> +++ b/tests/acceptance/avocado_qemu/__init__.py
>> @@ -23,16 +23,22 @@ def is_readable_executable_file(path):
>>      return os.path.isfile(path) and os.access(path, os.R_OK | os.X_OK)
>>
>>
>> -def pick_default_qemu_bin():
>> +def pick_default_qemu_bin(arch=None):
>>      """
>>      Picks the path of a QEMU binary, starting either in the current working
>>      directory or in the source tree root directory.
>>
>> +    :param arch: the arch to use when looking for a QEMU binary (the target
>> +                 will match the arch given).  If None (the default) arch
>> +                 will be the current host system arch (as given by
>> +                 :func:`os.uname`).
>> +    :param arch: str
> 
> Do you mean
> 
>     :type arch: str
> 
> ?
> 

Hi Murilo,

Yes, thanks for catching that.

>>      :returns: the path to the default QEMU binary or None if one could not
>>                be found
>>      :rtype: str or None
>>      """
>> -    arch = os.uname()[4]
>> +    if arch is None:
>> +        arch = os.uname()[4]
>>      qemu_bin_relative_path = os.path.join("%s-softmmu" % arch,
>>                                            "qemu-system-%s" % arch)
> 
> On ppc64le systems, this can lead to an unexisting path.
> 
> For instance, os.uname() returns:
> 
>     >>> os.uname()
>     ('Linux', 'localhost', '4.15.0-34-generic', '#37-Ubuntu SMP Mon Aug 27 15:18:58 UTC 2018', 'ppc64le')
> 
> Then, qemu_bin_relative_path would be set to:
> 
>     ppc64le-softmmu/qemu-system-ppc64le
> 
> However, the correct path is:
> 
>     ppc64-softmmu/qemu-system-ppc64
> 
> I'm not sure if ppc64le is the only case where the target name is different from
> the OS architecture.
> 
> If you decide not to handle this now, at least add a FIXME, if possible.
> 

You're correct, but this behavior is also unrelated to the *this*
specific change (the uname() based arch was already being used). Because
of that I'd rather send the fix in another series.

To track the fix I've created the following card:

https://trello.com/c/0quVUfKi/48-support-target-binary-selection-on-ppc64le

Thanks!
- Cleber.

>>      if is_readable_executable_file(qemu_bin_relative_path):
>> @@ -47,8 +53,9 @@ def pick_default_qemu_bin():
>>  class Test(avocado.Test):
>>      def setUp(self):
>>          self.vm = None
>> +        self.arch = self.params.get('arch', default=os.uname()[4])
>>          self.qemu_bin = self.params.get('qemu_bin',
>> -                                        default=pick_default_qemu_bin())
>> +                                        default=pick_default_qemu_bin(self.arch))
>>          if self.qemu_bin is None:
>>              self.cancel("No QEMU binary defined or found in the source tree")
>>          self.vm = QEMUMachine(self.qemu_bin)
>> --
>> 2.17.1
>>
>>
> 
> --
> Murilo
>
diff mbox series

Patch

diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst
index 727c4019b5..4178ae5022 100644
--- a/docs/devel/testing.rst
+++ b/docs/devel/testing.rst
@@ -659,6 +659,17 @@  vm
 A QEMUMachine instance, initially configured according to the given
 ``qemu_bin`` parameter.
 
+arch
+~~~~
+
+The architecture that will be used on a number of different
+scenarios.  For instance, when a QEMU binary is not explicitly given,
+the one selected will depend on this attribute.
+
+The ``arch`` attribute will be set to the test parameter of the same
+name, and if one is not given explicitly, it will default to the
+system architecture (as given by ``uname``).
+
 qemu_bin
 ~~~~~~~~
 
@@ -681,6 +692,13 @@  like the following:
 
   PARAMS (key=qemu_bin, path=*, default=x86_64-softmmu/qemu-system-x86_64) => 'x86_64-softmmu/qemu-system-x86_64
 
+arch
+~~~~
+
+The architecture that will be used on a number of different scenarios.
+This parameter has a direct relation with the ``arch`` attribute.  If
+not given, it will default to None.
+
 qemu_bin
 ~~~~~~~~
 
diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py
index d8d5b48dac..9329d9b9ec 100644
--- a/tests/acceptance/avocado_qemu/__init__.py
+++ b/tests/acceptance/avocado_qemu/__init__.py
@@ -23,16 +23,22 @@  def is_readable_executable_file(path):
     return os.path.isfile(path) and os.access(path, os.R_OK | os.X_OK)
 
 
-def pick_default_qemu_bin():
+def pick_default_qemu_bin(arch=None):
     """
     Picks the path of a QEMU binary, starting either in the current working
     directory or in the source tree root directory.
 
+    :param arch: the arch to use when looking for a QEMU binary (the target
+                 will match the arch given).  If None (the default) arch
+                 will be the current host system arch (as given by
+                 :func:`os.uname`).
+    :param arch: str
     :returns: the path to the default QEMU binary or None if one could not
               be found
     :rtype: str or None
     """
-    arch = os.uname()[4]
+    if arch is None:
+        arch = os.uname()[4]
     qemu_bin_relative_path = os.path.join("%s-softmmu" % arch,
                                           "qemu-system-%s" % arch)
     if is_readable_executable_file(qemu_bin_relative_path):
@@ -47,8 +53,9 @@  def pick_default_qemu_bin():
 class Test(avocado.Test):
     def setUp(self):
         self.vm = None
+        self.arch = self.params.get('arch', default=os.uname()[4])
         self.qemu_bin = self.params.get('qemu_bin',
-                                        default=pick_default_qemu_bin())
+                                        default=pick_default_qemu_bin(self.arch))
         if self.qemu_bin is None:
             self.cancel("No QEMU binary defined or found in the source tree")
         self.vm = QEMUMachine(self.qemu_bin)