[PATCH 0/3] add [ as alias for test, fix 0/1 argument handling

[PATCH 0/3] add [ as alias for test, fix 0/1 argument handling

[Rasmus Villemoes]

Make 'test' behave a little more like its cousins in other shells, by
allowing the [ ... ] spelling, and while here, fix up the handling of
a single, non-empty argument to comply with POSIX.

Rasmus Villemoes (3):
  cmd: test: allow using [ as alias for test
  test: add tests for left-bracket alias for 'test' command
  cmd: test: fix handling of single-argument form of test

 cmd/test.c     | 30 +++++++++++++++++++++++--
 test/hush/if.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 89 insertions(+), 2 deletions(-)

-- 
2.53.0

[PATCH 1/3] cmd: test: allow using [ as alias for test

[Rasmus Villemoes]

I often find myself writing something like

  if [ "$somevar" = 123 ] ; then ...

only to realize that that syntax doesn't work in U-Boot shell, and
must be spelled

  if test "$somevar" = 123 ; then

It only takes a few lines of code to support this POSIX-standardized
alias for test, and helps developers focus on their actual problems
instead of dealing with such unexpected quirks of the shell.

Signed-off-by: Rasmus Villemoes <ravi@prevas.dk>
---
 cmd/test.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/cmd/test.c b/cmd/test.c
index a9ac07e6143..f0645ef98f1 100644
--- a/cmd/test.c
+++ b/cmd/test.c
@@ -63,6 +63,14 @@ static int do_test(struct cmd_tbl *cmdtp, int flag, int argc,
 	char * const *ap;
 	int i, op, left, adv, expr, last_expr, last_unop, last_binop;
 
+	if (!strcmp(argv[0], "[")) {
+		if (strcmp(argv[argc - 1], "]")) {
+			printf("[: missing terminating ]\n");
+			return 1;
+		}
+		argc--;
+	}
+
 	/* args? */
 	if (argc < 3)
 		return 1;
@@ -212,6 +220,17 @@ U_BOOT_CMD(
 	"[args..]"
 );
 
+/*
+ * This does not use the U_BOOT_CMD macro as [ can't be used in symbol names
+ */
+ll_entry_declare(struct cmd_tbl, lbracket, cmd) = {
+	"[",	CONFIG_SYS_MAXARGS, cmd_always_repeatable, do_test,
+	"alias for 'test'",
+#ifdef CONFIG_SYS_LONGHELP
+	" <test expression> ]"
+#endif /* CONFIG_SYS_LONGHELP */
+};
+
 static int do_false(struct cmd_tbl *cmdtp, int flag, int argc,
 		    char *const argv[])
 {
-- 
2.53.0

[PATCH 2/3] test: add tests for left-bracket alias for 'test' command

[Rasmus Villemoes]

Duplicate a few of the existing test cases, using the [ spelling, and
also ensure that the presence of a matching ] as a separate and last
argument is enforced.

Signed-off-by: Rasmus Villemoes <ravi@prevas.dk>
---
 test/hush/if.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/test/hush/if.c b/test/hush/if.c
index ea615b246a9..148e9a53e90 100644
--- a/test/hush/if.c
+++ b/test/hush/if.c
@@ -315,3 +315,34 @@ static int hush_test_if_z_operator(struct unit_test_state *uts)
 	return 0;
 }
 HUSH_TEST(hush_test_if_z_operator, 0);
+
+static int hush_test_lbracket_alias(struct unit_test_state *uts)
+{
+	char if_formatted[128];
+	const char *missing_rbracket_error = "[: missing terminating ]";
+
+	sprintf(if_formatted, if_format, "[ aaa = aaa ]");
+	ut_assertok(run_command(if_formatted, 0));
+
+	sprintf(if_formatted, if_format, "[ aaa = bbb ]");
+	ut_asserteq(1, run_command(if_formatted, 0));
+
+	sprintf(if_formatted, if_format, "[ aaa = aaa");
+	ut_asserteq(1, run_command(if_formatted, 0));
+	ut_assert_nextline(missing_rbracket_error);
+
+	sprintf(if_formatted, if_format, "[ aaa = bbb");
+	ut_asserteq(1, run_command(if_formatted, 0));
+	ut_assert_nextline(missing_rbracket_error);
+
+	sprintf(if_formatted, if_format, "[ aaa = aaa]");
+	ut_asserteq(1, run_command(if_formatted, 0));
+	ut_assert_nextline(missing_rbracket_error);
+
+	sprintf(if_formatted, if_format, "[ aaa = bbb]");
+	ut_asserteq(1, run_command(if_formatted, 0));
+	ut_assert_nextline(missing_rbracket_error);
+
+	return 0;
+}
+HUSH_TEST(hush_test_lbracket_alias, UTF_CONSOLE);
-- 
2.53.0

[PATCH 3/3] cmd: test: fix handling of single-argument form of test

[Rasmus Villemoes]

POSIX states that

  0 arguments:
      Exit false (1).
  1 argument:
      Exit true (0) if $1 is not null; otherwise, exit false.

and at least bash and busybox sh behave that way.

The current 'argc < 3' does the right thing for a non-existing or
empty argv[1], but not for a non-empty argv[1]. Fix that and add
corresponding test cases.

Signed-off-by: Rasmus Villemoes <ravi@prevas.dk>
---
 cmd/test.c     | 11 +++++++++--
 test/hush/if.c | 30 ++++++++++++++++++++++++++++++
 2 files changed, 39 insertions(+), 2 deletions(-)

diff --git a/cmd/test.c b/cmd/test.c
index f0645ef98f1..0d0f090386c 100644
--- a/cmd/test.c
+++ b/cmd/test.c
@@ -71,10 +71,17 @@ static int do_test(struct cmd_tbl *cmdtp, int flag, int argc,
 		argc--;
 	}
 
-	/* args? */
-	if (argc < 3)
+	/*
+	 * Per POSIX, 'test' with 0 arguments should return 1, while
+	 * 'test <arg>' should be equivalent to 'test -n <arg>',
+	 * i.e. true if and only if <arg> is not empty.
+	 */
+	if (argc < 2)
 		return 1;
 
+	if (argc == 2)
+		return !strcmp(argv[1], "");
+
 #ifdef DEBUG
 	{
 		debug("test(%d):", argc);
diff --git a/test/hush/if.c b/test/hush/if.c
index 148e9a53e90..e82a95aaf27 100644
--- a/test/hush/if.c
+++ b/test/hush/if.c
@@ -32,6 +32,15 @@ static int hush_test_if_base(struct unit_test_state *uts)
 	sprintf(if_formatted, if_format, "false");
 	ut_asserteq(1, run_command(if_formatted, 0));
 
+	sprintf(if_formatted, if_format, "test");
+	ut_asserteq(1, run_command(if_formatted, 0));
+
+	sprintf(if_formatted, if_format, "test ''");
+	ut_asserteq(1, run_command(if_formatted, 0));
+
+	sprintf(if_formatted, if_format, "test 'abc'");
+	ut_assertok(run_command(if_formatted, 0));
+
 	return 0;
 }
 HUSH_TEST(hush_test_if_base, 0);
@@ -343,6 +352,27 @@ static int hush_test_lbracket_alias(struct unit_test_state *uts)
 	ut_asserteq(1, run_command(if_formatted, 0));
 	ut_assert_nextline(missing_rbracket_error);
 
+	sprintf(if_formatted, if_format, "[ ]");
+	ut_asserteq(1, run_command(if_formatted, 0));
+
+	sprintf(if_formatted, if_format, "[");
+	ut_asserteq(1, run_command(if_formatted, 0));
+	ut_assert_nextline(missing_rbracket_error);
+
+	sprintf(if_formatted, if_format, "[ '' ]");
+	ut_asserteq(1, run_command(if_formatted, 0));
+
+	sprintf(if_formatted, if_format, "[ ''");
+	ut_asserteq(1, run_command(if_formatted, 0));
+	ut_assert_nextline(missing_rbracket_error);
+
+	sprintf(if_formatted, if_format, "[ 'abc' ]");
+	ut_assertok(run_command(if_formatted, 0));
+
+	sprintf(if_formatted, if_format, "[ 'abc'");
+	ut_asserteq(1, run_command(if_formatted, 0));
+	ut_assert_nextline(missing_rbracket_error);
+
 	return 0;
 }
 HUSH_TEST(hush_test_lbracket_alias, UTF_CONSOLE);
-- 
2.53.0

Re: [PATCH 1/3] cmd: test: allow using [ as alias for test

[Quentin Schulz]

Hi Rasmus,

On 3/11/26 1:09 PM, Rasmus Villemoes wrote:
> I often find myself writing something like
> 
>    if [ "$somevar" = 123 ] ; then ...
> 
> only to realize that that syntax doesn't work in U-Boot shell, and
> must be spelled
> 
>    if test "$somevar" = 123 ; then
> 
> It only takes a few lines of code to support this POSIX-standardized
> alias for test, and helps developers focus on their actual problems
> instead of dealing with such unexpected quirks of the shell.
> 
> Signed-off-by: Rasmus Villemoes <ravi@prevas.dk>
> ---
>   cmd/test.c | 19 +++++++++++++++++++

Please don't forget to update the docs. It still states "Unlike in 
ordinary shells, it cannot be spelled [.", c.f. 
https://docs.u-boot.org/en/latest/usage/cmd/test.html#description. The 
synopsis should also have a few examples using the new syntax.

Another question is if you have thought about/tested multi-expressions?

e.g.

[ aaa = aaa -a bbb = bbb ]

and possibly also

[ ! aaa = aaa ]

which I'm assuming should be equivalent to [ aaa != aaa ] (of course 
this test doesn't make much sense, but there seems to be support for the 
-e operator for checking file existence). I think adding tests for those 
would be nice too?

Cheers,
Quentin

Re: [PATCH 1/3] cmd: test: allow using [ as alias for test

[Rasmus Villemoes]

On Wed, Mar 11 2026, Quentin Schulz <quentin.schulz@cherry.de> wrote:

> Hi Rasmus,
>
> On 3/11/26 1:09 PM, Rasmus Villemoes wrote:
>> I often find myself writing something like
>>    if [ "$somevar" = 123 ] ; then ...
>> only to realize that that syntax doesn't work in U-Boot shell, and
>> must be spelled
>>    if test "$somevar" = 123 ; then
>> It only takes a few lines of code to support this POSIX-standardized
>> alias for test, and helps developers focus on their actual problems
>> instead of dealing with such unexpected quirks of the shell.
>> Signed-off-by: Rasmus Villemoes <ravi@prevas.dk>
>> ---
>>   cmd/test.c | 19 +++++++++++++++++++
>
> Please don't forget to update the docs. It still states "Unlike in
> ordinary shells, it cannot be spelled [.",
> c.f. https://docs.u-boot.org/en/latest/usage/cmd/test.html#description. The
> synopsis should also have a few examples using the new syntax.

Ah, yes, thanks. Rather amazing that I could complete forget having
written that only 10 months ago...

Will update.

> Another question is if you have thought about/tested multi-expressions?
>
> e.g.
>
> [ aaa = aaa -a bbb = bbb ]
>
> and possibly also
>
> [ ! aaa = aaa ]
>

Yes, I did think about those, and I can certainly copy-paste a few more
test cases and spell them using [], but as the implementation is so
simple (check last argv, decrement argc) I couldn't really imagine those
being any different than the simple cases.

> I think adding tests for those would be nice too?

Sure, will do.

Rasmus

Re: [PATCH 2/3] test: add tests for left-bracket alias for 'test' command

[Simon Glass]

Hi Rasmus,

On Wed, 11 Mar 2026 at 06:09, Rasmus Villemoes <ravi@prevas.dk> wrote:
>
> Duplicate a few of the existing test cases, using the [ spelling, and
> also ensure that the presence of a matching ] as a separate and last
> argument is enforced.
>
> Signed-off-by: Rasmus Villemoes <ravi@prevas.dk>
> ---
>  test/hush/if.c | 31 +++++++++++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
>
> diff --git a/test/hush/if.c b/test/hush/if.c
> index ea615b246a9..148e9a53e90 100644
> --- a/test/hush/if.c
> +++ b/test/hush/if.c
> @@ -315,3 +315,34 @@ static int hush_test_if_z_operator(struct unit_test_state *uts)
>         return 0;
>  }
>  HUSH_TEST(hush_test_if_z_operator, 0);
> +
> +static int hush_test_lbracket_alias(struct unit_test_state *uts)
> +{
> +       char if_formatted[128];
> +       const char *missing_rbracket_error = "[: missing terminating ]";
> +
> +       sprintf(if_formatted, if_format, "[ aaa = aaa ]");
> +       ut_assertok(run_command(if_formatted, 0));

How about using run_commandf() so you can do this in one line? Looks
good apart from that.

> +
> +       sprintf(if_formatted, if_format, "[ aaa = bbb ]");
> +       ut_asserteq(1, run_command(if_formatted, 0));
> +
> +       sprintf(if_formatted, if_format, "[ aaa = aaa");
> +       ut_asserteq(1, run_command(if_formatted, 0));
> +       ut_assert_nextline(missing_rbracket_error);
> +
> +       sprintf(if_formatted, if_format, "[ aaa = bbb");
> +       ut_asserteq(1, run_command(if_formatted, 0));
> +       ut_assert_nextline(missing_rbracket_error);
> +
> +       sprintf(if_formatted, if_format, "[ aaa = aaa]");
> +       ut_asserteq(1, run_command(if_formatted, 0));
> +       ut_assert_nextline(missing_rbracket_error);
> +
> +       sprintf(if_formatted, if_format, "[ aaa = bbb]");
> +       ut_asserteq(1, run_command(if_formatted, 0));
> +       ut_assert_nextline(missing_rbracket_error);
> +
> +       return 0;
> +}
> +HUSH_TEST(hush_test_lbracket_alias, UTF_CONSOLE);
> --
> 2.53.0
>

Regards,
Simon

Re: [PATCH 3/3] cmd: test: fix handling of single-argument form of test

[Simon Glass]

Hi Rasmus,

On Wed, 11 Mar 2026 at 06:09, Rasmus Villemoes <ravi@prevas.dk> wrote:
>
> POSIX states that
>
>   0 arguments:
>       Exit false (1).
>   1 argument:
>       Exit true (0) if $1 is not null; otherwise, exit false.
>
> and at least bash and busybox sh behave that way.
>
> The current 'argc < 3' does the right thing for a non-existing or
> empty argv[1], but not for a non-empty argv[1]. Fix that and add
> corresponding test cases.
>
> Signed-off-by: Rasmus Villemoes <ravi@prevas.dk>
> ---
>  cmd/test.c     | 11 +++++++++--
>  test/hush/if.c | 30 ++++++++++++++++++++++++++++++
>  2 files changed, 39 insertions(+), 2 deletions(-)
>
> diff --git a/cmd/test.c b/cmd/test.c
> index f0645ef98f1..0d0f090386c 100644
> --- a/cmd/test.c
> +++ b/cmd/test.c
> @@ -71,10 +71,17 @@ static int do_test(struct cmd_tbl *cmdtp, int flag, int argc,
>                 argc--;
>         }
>
> -       /* args? */
> -       if (argc < 3)
> +       /*
> +        * Per POSIX, 'test' with 0 arguments should return 1, while
> +        * 'test <arg>' should be equivalent to 'test -n <arg>',
> +        * i.e. true if and only if <arg> is not empty.
> +        */
> +       if (argc < 2)
>                 return 1;
>
> +       if (argc == 2)
> +               return !strcmp(argv[1], "");
> +
>  #ifdef DEBUG
>         {
>                 debug("test(%d):", argc);
> diff --git a/test/hush/if.c b/test/hush/if.c
> index 148e9a53e90..e82a95aaf27 100644
> --- a/test/hush/if.c
> +++ b/test/hush/if.c
> @@ -32,6 +32,15 @@ static int hush_test_if_base(struct unit_test_state *uts)
>         sprintf(if_formatted, if_format, "false");
>         ut_asserteq(1, run_command(if_formatted, 0));

Can you use run_commandf() here?

>
> +       sprintf(if_formatted, if_format, "test");
> +       ut_asserteq(1, run_command(if_formatted, 0));
> +
> +       sprintf(if_formatted, if_format, "test ''");
> +       ut_asserteq(1, run_command(if_formatted, 0));
> +
> +       sprintf(if_formatted, if_format, "test 'abc'");
> +       ut_assertok(run_command(if_formatted, 0));
> +
>         return 0;
>  }
>  HUSH_TEST(hush_test_if_base, 0);
> @@ -343,6 +352,27 @@ static int hush_test_lbracket_alias(struct unit_test_state *uts)
>         ut_asserteq(1, run_command(if_formatted, 0));
>         ut_assert_nextline(missing_rbracket_error);
>
> +       sprintf(if_formatted, if_format, "[ ]");
> +       ut_asserteq(1, run_command(if_formatted, 0));
> +
> +       sprintf(if_formatted, if_format, "[");
> +       ut_asserteq(1, run_command(if_formatted, 0));
> +       ut_assert_nextline(missing_rbracket_error);
> +
> +       sprintf(if_formatted, if_format, "[ '' ]");
> +       ut_asserteq(1, run_command(if_formatted, 0));
> +
> +       sprintf(if_formatted, if_format, "[ ''");
> +       ut_asserteq(1, run_command(if_formatted, 0));
> +       ut_assert_nextline(missing_rbracket_error);
> +
> +       sprintf(if_formatted, if_format, "[ 'abc' ]");
> +       ut_assertok(run_command(if_formatted, 0));
> +
> +       sprintf(if_formatted, if_format, "[ 'abc'");
> +       ut_asserteq(1, run_command(if_formatted, 0));
> +       ut_assert_nextline(missing_rbracket_error);
> +
>         return 0;
>  }
>  HUSH_TEST(hush_test_lbracket_alias, UTF_CONSOLE);
> --
> 2.53.0
>

Regards,
Simon

Re: [PATCH 2/3] test: add tests for left-bracket alias for 'test' command

[Rasmus Villemoes]

On Sun, Mar 15 2026, Simon Glass <sjg@chromium.org> wrote:

> Hi Rasmus,
>
> On Wed, 11 Mar 2026 at 06:09, Rasmus Villemoes <ravi@prevas.dk> wrote:
>>
>> Duplicate a few of the existing test cases, using the [ spelling, and
>> also ensure that the presence of a matching ] as a separate and last
>> argument is enforced.
>>
>> Signed-off-by: Rasmus Villemoes <ravi@prevas.dk>
>> ---
>>  test/hush/if.c | 31 +++++++++++++++++++++++++++++++
>>  1 file changed, 31 insertions(+)
>>
>> diff --git a/test/hush/if.c b/test/hush/if.c
>> index ea615b246a9..148e9a53e90 100644
>> --- a/test/hush/if.c
>> +++ b/test/hush/if.c
>> @@ -315,3 +315,34 @@ static int hush_test_if_z_operator(struct unit_test_state *uts)
>>         return 0;
>>  }
>>  HUSH_TEST(hush_test_if_z_operator, 0);
>> +
>> +static int hush_test_lbracket_alias(struct unit_test_state *uts)
>> +{
>> +       char if_formatted[128];
>> +       const char *missing_rbracket_error = "[: missing terminating ]";
>> +
>> +       sprintf(if_formatted, if_format, "[ aaa = aaa ]");
>> +       ut_assertok(run_command(if_formatted, 0));
>
> How about using run_commandf() so you can do this in one line? Looks
> good apart from that.
>

I did cringe a little when I saw that repeated sprintf()/run_command()
pattern all over that file, but I preferred to stay consistent with the
existing style.

If anything, one should do a whole-sale conversion of that file, but
then I'd not use run_commandf() directly, as one would still pass in
that if_format argument every time, so rather create a local macro that
wraps run_commandf() and provides that if_format as a literal string,
which would also enable format checking.

Rasmus

Re: [PATCH 2/3] test: add tests for left-bracket alias for 'test' command

[Simon Glass]

Hi Rasmus,

On Mon, 16 Mar 2026 at 02:17, Rasmus Villemoes <ravi@prevas.dk> wrote:
>
> On Sun, Mar 15 2026, Simon Glass <sjg@chromium.org> wrote:
>
> > Hi Rasmus,
> >
> > On Wed, 11 Mar 2026 at 06:09, Rasmus Villemoes <ravi@prevas.dk> wrote:
> >>
> >> Duplicate a few of the existing test cases, using the [ spelling, and
> >> also ensure that the presence of a matching ] as a separate and last
> >> argument is enforced.
> >>
> >> Signed-off-by: Rasmus Villemoes <ravi@prevas.dk>
> >> ---
> >>  test/hush/if.c | 31 +++++++++++++++++++++++++++++++
> >>  1 file changed, 31 insertions(+)
> >>
> >> diff --git a/test/hush/if.c b/test/hush/if.c
> >> index ea615b246a9..148e9a53e90 100644
> >> --- a/test/hush/if.c
> >> +++ b/test/hush/if.c
> >> @@ -315,3 +315,34 @@ static int hush_test_if_z_operator(struct unit_test_state *uts)
> >>         return 0;
> >>  }
> >>  HUSH_TEST(hush_test_if_z_operator, 0);
> >> +
> >> +static int hush_test_lbracket_alias(struct unit_test_state *uts)
> >> +{
> >> +       char if_formatted[128];
> >> +       const char *missing_rbracket_error = "[: missing terminating ]";
> >> +
> >> +       sprintf(if_formatted, if_format, "[ aaa = aaa ]");
> >> +       ut_assertok(run_command(if_formatted, 0));
> >
> > How about using run_commandf() so you can do this in one line? Looks
> > good apart from that.
> >
>
> I did cringe a little when I saw that repeated sprintf()/run_command()
> pattern all over that file, but I preferred to stay consistent with the
> existing style.
>
> If anything, one should do a whole-sale conversion of that file, but
> then I'd not use run_commandf() directly, as one would still pass in
> that if_format argument every time, so rather create a local macro that
> wraps run_commandf() and provides that if_format as a literal string,
> which would also enable format checking.

Well you could have a helper function which takes the string argument
and does the assert. The duplication makes it a bit harder to
maintain.

sprintf(if_formatted, if_format, "test aaa != aaa");
ut_asserteq(1, run_command(if_formatted, 0));

could be something like:

ut_asserteq(1, check_operation("test aaa != aaa"));

If you had the energy to adjust the rest of the file, that seems good too.

Regards,
Simon

Re: [PATCH 2/3] test: add tests for left-bracket alias for 'test' command

[Rasmus Villemoes]

On Tue, Mar 17 2026, Simon Glass <sjg@chromium.org> wrote:

> Hi Rasmus,
>
> On Mon, 16 Mar 2026 at 02:17, Rasmus Villemoes <ravi@prevas.dk> wrote:
>>
>> On Sun, Mar 15 2026, Simon Glass <sjg@chromium.org> wrote:
>>
>> > Hi Rasmus,
>> >
>> > On Wed, 11 Mar 2026 at 06:09, Rasmus Villemoes <ravi@prevas.dk> wrote:
>> >>
>> >> Duplicate a few of the existing test cases, using the [ spelling, and
>> >> also ensure that the presence of a matching ] as a separate and last
>> >> argument is enforced.
>> >>
>> >> Signed-off-by: Rasmus Villemoes <ravi@prevas.dk>
>> >> ---
>> >>  test/hush/if.c | 31 +++++++++++++++++++++++++++++++
>> >>  1 file changed, 31 insertions(+)
>> >>
>> >> diff --git a/test/hush/if.c b/test/hush/if.c
>> >> index ea615b246a9..148e9a53e90 100644
>> >> --- a/test/hush/if.c
>> >> +++ b/test/hush/if.c
>> >> @@ -315,3 +315,34 @@ static int hush_test_if_z_operator(struct unit_test_state *uts)
>> >>         return 0;
>> >>  }
>> >>  HUSH_TEST(hush_test_if_z_operator, 0);
>> >> +
>> >> +static int hush_test_lbracket_alias(struct unit_test_state *uts)
>> >> +{
>> >> +       char if_formatted[128];
>> >> +       const char *missing_rbracket_error = "[: missing terminating ]";
>> >> +
>> >> +       sprintf(if_formatted, if_format, "[ aaa = aaa ]");
>> >> +       ut_assertok(run_command(if_formatted, 0));
>> >
>> > How about using run_commandf() so you can do this in one line? Looks
>> > good apart from that.
>> >
>>
>> I did cringe a little when I saw that repeated sprintf()/run_command()
>> pattern all over that file, but I preferred to stay consistent with the
>> existing style.
>>
>> If anything, one should do a whole-sale conversion of that file, but
>> then I'd not use run_commandf() directly, as one would still pass in
>> that if_format argument every time, so rather create a local macro that
>> wraps run_commandf() and provides that if_format as a literal string,
>> which would also enable format checking.
>
> Well you could have a helper function which takes the string argument
> and does the assert. The duplication makes it a bit harder to
> maintain.
>
> sprintf(if_formatted, if_format, "test aaa != aaa");
> ut_asserteq(1, run_command(if_formatted, 0));
>
> could be something like:
>
> ut_asserteq(1, check_operation("test aaa != aaa"));

Yes, I'd definitely make sure that each test would only occupy one
source line instead of two. What I meant was that I didn't want to
convert

    sprintf(if_formatted, if_format, "test aaa = aaa");
    ut_assertok(run_command(if_formatted, 0));

mechanically into

   ut_assertok(run_commandf(if_format, "test aaa = aaa", 0));

and having to repeat that if_format thing on each and every line.

Also, I think I would prefer to change the assertok()s into asserteq(0,
... ) as I think the mix of assertok() and asserteq(1, ...) is not very
readable.

> If you had the energy to adjust the rest of the file, that seems good too.

I will fix that in a followup if the current patches get merged (they're
at v2 by the way), I do not have the energy to start over and doing the
mechanical changes first and then redo my patches on top of that, so
please don't request that I do that.

Rasmus

Re: [PATCH 2/3] test: add tests for left-bracket alias for 'test' command

[Simon Glass]

Hi Rasmus,

On Tue, 17 Mar 2026 at 09:12, Rasmus Villemoes <ravi@prevas.dk> wrote:
>
> On Tue, Mar 17 2026, Simon Glass <sjg@chromium.org> wrote:
>
> > Hi Rasmus,
> >
> > On Mon, 16 Mar 2026 at 02:17, Rasmus Villemoes <ravi@prevas.dk> wrote:
> >>
> >> On Sun, Mar 15 2026, Simon Glass <sjg@chromium.org> wrote:
> >>
> >> > Hi Rasmus,
> >> >
> >> > On Wed, 11 Mar 2026 at 06:09, Rasmus Villemoes <ravi@prevas.dk> wrote:
> >> >>
> >> >> Duplicate a few of the existing test cases, using the [ spelling, and
> >> >> also ensure that the presence of a matching ] as a separate and last
> >> >> argument is enforced.
> >> >>
> >> >> Signed-off-by: Rasmus Villemoes <ravi@prevas.dk>
> >> >> ---
> >> >>  test/hush/if.c | 31 +++++++++++++++++++++++++++++++
> >> >>  1 file changed, 31 insertions(+)
> >> >>
> >> >> diff --git a/test/hush/if.c b/test/hush/if.c
> >> >> index ea615b246a9..148e9a53e90 100644
> >> >> --- a/test/hush/if.c
> >> >> +++ b/test/hush/if.c
> >> >> @@ -315,3 +315,34 @@ static int hush_test_if_z_operator(struct unit_test_state *uts)
> >> >>         return 0;
> >> >>  }
> >> >>  HUSH_TEST(hush_test_if_z_operator, 0);
> >> >> +
> >> >> +static int hush_test_lbracket_alias(struct unit_test_state *uts)
> >> >> +{
> >> >> +       char if_formatted[128];
> >> >> +       const char *missing_rbracket_error = "[: missing terminating ]";
> >> >> +
> >> >> +       sprintf(if_formatted, if_format, "[ aaa = aaa ]");
> >> >> +       ut_assertok(run_command(if_formatted, 0));
> >> >
> >> > How about using run_commandf() so you can do this in one line? Looks
> >> > good apart from that.
> >> >
> >>
> >> I did cringe a little when I saw that repeated sprintf()/run_command()
> >> pattern all over that file, but I preferred to stay consistent with the
> >> existing style.
> >>
> >> If anything, one should do a whole-sale conversion of that file, but
> >> then I'd not use run_commandf() directly, as one would still pass in
> >> that if_format argument every time, so rather create a local macro that
> >> wraps run_commandf() and provides that if_format as a literal string,
> >> which would also enable format checking.
> >
> > Well you could have a helper function which takes the string argument
> > and does the assert. The duplication makes it a bit harder to
> > maintain.
> >
> > sprintf(if_formatted, if_format, "test aaa != aaa");
> > ut_asserteq(1, run_command(if_formatted, 0));
> >
> > could be something like:
> >
> > ut_asserteq(1, check_operation("test aaa != aaa"));
>
> Yes, I'd definitely make sure that each test would only occupy one
> source line instead of two. What I meant was that I didn't want to
> convert
>
>     sprintf(if_formatted, if_format, "test aaa = aaa");
>     ut_assertok(run_command(if_formatted, 0));
>
> mechanically into
>
>    ut_assertok(run_commandf(if_format, "test aaa = aaa", 0));
>
> and having to repeat that if_format thing on each and every line.
>
> Also, I think I would prefer to change the assertok()s into asserteq(0,
> ... ) as I think the mix of assertok() and asserteq(1, ...) is not very
> readable.
>
> > If you had the energy to adjust the rest of the file, that seems good too.
>
> I will fix that in a followup if the current patches get merged (they're
> at v2 by the way), I do not have the energy to start over and doing the
> mechanical changes first and then redo my patches on top of that, so
> please don't request that I do that.

That's fine with me, please do. Yes I think I missed v1.

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

Regards,
Simon