[v2] tests/functional: Convert most Aspeed machine tests

This is a simple conversion of the tests with some cleanups and
adjustments to match the new test framework. Replace the zephyr image
MD5 hashes with SHA256 hashes while at it.

The SDK tests depend on a ssh class from avocado.utils which is
difficult to replace. To be addressed separately.

Signed-off-by: Cédric Le Goater <clg@redhat.com>
---

 Changes in v2:

 - define class level Asset variables 

 tests/avocado/machine_aspeed.py     | 252 -------------------------
 tests/functional/meson.build        |   2 +
 tests/functional/test_arm_aspeed.py | 277 ++++++++++++++++++++++++++++
 3 files changed, 279 insertions(+), 252 deletions(-)
 create mode 100644 tests/functional/test_arm_aspeed.py

On 16/10/2024 22.38, Cédric Le Goater wrote:
> This is a simple conversion of the tests with some cleanups and
> adjustments to match the new test framework. Replace the zephyr image
> MD5 hashes with SHA256 hashes while at it.
> 
> The SDK tests depend on a ssh class from avocado.utils which is
> difficult to replace. To be addressed separately.
> 
> Signed-off-by: Cédric Le Goater <clg@redhat.com>
> ---
> 
>   Changes in v2:
> 
>   - define class level Asset variables
> 
>   tests/avocado/machine_aspeed.py     | 252 -------------------------
>   tests/functional/meson.build        |   2 +
>   tests/functional/test_arm_aspeed.py | 277 ++++++++++++++++++++++++++++
>   3 files changed, 279 insertions(+), 252 deletions(-)
>   create mode 100644 tests/functional/test_arm_aspeed.py
> 
> diff --git a/tests/avocado/machine_aspeed.py b/tests/avocado/machine_aspeed.py
> index 4e144bde9131..cb163e3a1106 100644
> --- a/tests/avocado/machine_aspeed.py
> +++ b/tests/avocado/machine_aspeed.py
> @@ -19,258 +19,6 @@
>   from avocado_qemu import has_cmd
>   from avocado.utils import archive
>   from avocado import skipUnless
> -from avocado import skipUnless
> -
> -
> -class AST1030Machine(QemuSystemTest):
> -    """Boots the zephyr os and checks that the console is operational"""
> -
> -    timeout = 10
> -
> -    def test_ast1030_zephyros_1_04(self):
> -        """
> -        :avocado: tags=arch:arm
> -        :avocado: tags=machine:ast1030-evb
> -        :avocado: tags=os:zephyr
> -        """
> -        tar_url = ('https://github.com/AspeedTech-BMC'
> -                   '/zephyr/releases/download/v00.01.04/ast1030-evb-demo.zip')
> -        tar_hash = '4c6a8ce3a8ba76ef1a65dae419ae3409343c4b20'
> -        tar_path = self.fetch_asset(tar_url, asset_hash=tar_hash)
> -        archive.extract(tar_path, self.workdir)
> -        kernel_file = self.workdir + "/ast1030-evb-demo/zephyr.elf"
> -        self.vm.set_console()
> -        self.vm.add_args('-kernel', kernel_file,
> -                         '-nographic')
> -        self.vm.launch()
> -        wait_for_console_pattern(self, "Booting Zephyr OS")
> -        exec_command_and_wait_for_pattern(self, "help",
> -                                          "Available commands")
> -
> -    def test_ast1030_zephyros_1_07(self):
> -        """
> -        :avocado: tags=arch:arm
> -        :avocado: tags=machine:ast1030-evb
> -        :avocado: tags=os:zephyr
> -        """
> -        tar_url = ('https://github.com/AspeedTech-BMC'
> -                   '/zephyr/releases/download/v00.01.07/ast1030-evb-demo.zip')
> -        tar_hash = '40ac87eabdcd3b3454ce5aad11fedc72a33ecda2'
> -        tar_path = self.fetch_asset(tar_url, asset_hash=tar_hash)
> -        archive.extract(tar_path, self.workdir)
> -        kernel_file = self.workdir + "/ast1030-evb-demo/zephyr.bin"
> -        self.vm.set_console()
> -        self.vm.add_args('-kernel', kernel_file,
> -                         '-nographic')
> -        self.vm.launch()
> -        wait_for_console_pattern(self, "Booting Zephyr OS")
> -        for shell_cmd in [
> -                'kernel stacks',
> -                'otp info conf',
> -                'otp info scu',
> -                'hwinfo devid',
> -                'crypto aes256_cbc_vault',
> -                'random get',
> -                'jtag JTAG1 sw_xfer high TMS',
> -                'adc ADC0 resolution 12',
> -                'adc ADC0 read 42',
> -                'adc ADC1 read 69',
> -                'i2c scan I2C_0',
> -                'i3c attach I3C_0',
> -                'hash test',
> -                'kernel uptime',
> -                'kernel reboot warm',
> -                'kernel uptime',
> -                'kernel reboot cold',
> -                'kernel uptime',
> -        ]: exec_command_and_wait_for_pattern(self, shell_cmd, "uart:~$")
> -
> -class AST2x00Machine(QemuSystemTest):
> -
> -    timeout = 180
> -
> -    def wait_for_console_pattern(self, success_message, vm=None):
> -        wait_for_console_pattern(self, success_message,
> -                                 failure_message='Kernel panic - not syncing',
> -                                 vm=vm)

You're losing the "failure_message='Kernel panic - not syncing'" check in 
your new code. Maybe not worth the effort to copy it over, but still, should 
be mentioned at least in the commit description, I think.

Alternatively, you could also use the LinuxKernelTest base class instead 
which has this wait_for_console_pattern magic included already.

> -    def do_test_arm_aspeed(self, image):
> -        self.vm.set_console()
> -        self.vm.add_args('-drive', 'file=' + image + ',if=mtd,format=raw',
> -                         '-net', 'nic')
> -        self.vm.launch()
> -
> -        self.wait_for_console_pattern("U-Boot 2016.07")
> -        self.wait_for_console_pattern("## Loading kernel from FIT Image at 20080000")
> -        self.wait_for_console_pattern("Starting kernel ...")
> -        self.wait_for_console_pattern("Booting Linux on physical CPU 0x0")
> -        wait_for_console_pattern(self,
> -                "aspeed-smc 1e620000.spi: read control register: 203b0641")
> -        self.wait_for_console_pattern("ftgmac100 1e660000.ethernet eth0: irq ")
> -        self.wait_for_console_pattern("systemd[1]: Set hostname to")
...
> -    @skipUnless(*has_cmd('swtpm'))
> -    def test_arm_ast2600_evb_buildroot_tpm(self):
> -        """
> -        :avocado: tags=arch:arm
> -        :avocado: tags=machine:ast2600-evb
> -        """
> -
> -        image_url = ('https://github.com/legoater/qemu-aspeed-boot/raw/master/'
> -                     'images/ast2600-evb/buildroot-2023.02-tpm/flash.img')
> -        image_hash = ('a46009ae8a5403a0826d607215e731a8c68d27c14c41e55331706b8f9c7bd997')
> -        image_path = self.fetch_asset(image_url, asset_hash=image_hash,
> -                                      algorithm='sha256')
> -
> -        # force creation of VM object, which also defines self._sd
> -        vm = self.vm
> -
> -        socket = os.path.join(self._sd.name, 'swtpm-socket')
> -
> -        subprocess.run(['swtpm', 'socket', '-d', '--tpm2',
> -                        '--tpmstate', f'dir={self.vm.temp_dir}',
> -                        '--ctrl', f'type=unixio,path={socket}'])
> -
> -        self.vm.add_args('-chardev', f'socket,id=chrtpm,path={socket}')
> -        self.vm.add_args('-tpmdev', 'emulator,id=tpm0,chardev=chrtpm')
> -        self.vm.add_args('-device',
> -                         'tpm-tis-i2c,tpmdev=tpm0,bus=aspeed.i2c.bus.12,address=0x2e')
> -        self.do_test_arm_aspeed_buildroot_start(image_path, '0xf00', 'Aspeed AST2600 EVB')
> -
> -        exec_command_and_wait_for_pattern(self,
> -            'echo tpm_tis_i2c 0x2e > /sys/bus/i2c/devices/i2c-12/new_device',
> -            'tpm_tis_i2c 12-002e: 2.0 TPM (device-id 0x1, rev-id 1)');
> -        exec_command_and_wait_for_pattern(self,
> -            'cat /sys/class/tpm/tpm0/pcr-sha256/0',
> -            'B804724EA13F52A9072BA87FE8FDCC497DFC9DF9AA15B9088694639C431688E0');
> -
> -        self.do_test_arm_aspeed_buildroot_poweroff()
>   
>   class AST2x00MachineSDK(QemuSystemTest, LinuxSSHMixIn):

You also copied AST2x00MachineMMC but you didn't remove it from the avocado 
test yet, so there is a hunk missing here, I think?

> diff --git a/tests/functional/test_arm_aspeed.py b/tests/functional/test_arm_aspeed.py
> new file mode 100644
> index 000000000000..37e0c0734b25
> --- /dev/null
> +++ b/tests/functional/test_arm_aspeed.py
> @@ -0,0 +1,277 @@
> +#!/usr/bin/env python3
> +#
> +# Functional test that boots the ASPEED SoCs with firmware
> +#
> +# Copyright (C) 2022 ASPEED Technology Inc
> +#
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +
> +import os
> +import time
> +import subprocess
> +import tempfile
> +
> +from qemu_test import QemuSystemTest, Asset
> +from qemu_test import wait_for_console_pattern
> +from qemu_test import exec_command_and_wait_for_pattern
> +from qemu_test import interrupt_interactive_console_until_pattern
> +from qemu_test import exec_command
> +from qemu_test import has_cmd
> +from qemu_test.utils import archive_extract
> +from zipfile import ZipFile
> +from unittest import skipUnless
> +
> +class AST1030Machine(QemuSystemTest):
> +
> +    ASSET_ZEPHYR_1_04 = Asset(
> +        ('https://github.com/AspeedTech-BMC'
> +         '/zephyr/releases/download/v00.01.04/ast1030-evb-demo.zip'),
> +        '4ac6210adcbc61294927918707c6762483fd844dde5e07f3ba834ad1f91434d3')
> +
> +    def test_ast1030_zephyros_1_04(self):
> +        kernel_name = "ast1030-evb-demo/zephyr.elf"
> +        zip_file = self.ASSET_ZEPHYR_1_04.fetch()
> +        with ZipFile(zip_file, 'r') as zf:
> +                     zf.extract(kernel_name, path=self.workdir)
> +        kernel_file = os.path.join(self.workdir, kernel_name)
> +
> +        self.set_machine('ast1030-evb')

It's slightly better to do the set_machine() at the very top of the 
function. set_machine() can cancel the test in case the machine is not 
available in the QEMU binary, so this should best be done before fetching 
and extracting any assets.

> +        self.vm.set_console()
> +        self.vm.add_args('-kernel', kernel_file, '-nographic')
> +        self.vm.launch()
> +        wait_for_console_pattern(self, "Booting Zephyr OS")
> +        exec_command_and_wait_for_pattern(self, "help",
> +                                          "Available commands")
> +
> +    ASSET_ZEPHYR_1_07 = Asset(
> +        ('https://github.com/AspeedTech-BMC'
> +         '/zephyr/releases/download/v00.01.07/ast1030-evb-demo.zip'),
> +        'ad52e27959746988afaed8429bf4e12ab988c05c4d07c9d90e13ec6f7be4574c')
> +
> +    def test_ast1030_zephyros_1_07(self):
> +        kernel_name = "ast1030-evb-demo/zephyr.bin"
> +        zip_file = self.ASSET_ZEPHYR_1_07.fetch()
> +        with ZipFile(zip_file, 'r') as zf:
> +                     zf.extract(kernel_name, path=self.workdir)
> +        kernel_file = os.path.join(self.workdir, kernel_name)
> +
> +        self.set_machine('ast1030-evb')

dito

> +        self.vm.set_console()
> +        self.vm.add_args('-kernel', kernel_file, '-nographic')
> +        self.vm.launch()
> +        wait_for_console_pattern(self, "Booting Zephyr OS")
> +        for shell_cmd in [
> +                'kernel stacks',
> +                'otp info conf',
> +                'otp info scu',
> +                'hwinfo devid',
> +                'crypto aes256_cbc_vault',
> +                'random get',
> +                'jtag JTAG1 sw_xfer high TMS',
> +                'adc ADC0 resolution 12',
> +                'adc ADC0 read 42',
> +                'adc ADC1 read 69',
> +                'i2c scan I2C_0',
> +                'i3c attach I3C_0',
> +                'hash test',
> +                'kernel uptime',
> +                'kernel reboot warm',
> +                'kernel uptime',
> +                'kernel reboot cold',
> +                'kernel uptime',
> +        ]: exec_command_and_wait_for_pattern(self, shell_cmd, "uart:~$")
> +
> +class AST2x00Machine(QemuSystemTest):
> +
> +    def do_test_arm_aspeed(self, machine, image):
> +        self.set_machine(machine)
> +        self.vm.set_console()
> +        self.vm.add_args('-drive', 'file=' + image + ',if=mtd,format=raw',
> +                         '-net', 'nic')
> +        self.vm.launch()
> +
> +        wait_for_console_pattern(self, "U-Boot 2016.07")
> +        wait_for_console_pattern(self, "## Loading kernel from FIT Image at 20080000")
> +        wait_for_console_pattern(self, "Starting kernel ...")
> +        wait_for_console_pattern(self, "Booting Linux on physical CPU 0x0")
> +        wait_for_console_pattern(self,
> +                "aspeed-smc 1e620000.spi: read control register: 203b0641")
> +        wait_for_console_pattern(self, "ftgmac100 1e660000.ethernet eth0: irq ")
> +        wait_for_console_pattern(self, "systemd[1]: Set hostname to")
> +
> +    ASSET_PALMETTO_FLASH = Asset(
> +        ('https://github.com/openbmc/openbmc/releases/download/2.9.0/'
> +         'obmc-phosphor-image-palmetto.static.mtd'),
> +        '3e13bbbc28e424865dc42f35ad672b10f2e82cdb11846bb28fa625b48beafd0d');
> +
> +    def test_arm_ast2400_palmetto_openbmc_v2_9_0(self):
> +        image_path = self.ASSET_PALMETTO_FLASH.fetch()
> +
> +        self.do_test_arm_aspeed('palmetto-bmc', image_path)
> +
> +    ASSET_ROMULUS_FLASH = Asset(
> +        ('https://github.com/openbmc/openbmc/releases/download/2.9.0/'
> +         'obmc-phosphor-image-romulus.static.mtd'),
> +        '820341076803f1955bc31e647a512c79f9add4f5233d0697678bab4604c7bb25')
> +
> +    def test_arm_ast2500_romulus_openbmc_v2_9_0(self):
> +        image_path = self.ASSET_ROMULUS_FLASH.fetch()
> +
> +        self.do_test_arm_aspeed('romulus-bmc', image_path)
> +
> +    def do_test_arm_aspeed_buildroot_start(self, image, cpu_id, pattern='Aspeed EVB'):
> +        self.require_netdev('user')
> +        self.vm.set_console()
> +        self.vm.add_args('-drive', 'file=' + image + ',if=mtd,format=raw',
> +                         '-net', 'nic', '-net', 'user')
> +        self.vm.launch()
> +
> +        wait_for_console_pattern(self, 'U-Boot 2019.04')
> +        wait_for_console_pattern(self, '## Loading kernel from FIT Image')
> +        wait_for_console_pattern(self, 'Starting kernel ...')
> +        wait_for_console_pattern(self, 'Booting Linux on physical CPU ' + cpu_id)
> +        wait_for_console_pattern(self, 'lease of 10.0.2.15')
> +        # the line before login:
> +        wait_for_console_pattern(self, pattern)
> +        time.sleep(0.1)
> +        exec_command(self, 'root')
> +        time.sleep(0.1)
> +        exec_command(self, "passw0rd")
> +
> +    def do_test_arm_aspeed_buildroot_poweroff(self):
> +        exec_command_and_wait_for_pattern(self, 'poweroff',
> +                                          'reboot: System halted');
> +
> +    ASSET_BR2_202311_AST2500_FLASH = Asset(
> +        ('https://github.com/legoater/qemu-aspeed-boot/raw/master/'
> +         'images/ast2500-evb/buildroot-2023.11/flash.img'),
> +        'c23db6160cf77d0258397eb2051162c8473a56c441417c52a91ba217186e715f')
> +
> +    def test_arm_ast2500_evb_buildroot(self):
> +        image_path = self.ASSET_BR2_202311_AST2500_FLASH.fetch()
> +
> +        self.set_machine('ast2500-evb')

dito, switch the order of the above two lines, please.

> +        self.vm.add_args('-device',
> +                         'tmp105,bus=aspeed.i2c.bus.3,address=0x4d,id=tmp-test');
> +        self.do_test_arm_aspeed_buildroot_start(image_path, '0x0',
> +                                                'Aspeed AST2500 EVB')
> +
> +        exec_command_and_wait_for_pattern(self,
> +             'echo lm75 0x4d > /sys/class/i2c-dev/i2c-3/device/new_device',
> +             'i2c i2c-3: new_device: Instantiated device lm75 at 0x4d');
> +        exec_command_and_wait_for_pattern(self,
> +                             'cat /sys/class/hwmon/hwmon1/temp1_input', '0')
> +        self.vm.cmd('qom-set', path='/machine/peripheral/tmp-test',
> +                    property='temperature', value=18000);
> +        exec_command_and_wait_for_pattern(self,
> +                             'cat /sys/class/hwmon/hwmon1/temp1_input', '18000')
> +
> +        self.do_test_arm_aspeed_buildroot_poweroff()
> +
> +    ASSET_BR2_202311_AST2600_FLASH = Asset(
> +        ('https://github.com/legoater/qemu-aspeed-boot/raw/master/'
> +         'images/ast2600-evb/buildroot-2023.11/flash.img'),
> +        'b62808daef48b438d0728ee07662290490ecfa65987bb91294cafb1bb7ad1a68')
> +
> +    def test_arm_ast2600_evb_buildroot(self):
> +        image_path = self.ASSET_BR2_202311_AST2600_FLASH.fetch()
> +
> +        self.set_machine('ast2600-evb')

dito

> +        self.vm.add_args('-device',
> +                         'tmp105,bus=aspeed.i2c.bus.3,address=0x4d,id=tmp-test');
> +        self.vm.add_args('-device',
> +                         'ds1338,bus=aspeed.i2c.bus.3,address=0x32');
> +        self.vm.add_args('-device',
> +                         'i2c-echo,bus=aspeed.i2c.bus.3,address=0x42');
> +        self.do_test_arm_aspeed_buildroot_start(image_path, '0xf00', 'Aspeed AST2600 EVB')
> +
> +        exec_command_and_wait_for_pattern(self,
> +             'echo lm75 0x4d > /sys/class/i2c-dev/i2c-3/device/new_device',
> +             'i2c i2c-3: new_device: Instantiated device lm75 at 0x4d');
> +        exec_command_and_wait_for_pattern(self,
> +                             'cat /sys/class/hwmon/hwmon1/temp1_input', '0')
> +        self.vm.cmd('qom-set', path='/machine/peripheral/tmp-test',
> +                    property='temperature', value=18000);
> +        exec_command_and_wait_for_pattern(self,
> +                             'cat /sys/class/hwmon/hwmon1/temp1_input', '18000')
> +
> +        exec_command_and_wait_for_pattern(self,
> +             'echo ds1307 0x32 > /sys/class/i2c-dev/i2c-3/device/new_device',
> +             'i2c i2c-3: new_device: Instantiated device ds1307 at 0x32');
> +        year = time.strftime("%Y")
> +        exec_command_and_wait_for_pattern(self, 'hwclock -f /dev/rtc1', year);
> +
> +        exec_command_and_wait_for_pattern(self,
> +             'echo slave-24c02 0x1064 > /sys/bus/i2c/devices/i2c-3/new_device',
> +             'i2c i2c-3: new_device: Instantiated device slave-24c02 at 0x64');
> +        exec_command(self, 'i2cset -y 3 0x42 0x64 0x00 0xaa i');
> +        time.sleep(0.1)
> +        exec_command_and_wait_for_pattern(self,
> +             'hexdump /sys/bus/i2c/devices/3-1064/slave-eeprom',
> +             '0000000 ffaa ffff ffff ffff ffff ffff ffff ffff');
> +        self.do_test_arm_aspeed_buildroot_poweroff()
> +
> +    ASSET_BR2_202302_AST2600_TPM_FLASH = Asset(
> +        ('https://github.com/legoater/qemu-aspeed-boot/raw/master/'
> +         'images/ast2600-evb/buildroot-2023.02-tpm/flash.img'),
> +        'a46009ae8a5403a0826d607215e731a8c68d27c14c41e55331706b8f9c7bd997')
> +
> +    @skipUnless(*has_cmd('swtpm'))
> +    def test_arm_ast2600_evb_buildroot_tpm(self):
> +        image_path = self.ASSET_BR2_202302_AST2600_TPM_FLASH.fetch()
> +
> +        socket_dir = tempfile.TemporaryDirectory(prefix="qemu_")
> +        socket = os.path.join(socket_dir.name, 'swtpm-socket')
> +
> +        self.set_machine('ast2600-evb')

Please move the set_machine() to the beginning of the function here, too.

> +        subprocess.run(['swtpm', 'socket', '-d', '--tpm2',
> +                        '--tpmstate', f'dir={self.vm.temp_dir}',
> +                        '--ctrl', f'type=unixio,path={socket}'])
> +
> +        self.vm.add_args('-chardev', f'socket,id=chrtpm,path={socket}')
> +        self.vm.add_args('-tpmdev', 'emulator,id=tpm0,chardev=chrtpm')
> +        self.vm.add_args('-device',
> +                         'tpm-tis-i2c,tpmdev=tpm0,bus=aspeed.i2c.bus.12,address=0x2e')
> +        self.do_test_arm_aspeed_buildroot_start(image_path, '0xf00', 'Aspeed AST2600 EVB')
> +
> +        exec_command_and_wait_for_pattern(self,
> +            'echo tpm_tis_i2c 0x2e > /sys/bus/i2c/devices/i2c-12/new_device',
> +            'tpm_tis_i2c 12-002e: 2.0 TPM (device-id 0x1, rev-id 1)');
> +        exec_command_and_wait_for_pattern(self,
> +            'cat /sys/class/tpm/tpm0/pcr-sha256/0',
> +            'B804724EA13F52A9072BA87FE8FDCC497DFC9DF9AA15B9088694639C431688E0');
> +
> +        self.do_test_arm_aspeed_buildroot_poweroff()
> +
> +class AST2x00MachineMMC(QemuSystemTest):
> +
> +    ASSET_RAINIER_EMMC = Asset(
> +        ('https://fileserver.linaro.org/s/B6pJTwWEkzSDi36/download/'
> +         'mmc-p10bmc-20240617.qcow2'),
> +        'd523fb478d2b84d5adc5658d08502bc64b1486955683814f89c6137518acd90b')
> +
> +    def test_arm_aspeed_emmc_boot(self):
> +        image_path = self.ASSET_RAINIER_EMMC.fetch()
> +
> +        self.require_netdev('user')
> +
> +        self.set_machine('rainier-bmc')

dito. require_netdev() should also be done before fetching the asset, since 
it can cancel the test, too.

> +        self.vm.set_console()
> +        self.vm.add_args('-drive',
> +                         'file=' + image_path + ',if=sd,id=sd2,index=2',
> +                         '-net', 'nic', '-net', 'user')
> +        self.vm.launch()
> +
> +        wait_for_console_pattern(self, 'U-Boot SPL 2019.04')
> +        wait_for_console_pattern(self, 'Trying to boot from MMC1')
> +        wait_for_console_pattern(self, 'U-Boot 2019.04')
> +        wait_for_console_pattern(self, 'eMMC 2nd Boot')
> +        wait_for_console_pattern(self, '## Loading kernel from FIT Image')
> +        wait_for_console_pattern(self, 'Starting kernel ...')
> +        wait_for_console_pattern(self, 'Booting Linux on physical CPU 0xf00')
> +        wait_for_console_pattern(self, 'mmcblk0: p1 p2 p3 p4 p5 p6 p7')
> +        wait_for_console_pattern(self, 'IBM eBMC (OpenBMC for IBM Enterprise')

  Thomas

PS:
Note to myself: Add some comments to the documentation about set_machine() 
and require_netdev() to the documentation to state that they should come 
first in test functions.

On 10/17/24 08:06, Thomas Huth wrote:
> On 16/10/2024 22.38, Cédric Le Goater wrote:
>> This is a simple conversion of the tests with some cleanups and
>> adjustments to match the new test framework. Replace the zephyr image
>> MD5 hashes with SHA256 hashes while at it.
>>
>> The SDK tests depend on a ssh class from avocado.utils which is
>> difficult to replace. To be addressed separately.
>>
>> Signed-off-by: Cédric Le Goater <clg@redhat.com>
>> ---
>>
>>   Changes in v2:
>>
>>   - define class level Asset variables
>>
>>   tests/avocado/machine_aspeed.py     | 252 -------------------------
>>   tests/functional/meson.build        |   2 +
>>   tests/functional/test_arm_aspeed.py | 277 ++++++++++++++++++++++++++++
>>   3 files changed, 279 insertions(+), 252 deletions(-)
>>   create mode 100644 tests/functional/test_arm_aspeed.py
>>
>> diff --git a/tests/avocado/machine_aspeed.py b/tests/avocado/machine_aspeed.py
>> index 4e144bde9131..cb163e3a1106 100644
>> --- a/tests/avocado/machine_aspeed.py
>> +++ b/tests/avocado/machine_aspeed.py
>> @@ -19,258 +19,6 @@
>>   from avocado_qemu import has_cmd
>>   from avocado.utils import archive
>>   from avocado import skipUnless
>> -from avocado import skipUnless
>> -
>> -
>> -class AST1030Machine(QemuSystemTest):
>> -    """Boots the zephyr os and checks that the console is operational"""
>> -
>> -    timeout = 10
>> -
>> -    def test_ast1030_zephyros_1_04(self):
>> -        """
>> -        :avocado: tags=arch:arm
>> -        :avocado: tags=machine:ast1030-evb
>> -        :avocado: tags=os:zephyr
>> -        """
>> -        tar_url = ('https://github.com/AspeedTech-BMC'
>> -                   '/zephyr/releases/download/v00.01.04/ast1030-evb-demo.zip')
>> -        tar_hash = '4c6a8ce3a8ba76ef1a65dae419ae3409343c4b20'
>> -        tar_path = self.fetch_asset(tar_url, asset_hash=tar_hash)
>> -        archive.extract(tar_path, self.workdir)
>> -        kernel_file = self.workdir + "/ast1030-evb-demo/zephyr.elf"
>> -        self.vm.set_console()
>> -        self.vm.add_args('-kernel', kernel_file,
>> -                         '-nographic')
>> -        self.vm.launch()
>> -        wait_for_console_pattern(self, "Booting Zephyr OS")
>> -        exec_command_and_wait_for_pattern(self, "help",
>> -                                          "Available commands")
>> -
>> -    def test_ast1030_zephyros_1_07(self):
>> -        """
>> -        :avocado: tags=arch:arm
>> -        :avocado: tags=machine:ast1030-evb
>> -        :avocado: tags=os:zephyr
>> -        """
>> -        tar_url = ('https://github.com/AspeedTech-BMC'
>> -                   '/zephyr/releases/download/v00.01.07/ast1030-evb-demo.zip')
>> -        tar_hash = '40ac87eabdcd3b3454ce5aad11fedc72a33ecda2'
>> -        tar_path = self.fetch_asset(tar_url, asset_hash=tar_hash)
>> -        archive.extract(tar_path, self.workdir)
>> -        kernel_file = self.workdir + "/ast1030-evb-demo/zephyr.bin"
>> -        self.vm.set_console()
>> -        self.vm.add_args('-kernel', kernel_file,
>> -                         '-nographic')
>> -        self.vm.launch()
>> -        wait_for_console_pattern(self, "Booting Zephyr OS")
>> -        for shell_cmd in [
>> -                'kernel stacks',
>> -                'otp info conf',
>> -                'otp info scu',
>> -                'hwinfo devid',
>> -                'crypto aes256_cbc_vault',
>> -                'random get',
>> -                'jtag JTAG1 sw_xfer high TMS',
>> -                'adc ADC0 resolution 12',
>> -                'adc ADC0 read 42',
>> -                'adc ADC1 read 69',
>> -                'i2c scan I2C_0',
>> -                'i3c attach I3C_0',
>> -                'hash test',
>> -                'kernel uptime',
>> -                'kernel reboot warm',
>> -                'kernel uptime',
>> -                'kernel reboot cold',
>> -                'kernel uptime',
>> -        ]: exec_command_and_wait_for_pattern(self, shell_cmd, "uart:~$")
>> -
>> -class AST2x00Machine(QemuSystemTest):
>> -
>> -    timeout = 180
>> -
>> -    def wait_for_console_pattern(self, success_message, vm=None):
>> -        wait_for_console_pattern(self, success_message,
>> -                                 failure_message='Kernel panic - not syncing',
>> -                                 vm=vm)
> 
> You're losing the "failure_message='Kernel panic - not syncing'" check in your new code. Maybe not worth the effort to copy it over, but still, should be mentioned at least in the commit description, I think.
> 
> Alternatively, you could also use the LinuxKernelTest base class instead which has this wait_for_console_pattern magic included already.

Ah ! I didn't know. I will use LinuxKernelTest instead.

> 
>> -    def do_test_arm_aspeed(self, image):
>> -        self.vm.set_console()
>> -        self.vm.add_args('-drive', 'file=' + image + ',if=mtd,format=raw',
>> -                         '-net', 'nic')
>> -        self.vm.launch()
>> -
>> -        self.wait_for_console_pattern("U-Boot 2016.07")
>> -        self.wait_for_console_pattern("## Loading kernel from FIT Image at 20080000")
>> -        self.wait_for_console_pattern("Starting kernel ...")
>> -        self.wait_for_console_pattern("Booting Linux on physical CPU 0x0")
>> -        wait_for_console_pattern(self,
>> -                "aspeed-smc 1e620000.spi: read control register: 203b0641")
>> -        self.wait_for_console_pattern("ftgmac100 1e660000.ethernet eth0: irq ")
>> -        self.wait_for_console_pattern("systemd[1]: Set hostname to")
> ...
>> -    @skipUnless(*has_cmd('swtpm'))
>> -    def test_arm_ast2600_evb_buildroot_tpm(self):
>> -        """
>> -        :avocado: tags=arch:arm
>> -        :avocado: tags=machine:ast2600-evb
>> -        """
>> -
>> -        image_url = ('https://github.com/legoater/qemu-aspeed-boot/raw/master/'
>> -                     'images/ast2600-evb/buildroot-2023.02-tpm/flash.img')
>> -        image_hash = ('a46009ae8a5403a0826d607215e731a8c68d27c14c41e55331706b8f9c7bd997')
>> -        image_path = self.fetch_asset(image_url, asset_hash=image_hash,
>> -                                      algorithm='sha256')
>> -
>> -        # force creation of VM object, which also defines self._sd
>> -        vm = self.vm
>> -
>> -        socket = os.path.join(self._sd.name, 'swtpm-socket')
>> -
>> -        subprocess.run(['swtpm', 'socket', '-d', '--tpm2',
>> -                        '--tpmstate', f'dir={self.vm.temp_dir}',
>> -                        '--ctrl', f'type=unixio,path={socket}'])
>> -
>> -        self.vm.add_args('-chardev', f'socket,id=chrtpm,path={socket}')
>> -        self.vm.add_args('-tpmdev', 'emulator,id=tpm0,chardev=chrtpm')
>> -        self.vm.add_args('-device',
>> -                         'tpm-tis-i2c,tpmdev=tpm0,bus=aspeed.i2c.bus.12,address=0x2e')
>> -        self.do_test_arm_aspeed_buildroot_start(image_path, '0xf00', 'Aspeed AST2600 EVB')
>> -
>> -        exec_command_and_wait_for_pattern(self,
>> -            'echo tpm_tis_i2c 0x2e > /sys/bus/i2c/devices/i2c-12/new_device',
>> -            'tpm_tis_i2c 12-002e: 2.0 TPM (device-id 0x1, rev-id 1)');
>> -        exec_command_and_wait_for_pattern(self,
>> -            'cat /sys/class/tpm/tpm0/pcr-sha256/0',
>> -            'B804724EA13F52A9072BA87FE8FDCC497DFC9DF9AA15B9088694639C431688E0');
>> -
>> -        self.do_test_arm_aspeed_buildroot_poweroff()
>>   class AST2x00MachineSDK(QemuSystemTest, LinuxSSHMixIn):
> 
> You also copied AST2x00MachineMMC but you didn't remove it from the avocado test yet, so there is a hunk missing here, I think?

oh. yes, I should remove it from the avocado test.

I was initially hesitating on having multiple test_arm_aspeed.py files:
one per machine or one per fw image type. This to set the timeout more
precisely in the meson.build file, since the SDK FW takes longer to
boot, and also to handle more complex test scenarios, like the SDK.

What would be your recommandation ?


Btw, I have been trying to rewrite the LinuxSSHMixIn class with a
minimal ssh Session class. It is not trivial :/
  
>> diff --git a/tests/functional/test_arm_aspeed.py b/tests/functional/test_arm_aspeed.py
>> new file mode 100644
>> index 000000000000..37e0c0734b25
>> --- /dev/null
>> +++ b/tests/functional/test_arm_aspeed.py
>> @@ -0,0 +1,277 @@
>> +#!/usr/bin/env python3
>> +#
>> +# Functional test that boots the ASPEED SoCs with firmware
>> +#
>> +# Copyright (C) 2022 ASPEED Technology Inc
>> +#
>> +# SPDX-License-Identifier: GPL-2.0-or-later
>> +
>> +import os
>> +import time
>> +import subprocess
>> +import tempfile
>> +
>> +from qemu_test import QemuSystemTest, Asset
>> +from qemu_test import wait_for_console_pattern
>> +from qemu_test import exec_command_and_wait_for_pattern
>> +from qemu_test import interrupt_interactive_console_until_pattern
>> +from qemu_test import exec_command
>> +from qemu_test import has_cmd
>> +from qemu_test.utils import archive_extract
>> +from zipfile import ZipFile
>> +from unittest import skipUnless
>> +
>> +class AST1030Machine(QemuSystemTest):
>> +
>> +    ASSET_ZEPHYR_1_04 = Asset(
>> +        ('https://github.com/AspeedTech-BMC'
>> +         '/zephyr/releases/download/v00.01.04/ast1030-evb-demo.zip'),
>> +        '4ac6210adcbc61294927918707c6762483fd844dde5e07f3ba834ad1f91434d3')
>> +
>> +    def test_ast1030_zephyros_1_04(self):
>> +        kernel_name = "ast1030-evb-demo/zephyr.elf"
>> +        zip_file = self.ASSET_ZEPHYR_1_04.fetch()
>> +        with ZipFile(zip_file, 'r') as zf:
>> +                     zf.extract(kernel_name, path=self.workdir)
>> +        kernel_file = os.path.join(self.workdir, kernel_name)
>> +
>> +        self.set_machine('ast1030-evb')
> 
> It's slightly better to do the set_machine() at the very top of the function. set_machine() can cancel the test in case the machine is not available in the QEMU binary, so this should best be done before fetching and extracting any assets.

Should set_machine() be the first call of a test function ?

[ ... ]

> PS:
> Note to myself: Add some comments to the documentation about set_machine() and require_netdev() to the documentation to state that they should come first in test functions.

Yes. That would be a good idea.

Thanks for the review,

C.

On 17/10/2024 09.09, Cédric Le Goater wrote:
> On 10/17/24 08:06, Thomas Huth wrote:
>> On 16/10/2024 22.38, Cédric Le Goater wrote:
>>> This is a simple conversion of the tests with some cleanups and
>>> adjustments to match the new test framework. Replace the zephyr image
>>> MD5 hashes with SHA256 hashes while at it.
>>>
>>> The SDK tests depend on a ssh class from avocado.utils which is
>>> difficult to replace. To be addressed separately.
>>>
>>> Signed-off-by: Cédric Le Goater <clg@redhat.com>
...
>>> -    @skipUnless(*has_cmd('swtpm'))
>>> -    def test_arm_ast2600_evb_buildroot_tpm(self):
>>> -        """
>>> -        :avocado: tags=arch:arm
>>> -        :avocado: tags=machine:ast2600-evb
>>> -        """
>>> -
>>> -        image_url = ('https://github.com/legoater/qemu-aspeed-boot/raw/ 
>>> master/'
>>> -                     'images/ast2600-evb/buildroot-2023.02-tpm/flash.img')
>>> -        image_hash = 
>>> ('a46009ae8a5403a0826d607215e731a8c68d27c14c41e55331706b8f9c7bd997')
>>> -        image_path = self.fetch_asset(image_url, asset_hash=image_hash,
>>> -                                      algorithm='sha256')
>>> -
>>> -        # force creation of VM object, which also defines self._sd
>>> -        vm = self.vm
>>> -
>>> -        socket = os.path.join(self._sd.name, 'swtpm-socket')
>>> -
>>> -        subprocess.run(['swtpm', 'socket', '-d', '--tpm2',
>>> -                        '--tpmstate', f'dir={self.vm.temp_dir}',
>>> -                        '--ctrl', f'type=unixio,path={socket}'])
>>> -
>>> -        self.vm.add_args('-chardev', f'socket,id=chrtpm,path={socket}')
>>> -        self.vm.add_args('-tpmdev', 'emulator,id=tpm0,chardev=chrtpm')
>>> -        self.vm.add_args('-device',
>>> -                         'tpm-tis- 
>>> i2c,tpmdev=tpm0,bus=aspeed.i2c.bus.12,address=0x2e')
>>> -        self.do_test_arm_aspeed_buildroot_start(image_path, '0xf00', 
>>> 'Aspeed AST2600 EVB')
>>> -
>>> -        exec_command_and_wait_for_pattern(self,
>>> -            'echo tpm_tis_i2c 0x2e > /sys/bus/i2c/devices/i2c-12/ 
>>> new_device',
>>> -            'tpm_tis_i2c 12-002e: 2.0 TPM (device-id 0x1, rev-id 1)');
>>> -        exec_command_and_wait_for_pattern(self,
>>> -            'cat /sys/class/tpm/tpm0/pcr-sha256/0',
>>> -            
>>> 'B804724EA13F52A9072BA87FE8FDCC497DFC9DF9AA15B9088694639C431688E0');
>>> -
>>> -        self.do_test_arm_aspeed_buildroot_poweroff()
>>>   class AST2x00MachineSDK(QemuSystemTest, LinuxSSHMixIn):
>>
>> You also copied AST2x00MachineMMC but you didn't remove it from the 
>> avocado test yet, so there is a hunk missing here, I think?
> 
> oh. yes, I should remove it from the avocado test.
> 
> I was initially hesitating on having multiple test_arm_aspeed.py files:
> one per machine or one per fw image type. This to set the timeout more
> precisely in the meson.build file, since the SDK FW takes longer to
> boot, and also to handle more complex test scenarios, like the SDK.
> 
> What would be your recommandation ?

If the tests do not share anything at all (neither common python code, nor 
the same firmware binary asset), I think it's maybe better to put them into 
separate files, indeed. That way we can also run the tests in parallel when 
you add "-j$(nproc)" to your "make check-functional" line.

> Btw, I have been trying to rewrite the LinuxSSHMixIn class with a
> minimal ssh Session class. It is not trivial :/

:-(

Did you peek at tests/vm/basevm.py ? I was hoping we could lent some code 
there...

...
>>> +    def test_ast1030_zephyros_1_04(self):
>>> +        kernel_name = "ast1030-evb-demo/zephyr.elf"
>>> +        zip_file = self.ASSET_ZEPHYR_1_04.fetch()
>>> +        with ZipFile(zip_file, 'r') as zf:
>>> +                     zf.extract(kernel_name, path=self.workdir)
>>> +        kernel_file = os.path.join(self.workdir, kernel_name)
>>> +
>>> +        self.set_machine('ast1030-evb')
>>
>> It's slightly better to do the set_machine() at the very top of the 
>> function. set_machine() can cancel the test in case the machine is not 
>> available in the QEMU binary, so this should best be done before fetching 
>> and extracting any assets.
> 
> Should set_machine() be the first call of a test function ?

Yes, either set_machine() or require_netdev() should be at the very top, 
since they could cancel the test.

  Thomas