Re: [Qemu-devel] [RFC PATCH 3/3] acceptance tests: Add EDK2 AAVMF boot and console checking test

[Cleber Rosa <crosa@redhat.com>]

On 9/27/18 8:30 PM, Philippe Mathieu-Daudé wrote:
> This test boots EDK2 AAVMF and check the debug console (PL011) reports enough
> information on the initialized devices.
> 
> Example:
> 
>     $ avocado --show=console run tests/acceptance/boot_firmware.py --cit-parameter-file aarch64.cit
> 
> having aarch64.cit:
> 
> [parameters]
> qemu_bin: aarch64-softmmu/qemu-system-aarch64
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> - requires (even for single parameter):
>   $ pip install avocado-framework-plugin-varianter-cit
> 

Avocado 65.0 now features a core parameter passing option (one that
doesn't depend on the varianter).  Command line option is documented as:

  -p NAME_VALUE, --test-parameter NAME_VALUE
                Parameter name and value to pass to all tests. This is
                only applicable when not using a varianter plugin.
                This option format must be given in the NAME=VALUE
                format, and may be given any number of times, or per
                parameter.

> - use gzip kludge (avocado only support tarballs)
> 

I'll finish the work to get that into the avocado.utils.archive module.

> - can not set $arch param since pick_default_qemu_bin() forces to host os.uname()[4]

This is definitely a common requirement, and something to be addressed
ASAP.  I have a patch on my tree (pretty simple change) that I'll be
sending shortly.

> ---
>  tests/acceptance/boot_firmware.py | 43 +++++++++++++++++++++++++++++++
>  1 file changed, 43 insertions(+)
> 
> diff --git a/tests/acceptance/boot_firmware.py b/tests/acceptance/boot_firmware.py
> index 05f6728212..4f5554c0ad 100644
> --- a/tests/acceptance/boot_firmware.py
> +++ b/tests/acceptance/boot_firmware.py
> @@ -10,6 +10,7 @@
>  # SPDX-License-Identifier: GPL-2.0-or-later
>  
>  import os
> +import gzip
>  import time
>  import shutil
>  import logging
> @@ -122,3 +123,45 @@ class BootFirmwareX86(Test):
>              if line + '\r\n' not in lines:
>                  self.fail('missing: %s' % line)
>          shutil.rmtree(tmpdirname, ignore_errors=True)
> +
> +
> +class BootFirmwareAarch64(Test):
> +    """
> +    Boots the EDK2 firmware on a default virt machine and checks the console is
> +    operational
> +
> +    :avocado: enable
> +    :avocado: tags=arch:aarch64

Yay, I like where you're going here.  This matches what I said before
about having some standardization on the tags.

On a related topic, sometimes tests can be written in an abstract way
that parameters only will allow the same test to be reused in different
arches, but, sometimes we may need to explicit include/exclude some
tests on some executions - then tags could be a foundation for that.

> +    :avocado: tags=aarch64,quick
> +    """
> +
> +    timeout = 15
> +
> +    def test_aavmf(self):
> +        tmpdirname = tempfile.mkdtemp()
> +        image_url = ('http://snapshots.linaro.org/components/kernel/'
> +                    'leg-virt-tianocore-edk2-upstream/latest/'
> +                    'QEMU-AARCH64/DEBUG_GCC5/QEMU_EFI.img.gz')
> +        image_path_gz = self.fetch_asset(image_url)
> +        image_path = os.path.join(tmpdirname, 'flash.img')
> +        with gzip.open(image_path_gz) as gz, open(image_path, 'wb') as img:
> +            shutil.copyfileobj(gz, img)
> +
> +        self.vm.set_machine('virt')
> +        self.vm.set_console()
> +        self.vm.add_args('-nographic',
> +                         '-cpu', 'cortex-a57',
> +                         '-m', '1G',

Is there a reason for defining this exact amount of memory?  My question
has to do with the idea of setting some "sane" hardware defaults at the
base test level (to avoid boiler plate code).

> +                         '-pflash', image_path)
> +        self.vm.launch()
> +        console = self.vm.console_socket.makefile()
> +        serial_logger = logging.getLogger('serial')
> +
> +        # serial console checks
> +        while True:
> +            msg = console.readline()
> +            if not '\x1b' in msg: # ignore ANSI sequences
> +                serial_logger.debug(msg.strip())
> +            if 'Start PXE over IPv4InstallProtocolInterface:' in msg:
> +                break
> +        shutil.rmtree(tmpdirname, ignore_errors=True)
> 

Again, seems like a candidate for the ideas presented on the previous
patches.

Thanks for sending those, lots of interesting opportunities.

- Cleber.