[PATCH v5 6/7] test: vboot: refactor run_bootm

[Simon Glass]

Hi Ludwig,

On 2026-05-28T11:47:57, Ludwig Nussel <ludwig.nussel at siemens.com> wrote:
> test: vboot: refactor run_bootm
>
> - split off a generic part of run_bootm. The helper function is for
>   future use in similar commands
> - moves u-boot restart below section opening for better log structuring
> - only look for expected string in last command. This way a failed
>   assert() doesn't dump unrelated output.
>
> Signed-off-by: Ludwig Nussel <ludwig.nussel at siemens.com>
>
> test/py/tests/test_vboot.py | 54 +++++++++++++++++++++++++++++++++------------
>  1 file changed, 40 insertions(+), 14 deletions(-)

> diff --git a/test/py/tests/test_vboot.py b/test/py/tests/test_vboot.py
> @@ -151,6 +151,37 @@ def test_vboot(ubman, name, sha_algo, padding, sign_options, required,
> +        if not fit:
> +            fit = '%stest.fit' % tmpdir
> +        # run commands that just prepare for the actually relevant one
> +        ubman.run_command('host load hostfs - 100 %s' % fit)
> +        if (len(commands) > 1):
> +            for cmd in commands[:-1]:
> +                ubman.run_command(cmd)

The 'if (len(commands) > 1):' guard is redundant - for a 1-element
list commands[:-1] is empty and the loop is a no-op. Please drop the
guard (and the parens, which aren't Python style). While you're here,
please use f-strings for the fit default and the 'host load' command
to match the new f-string in run_bootm()

> diff --git a/test/py/tests/test_vboot.py b/test/py/tests/test_vboot.py
> @@ -151,6 +151,37 @@ def test_vboot(ubman, name, sha_algo, padding, sign_options, required,
> +    def run_fit_commands(commands, expect_string, fit=None):
> +        """Load fit image and run commands in U-Boot.
> +
> +        Asserts that 'expect_string' is contained in the output of the last command
> +
> +        This always starts a fresh U-Boot instance since the device tree may
> +        contain a new public key.
> +
> +        Args:
> +            commands: list of commands to run
> +            expect_string: A string which is expected in the output
> +            fit: FIT filename to load and verify
> +        """

Missing a Returns: section - the function returns the joined output of
the last command and callers rely on that.

> diff --git a/test/py/tests/test_vboot.py b/test/py/tests/test_vboot.py
> @@ -166,20 +197,15 @@ def test_vboot(ubman, name, sha_algo, padding, sign_options, required,
> +        with ubman.log.section(f'Verified boot {sha_algo} {test_type}: looking for '{expect_string}''):
> +
> +            output = run_fit_commands(['fdt addr 100', 'bootm 100'], expect_string, fit)
> +
> +            if boots:
> +                assert 'sandbox: continuing, as we cannot run' in output
> +            else:
> +                assert 'sandbox: continuing, as we cannot run' not in output

Please drop the blank line directly after 'with' so the block reads as
one unit. Same in run_fit_commands() and run_iminfo() in patch 7.

With those:

Reviewed-by: Simon Glass <sjg at chromium.org>

Regards,
Simon