Re: [PATCH 2/2] test: cmd: add a test for bootd

Hi Mehmet,

On 2026-07-30T15:51:17, Mehmet Fide <[email protected]> wrote:
> test: cmd: add a test for bootd
>
> There is no test for the bootd command. Add one to the cmd suite that
> covers the documented behaviour: bootd and its "boot" alias run the
> command held in the bootcmd environment variable, and the return value
> of bootd is the one of that command.
>
> Signed-off-by: Mehmet Fide <[email protected]>
>
> test/cmd/Makefile |  3 +++
>  test/cmd/bootd.c  | 35 +++++++++++++++++++++++++++++++++++
>  2 files changed, 38 insertions(+)

> diff --git a/test/cmd/Makefile b/test/cmd/Makefile
> @@ -17,6 +17,9 @@ ifdef CONFIG_CONSOLE_RECORD
>  obj-$(CONFIG_CMD_ACPI) += acpi.o
>  endif
>  obj-$(CONFIG_CMD_BDI) += bdinfo.o
> +ifdef CONFIG_CONSOLE_RECORD
> +obj-$(CONFIG_CMD_BOOTD) += bootd.o
> +endif

The CONFIG_CONSOLE_RECORD ifdef is not needed. UTF_CONSOLE means the
runner skips the test when console recording is unavailable, and
bdinfo.o just above has no such guard. Please add
'obj-$(CONFIG_CMD_BOOTD) += bootd.o' next to the bdinfo line.

> diff --git a/test/cmd/bootd.c b/test/cmd/bootd.c
> @@ -0,0 +1,35 @@
> +     /* the return value is the one of the command in bootcmd */
> +     ut_assertok(env_set("bootcmd", "false"));
> +     ut_asserteq(1, run_command("bootd", 0));
> +     ut_assert_console_end();

'false' is a hush builtin, so this leg only works when the sandbox
build uses that parser. Please either note the assumption in a comment
or pick something that does not depend on the shell.

> diff --git a/test/cmd/bootd.c b/test/cmd/bootd.c
> @@ -0,0 +1,35 @@
> +static int cmd_bootd_test(struct unit_test_state *uts)
> +{
> +     ut_assertok(env_set("bootcmd", "echo hello bootd"));

The test clobbers 'bootcmd' and unsets it at the end without
preserving the previous value. Please save it with env_get() and
restore it in cleanup, so a later test that relies on the environment
is not affected by ordering.

> diff --git a/test/cmd/bootd.c b/test/cmd/bootd.c
> @@ -0,0 +1,35 @@
> +     ut_assertok(env_set("bootcmd", NULL));
> +
> +     return 0;
> +}
> +CMD_TEST(cmd_bootd_test, UTF_CONSOLE);

The cover letter mentions the recursion guard in cmd_process() never
fires on sandbox, so I won't ask for a test of that here, but it would
be nice to assert that do_bootd() itself, when invoked with
CMD_FLAG_BOOTD and bootcmd='bootd', does not recurse - that part of
the guard lives in the code you moved, and you can exercise it
directly with cmd_process_error() or by calling do_bootd() with the
flag set. What do you think?

Regards,
Simon