Hi Maxin,
On 2026-10-29T17:34:37+11:00, Maxin John <[email protected]> wrote:
> test: cmd: add tests for the binop command
>
> binop has had no tests so far. Add some, so that bugs like the heap
> overrun are caught in CI.
>
> Each operation (xor, or, and) is run on hex strings, env vars or
> memory, with the result going to an env var, memory or the console.
>
> The tests also check that:
> - short inputs are zero-padded
> - console output wraps after 16 bytes
> - bad counts, operators, addresses or hex strings are rejected
> without touching the destination
>
> There is also a test for the heap overrun.
>
> Signed-off-by: Maxin B. John <[email protected]>
>
> test/cmd/Makefile | 1 +
> test/cmd/binop.c | 192
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 193 insertions(+)
> diff --git a/test/cmd/binop.c b/test/cmd/binop.c
> @@ -0,0 +1,192 @@
> + /* Output wraps after 16 bytes */
> + ut_assertok(run_command("binop xor 17
> 000102030405060708090a0b0c0d0e0f10 00",
> + 0));
> + ut_assert_nextline("00 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f ");
> + ut_assert_nextline("10 ");
> + ut_assert_console_end();
Please can you add a case with a count of exactly 16? With the current
loop, i == 15 outputs a newline and the final puts('\n') outputs
another, so the user sees a blank line after the data. If that is
intended, a test should pin it down. Otherwise it is worth fixing in
the binop clean-up patch, since this is the edge case most likely to
go wrong.
> diff --git a/test/cmd/binop.c b/test/cmd/binop.c
> @@ -0,0 +1,192 @@
> +static int cmd_test_binop_invalid(struct unit_test_state *uts)
> +{
> + /* Failed operations must not change binop_out */
> + ut_assertok(env_set("binop_out", "deadbeef"));
> +
> + /* Literal hex string: too long, odd length, not hex */
> + ut_asserteq(1, run_command("binop xor 2 010203 01 binop_out", 0));
run_command() returns 1 for every failure, including a usage error, so
these checks cannot tell which validation rejected the input. For
example, if '*nope' were later accepted as address 0 and the command
failed for some other reason, the test would still pass. It would be
worth making this a UTF_CONSOLE test and checking the error message
(or the usage text) after each command, as other tests in test/cmd do.
What do you think?
Regards,
Simon
