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
