[PATCH v3 6/6] test: cmd: setexpr: add format string tests

[Simon Glass]

Hi Roland,

On Fri, 23 Jul 2021 at 06:30, Roland Gaudig
<roland.gaudig-oss at weidmueller.com> wrote:
>
> From: Roland Gaudig <roland.gaudig at weidmueller.com>
>
> Add test cases for the setexpr format string operator.
>
> Signed-off-by: Roland Gaudig <roland.gaudig at weidmueller.com>
> ---
>
> (no changes since v1)
>
>  test/cmd/setexpr.c | 84 ++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 84 insertions(+)
>
> diff --git a/test/cmd/setexpr.c b/test/cmd/setexpr.c
> index 08b6e6e724..0dc94f7e61 100644
> --- a/test/cmd/setexpr.c
> +++ b/test/cmd/setexpr.c
> @@ -386,6 +386,90 @@ static int setexpr_test_str_long(struct unit_test_state *uts)
>  }
>  SETEXPR_TEST(setexpr_test_str_long, UT_TESTF_CONSOLE_REC);
>
> +#ifdef CONFIG_CMD_SETEXPR_FMT
> +/* Test 'setexpr' command with simply setting integers */

comment needs an update

> +static int setexpr_test_fmt(struct unit_test_state *uts)
> +{
> +       u8 *buf;
> +
> +       buf = map_sysmem(0, BUF_SIZE);
> +       memset(buf, '\xff', BUF_SIZE);
> +
> +       /* Test decimal conversion */
> +       ut_assertok(run_command("setexpr fred fmt %d 0xff", 0));
> +       ut_asserteq_str("255", env_get("fred"));

I suggest a blank line before each comment to make this easier to read.

Also how about adding a helper that does the run_command() and returns
the value of env_get("fred") ? Then your tests might fit in one
statement, and perhaps even one line in some cases.

> +       /* Test hexadecimal conversion with 0x prefix and 4 digits */
> +       ut_assertok(run_command("setexpr fred fmt 0x%04x 257", 0));

nit: that should really be %#04x so we teach people to use that
feature. Another one below.

> +       ut_asserteq_str("0x0257", env_get("fred"));
> +       /* Test octal conversion with % prefix */
> +       ut_assertok(run_command("setexpr fred fmt %%%o 8", 0));
> +       ut_asserteq_str("%10", env_get("fred"));
> +       /* Test argument surrounded by %% */
> +       ut_assertok(run_command("setexpr fred fmt %%%x%% 0xff", 0));
> +       ut_asserteq_str("%ff%", env_get("fred"));
> +       /* Test escape sequence */
> +       ut_assertok(run_command("setexpr fred fmt \"hello\\040world\"", 0));
> +       ut_asserteq_str("hello world", env_get("fred"));
> +       /* Test %b with string containing octal escape sequence */
> +       ut_assertok(run_command("setexpr fred fmt oh%bno \137", 0));
> +       ut_asserteq_str("oh_no", env_get("fred"));
> +       /* Test %b with string containing \c escape sequence */
> +       ut_assertok(run_command("setexpr fred fmt hello%bworld \"\\c\"", 0));
> +       ut_asserteq_str("hello", env_get("fred"));
> +       /* Test multiple arguments referencing environment varialbes */
> +       ut_assertok(run_command("setenv a eff", 0));
> +       ut_assertok(run_command("setenv b hello", 0));
> +       ut_assertok(run_command("setenv c 0x63", 0));
> +       ut_assertok(run_command("setenv d world", 0));
> +       ut_assertok(run_command("setexpr fred fmt \"0x%08x-%s-%d-%s\" $a $b $c $d", 0));
> +       ut_asserteq_str("0x00000eff-hello-99-world", env_get("fred"));
> +       /* Test with two format specifiers, but only one argument */
> +       ut_assertok(run_command("setexpr fred fmt %d_%x 100", 0));
> +       ut_asserteq_str("256_0", env_get("fred"));
> +       /* Test maximum string length */
> +       ut_assertok(run_command("setexpr fred fmt \"%0127d\" 7b", 0));
> +       ut_asserteq_str("0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000123", env_get("fred"));
> +       /* Test maximum unsigned integer size */
> +       ut_assertok(run_command("setexpr fred fmt %u ffffffffffffffff", 0));

I think this needs to check sizeof(ulong) because this will only work
on 64-bit machines.

How about splitting these ones out into two separate test functions,
one for 32-bit and one for 64-bit?

> +       ut_asserteq_str("18446744073709551615", env_get("fred"));
> +       /* Test maximum positive integer size */
> +       ut_assertok(run_command("setexpr fred fmt %d 7fffffffffffffff", 0));
> +       ut_asserteq_str("9223372036854775807", env_get("fred"));
> +       /* Test maximum negative integer size */
> +       ut_assertok(run_command("setexpr fred fmt %d 8000000000000000", 0));
> +       ut_asserteq_str("-9223372036854775808", env_get("fred"));
> +       /* Test minimum negative integer size */
> +       ut_assertok(run_command("setexpr fred fmt %d ffffffffffffffff", 0));
> +       ut_asserteq_str("-1", env_get("fred"));
> +       /* Test signed value with + sign */
> +       ut_assertok(run_command("setexpr fred fmt %d +5", 0));
> +       ut_asserteq_str("5", env_get("fred"));
> +       /* Test signed value with - sign */
> +       ut_assertok(run_command("setexpr fred fmt %d -4", 0));
> +       ut_asserteq_str("-4", env_get("fred"));
> +       /* Test unsigned value with + sign */
> +       ut_assertok(run_command("setexpr fred fmt %u +3", 0));
> +       ut_asserteq_str("3", env_get("fred"));
> +       /* Test unsigned value with - sign */
> +       ut_assertok(run_command("setexpr fred fmt %x -2", 0));
> +       ut_asserteq_str("fffffffffffffffe", env_get("fred"));
> +       /* Error test with missing format specifier */
> +       ut_asserteq(1, run_command("setexpr fred fmd hello 0xff", 0));
> +       /* Error test with invalid format type */
> +       ut_asserteq(1, run_command("setexpr fred fmt %a 0xff", 0));
> +       /* Error test with incomplete format specifier */
> +       ut_asserteq(1, run_command("setexpr fred fmt hello% bf", 0));
> +       /* Error exceeding maximum string length */
> +       ut_asserteq(1, run_command("setexpr fred fmt \"%0128d\" 456", 0));
> +
> +       unmap_sysmem(buf);
> +
> +       return 0;
> +}
> +
> +SETEXPR_TEST(setexpr_test_fmt, UT_TESTF_CONSOLE_REC);
> +#endif
> +
>  int do_ut_setexpr(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
>  {
>         struct unit_test *tests = UNIT_TEST_SUITE_START(setexpr_test);
> --
> 2.25.1
>

Regards,
Simon