[U-Boot] [PATCH V2 6/7] test/py: test the shell if command

Simon Glass sjg at chromium.org
Sat Dec 19 23:24:57 CET 2015


Hi Stephen,

On 2 December 2015 at 15:18, Stephen Warren <swarren at wwwdotorg.org> wrote:
> From: Stephen Warren <swarren at nvidia.com>
>
> Migrate all most tests from command_ut.c into the Python test system.
> This allows the tests to be run against any U-Boot binary that supports
> the if command (i.e. where hush is enabled) without requiring that
> binary to be permanently bloated with the code from command_ut.
>
> Some tests in command_ut.c can only be executed from C code, since they
> test internal (more unit-level) features of various U-Boot APIs. The
> migrated tests can all operate directly from the U-Boot console.

This seems to migrate more than just the 'if' tests suggested by the
commit subject. Perhaps it should be split into two?

Is there any point in running these tests on real hardware? It takes
forever due to needing to reset each time. Do we need to reset?

They fail on my board due I think to a problem with the printenv parsing:

Section: test_env_echo_exists
Stream: console
=> printenv
printenv
baudrate=115200
bootargs=root=/dev/sdb3 init=/sbin/init rootwait ro
bootcmd=ext2load scsi 0:3 01000000 /boot/vmlinuz; zboot 01000000
bootfile=bzImage
consoledev=ttyS0
fdtcontroladdr=acd3dbc0
hostname=x86
loadaddr=0x1000000
netdev=eth0
nfsboot=setenv bootargs root=/dev/nfs rw nfsroot=$serverip:$rootpath
ip=$ipaddr:$serverip:$gatewayip:$netmask:$hostname:$netdev:off
console=$consoledev,$baudrate $othbootargs;tftpboot $loadaddr
$bootfile;zboot $loadaddr
othbootargs=acpi=off
pciconfighost=1
ramboot=setenv bootargs root=/dev/ram rw
ip=$ipaddr:$serverip:$gatewayip:$netmask:$hostname:$netdev:off
console=$consoledev,$baudrate $othbootargs;tftpboot $loadaddr
$bootfile;tftpboot $ramdiskaddr $ramdiskfile;zboot $loadaddr 0
$ramdiskaddr $filesize
ramdiskaddr=0x2000000
ramdiskfile=initramfs.gz
rootpath=/opt/nfsroot
scsidevs=1
stderr=vga,serial
stdin=usbkbd,i8042-kbd,serial
stdout=serial

Environment size: 927/4092 bytes
=>
FAILED:
uboot_console = <uboot_console_exec_attach.ConsoleExecAttach object at
0x7feaa0f1a290>

    @pytest.fixture(scope="module")
    def state_test_env(uboot_console):
>       return StateTestEnv(uboot_console)

test/py/test_env.py:39:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <test_env.StateTestEnv object at 0x7feaa0d4b7d0>
uboot_console = <uboot_console_exec_attach.ConsoleExecAttach object at
0x7feaa0f1a290>

    def __init__(self, uboot_console):
        self.uboot_console = uboot_console
>       self.get_env()

test/py/test_env.py:13:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <test_env.StateTestEnv object at 0x7feaa0d4b7d0>

    def get_env(self):
        response = self.uboot_console.run_command("printenv")
        self.env = {}
        for l in response.splitlines():
            if not "=" in l:
                continue
>           (var, value) = l.strip().split("=")
E           ValueError: too many values to unpack

test/py/test_env.py:22: ValueError




>
> Signed-off-by: Stephen Warren <swarren at nvidia.com>
> ---
>  test/command_ut.c            | 133 ----------------------------------------
>  test/py/test_hush_if_test.py | 141 +++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 141 insertions(+), 133 deletions(-)
>  create mode 100644 test/py/test_hush_if_test.py
>
> diff --git a/test/command_ut.c b/test/command_ut.c
> index 926573a39543..35bd35ae2e30 100644
> --- a/test/command_ut.c
> +++ b/test/command_ut.c
> @@ -20,21 +20,6 @@ static int do_ut_cmd(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>         printf("%s: Testing commands\n", __func__);
>         run_command("env default -f -a", 0);
>
> -       /* run a single command */
> -       run_command("setenv single 1", 0);
> -       assert(!strcmp("1", getenv("single")));
> -
> -       /* make sure that compound statements work */
> -#ifdef CONFIG_SYS_HUSH_PARSER
> -       run_command("if test -n ${single} ; then setenv check 1; fi", 0);
> -       assert(!strcmp("1", getenv("check")));
> -       run_command("setenv check", 0);
> -#endif
> -
> -       /* commands separated by ; */
> -       run_command_list("setenv list 1; setenv list ${list}1", -1, 0);
> -       assert(!strcmp("11", getenv("list")));
> -
>         /* commands separated by \n */
>         run_command_list("setenv list 1\n setenv list ${list}1", -1, 0);
>         assert(!strcmp("11", getenv("list")));
> @@ -43,11 +28,6 @@ static int do_ut_cmd(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>         run_command_list("setenv list 1${list}\n", -1, 0);
>         assert(!strcmp("111", getenv("list")));
>
> -       /* three commands in a row */
> -       run_command_list("setenv list 1\n setenv list ${list}2; "
> -               "setenv list ${list}3", -1, 0);
> -       assert(!strcmp("123", getenv("list")));
> -
>         /* a command string with \0 in it. Stuff after \0 should be ignored */
>         run_command("setenv list", 0);
>         run_command_list(test_cmd, sizeof(test_cmd), 0);
> @@ -66,13 +46,6 @@ static int do_ut_cmd(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>         assert(run_command_list("false", -1, 0) == 1);
>         assert(run_command_list("echo", -1, 0) == 0);
>
> -       run_command("setenv foo 'setenv monty 1; setenv python 2'", 0);
> -       run_command("run foo", 0);
> -       assert(getenv("monty") != NULL);
> -       assert(!strcmp("1", getenv("monty")));
> -       assert(getenv("python") != NULL);
> -       assert(!strcmp("2", getenv("python")));
> -
>  #ifdef CONFIG_SYS_HUSH_PARSER
>         run_command("setenv foo 'setenv black 1\nsetenv adder 2'", 0);
>         run_command("run foo", 0);
> @@ -80,112 +53,6 @@ static int do_ut_cmd(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>         assert(!strcmp("1", getenv("black")));
>         assert(getenv("adder") != NULL);
>         assert(!strcmp("2", getenv("adder")));
> -
> -       /* Test the 'test' command */
> -
> -#define HUSH_TEST(name, expr, expected_result) \
> -       run_command("if test " expr " ; then " \
> -                       "setenv " #name "_" #expected_result " y; else " \
> -                       "setenv " #name "_" #expected_result " n; fi", 0); \
> -       assert(!strcmp(#expected_result, getenv(#name "_" #expected_result))); \
> -       setenv(#name "_" #expected_result, NULL);
> -
> -       /* Basic operators */
> -       HUSH_TEST(streq, "aaa = aaa", y);
> -       HUSH_TEST(streq, "aaa = bbb", n);
> -
> -       HUSH_TEST(strneq, "aaa != bbb", y);
> -       HUSH_TEST(strneq, "aaa != aaa", n);
> -
> -       HUSH_TEST(strlt, "aaa < bbb", y);
> -       HUSH_TEST(strlt, "bbb < aaa", n);
> -
> -       HUSH_TEST(strgt, "bbb > aaa", y);
> -       HUSH_TEST(strgt, "aaa > bbb", n);
> -
> -       HUSH_TEST(eq, "123 -eq 123", y);
> -       HUSH_TEST(eq, "123 -eq 456", n);
> -
> -       HUSH_TEST(ne, "123 -ne 456", y);
> -       HUSH_TEST(ne, "123 -ne 123", n);
> -
> -       HUSH_TEST(lt, "123 -lt 456", y);
> -       HUSH_TEST(lt_eq, "123 -lt 123", n);
> -       HUSH_TEST(lt, "456 -lt 123", n);
> -
> -       HUSH_TEST(le, "123 -le 456", y);
> -       HUSH_TEST(le_eq, "123 -le 123", y);
> -       HUSH_TEST(le, "456 -le 123", n);
> -
> -       HUSH_TEST(gt, "456 -gt 123", y);
> -       HUSH_TEST(gt_eq, "123 -gt 123", n);
> -       HUSH_TEST(gt, "123 -gt 456", n);
> -
> -       HUSH_TEST(ge, "456 -ge 123", y);
> -       HUSH_TEST(ge_eq, "123 -ge 123", y);
> -       HUSH_TEST(ge, "123 -ge 456", n);
> -
> -       HUSH_TEST(z, "-z \"\"", y);
> -       HUSH_TEST(z, "-z \"aaa\"", n);
> -
> -       HUSH_TEST(n, "-n \"aaa\"", y);
> -       HUSH_TEST(n, "-n \"\"", n);
> -
> -       /* Inversion of simple tests */
> -       HUSH_TEST(streq_inv, "! aaa = aaa", n);
> -       HUSH_TEST(streq_inv, "! aaa = bbb", y);
> -
> -       HUSH_TEST(streq_inv_inv, "! ! aaa = aaa", y);
> -       HUSH_TEST(streq_inv_inv, "! ! aaa = bbb", n);
> -
> -       /* Binary operators */
> -       HUSH_TEST(or_0_0, "aaa != aaa -o bbb != bbb", n);
> -       HUSH_TEST(or_0_1, "aaa != aaa -o bbb = bbb", y);
> -       HUSH_TEST(or_1_0, "aaa = aaa -o bbb != bbb", y);
> -       HUSH_TEST(or_1_1, "aaa = aaa -o bbb = bbb", y);
> -
> -       HUSH_TEST(and_0_0, "aaa != aaa -a bbb != bbb", n);
> -       HUSH_TEST(and_0_1, "aaa != aaa -a bbb = bbb", n);
> -       HUSH_TEST(and_1_0, "aaa = aaa -a bbb != bbb", n);
> -       HUSH_TEST(and_1_1, "aaa = aaa -a bbb = bbb", y);
> -
> -       /* Inversion within binary operators */
> -       HUSH_TEST(or_0_0_inv, "! aaa != aaa -o ! bbb != bbb", y);
> -       HUSH_TEST(or_0_1_inv, "! aaa != aaa -o ! bbb = bbb", y);
> -       HUSH_TEST(or_1_0_inv, "! aaa = aaa -o ! bbb != bbb", y);
> -       HUSH_TEST(or_1_1_inv, "! aaa = aaa -o ! bbb = bbb", n);
> -
> -       HUSH_TEST(or_0_0_inv_inv, "! ! aaa != aaa -o ! ! bbb != bbb", n);
> -       HUSH_TEST(or_0_1_inv_inv, "! ! aaa != aaa -o ! ! bbb = bbb", y);
> -       HUSH_TEST(or_1_0_inv_inv, "! ! aaa = aaa -o ! ! bbb != bbb", y);
> -       HUSH_TEST(or_1_1_inv_inv, "! ! aaa = aaa -o ! ! bbb = bbb", y);
> -
> -       setenv("ut_var_nonexistent", NULL);
> -       setenv("ut_var_exists", "1");
> -       HUSH_TEST(z_varexp_quoted, "-z \"$ut_var_nonexistent\"", y);
> -       HUSH_TEST(z_varexp_quoted, "-z \"$ut_var_exists\"", n);
> -       setenv("ut_var_exists", NULL);
> -
> -       run_command("setenv ut_var_space \" \"", 0);
> -       assert(!strcmp(getenv("ut_var_space"), " "));
> -       run_command("setenv ut_var_test $ut_var_space", 0);
> -       assert(!getenv("ut_var_test"));
> -       run_command("setenv ut_var_test \"$ut_var_space\"", 0);
> -       assert(!strcmp(getenv("ut_var_test"), " "));
> -       run_command("setenv ut_var_test \" 1${ut_var_space}${ut_var_space} 2 \"", 0);
> -       assert(!strcmp(getenv("ut_var_test"), " 1   2 "));
> -       setenv("ut_var_space", NULL);
> -       setenv("ut_var_test", NULL);
> -
> -#ifdef CONFIG_SANDBOX
> -       /* File existence */
> -       HUSH_TEST(e, "-e hostfs - creating_this_file_breaks_uboot_unit_test", n);
> -       run_command("sb save hostfs - creating_this_file_breaks_uboot_unit_test 0 1", 0);
> -       HUSH_TEST(e, "-e hostfs - creating_this_file_breaks_uboot_unit_test", y);
> -       /* Perhaps this could be replaced by an "rm" shell command one day */
> -       assert(!os_unlink("creating_this_file_breaks_uboot_unit_test"));
> -       HUSH_TEST(e, "-e hostfs - creating_this_file_breaks_uboot_unit_test", n);
> -#endif

Are you able to drop the os.h header?

>  #endif
>
>         assert(run_command("", 0) == 0);
> diff --git a/test/py/test_hush_if_test.py b/test/py/test_hush_if_test.py
> new file mode 100644
> index 000000000000..e39e53613500
> --- /dev/null
> +++ b/test/py/test_hush_if_test.py
> @@ -0,0 +1,141 @@
> +# Copyright (c) 2015, NVIDIA CORPORATION. All rights reserved.
> +#
> +# SPDX-License-Identifier: GPL-2.0
> +
> +import os
> +import os.path
> +import pytest
> +
> +subtests = (
> +    # Base if functionality
> +
> +    ("true", True),
> +    ("false", False),
> +
> +    # Basic operators
> +
> +    ("test aaa = aaa", True),
> +    ("test aaa = bbb", False),
> +
> +    ("test aaa != bbb", True),
> +    ("test aaa != aaa", False),
> +
> +    ("test aaa < bbb", True),
> +    ("test bbb < aaa", False),
> +
> +    ("test bbb > aaa", True),
> +    ("test aaa > bbb", False),
> +
> +    ("test 123 -eq 123", True),
> +    ("test 123 -eq 456", False),
> +
> +    ("test 123 -ne 456", True),
> +    ("test 123 -ne 123", False),
> +
> +    ("test 123 -lt 456", True),
> +    ("test 123 -lt 123", False),
> +    ("test 456 -lt 123", False),
> +
> +    ("test 123 -le 456", True),
> +    ("test 123 -le 123", True),
> +    ("test 456 -le 123", False),
> +
> +    ("test 456 -gt 123", True),
> +    ("test 123 -gt 123", False),
> +    ("test 123 -gt 456", False),
> +
> +    ("test 456 -ge 123", True),
> +    ("test 123 -ge 123", True),
> +    ("test 123 -ge 456", False),
> +
> +    ("test -z \"\"", True),
> +    ("test -z \"aaa\"", False),
> +
> +    ("test -n \"aaa\"", True),
> +    ("test -n \"\"", False),
> +
> +    # Inversion of simple tests
> +
> +    ("test ! aaa = aaa", False),
> +    ("test ! aaa = bbb", True),
> +    ("test ! ! aaa = aaa", True),
> +    ("test ! ! aaa = bbb", False),
> +
> +    # Binary operators
> +
> +    ("test aaa != aaa -o bbb != bbb", False),
> +    ("test aaa != aaa -o bbb = bbb", True),
> +    ("test aaa = aaa -o bbb != bbb", True),
> +    ("test aaa = aaa -o bbb = bbb", True),
> +
> +    ("test aaa != aaa -a bbb != bbb", False),
> +    ("test aaa != aaa -a bbb = bbb", False),
> +    ("test aaa = aaa -a bbb != bbb", False),
> +    ("test aaa = aaa -a bbb = bbb", True),
> +
> +    # Inversion within binary operators
> +
> +    ("test ! aaa != aaa -o ! bbb != bbb", True),
> +    ("test ! aaa != aaa -o ! bbb = bbb", True),
> +    ("test ! aaa = aaa -o ! bbb != bbb", True),
> +    ("test ! aaa = aaa -o ! bbb = bbb", False),
> +
> +    ("test ! ! aaa != aaa -o ! ! bbb != bbb", False),
> +    ("test ! ! aaa != aaa -o ! ! bbb = bbb", True),
> +    ("test ! ! aaa = aaa -o ! ! bbb != bbb", True),
> +    ("test ! ! aaa = aaa -o ! ! bbb = bbb", True),
> +
> +    # -z operator
> +
> +    ("test -z \"$ut_var_nonexistent\"", True),
> +    ("test -z \"$ut_var_exists\"", False),
> +)
> +
> +def exec_hush_if(uboot_console, expr, result):
> +    cmd = "if " + expr + "; then echo true; else echo false; fi"
> +    response = uboot_console.run_command(cmd)
> +    assert response.strip() == str(result).lower()
> +
> + at pytest.mark.buildconfigspec("sys_hush_parser")
> +def test_hush_if_test_setup(uboot_console):
> +    uboot_console.run_command("setenv ut_var_nonexistent")
> +    uboot_console.run_command("setenv ut_var_exists 1")
> +
> + at pytest.mark.buildconfigspec("sys_hush_parser")
> + at pytest.mark.parametrize("expr,result", subtests)
> +def test_hush_if_test(uboot_console, expr, result):
> +    exec_hush_if(uboot_console, expr, result)
> +
> + at pytest.mark.buildconfigspec("sys_hush_parser")
> +def test_hush_if_test_teardown(uboot_console):
> +    uboot_console.run_command("setenv ut_var_exists")
> +
> + at pytest.mark.buildconfigspec("sys_hush_parser")
> +# We might test this on real filesystems via UMS, DFU, "save", etc.
> +# Of those, only UMS currently allows file removal though.
> + at pytest.mark.boardspec("sandbox")
> +def test_hush_if_test_host_file_exists(uboot_console):
> +    test_file = uboot_console.config.result_dir + \
> +        "/creating_this_file_breaks_uboot_tests"
> +
> +    try:
> +        os.unlink(test_file)
> +    except:
> +        pass
> +    assert not os.path.exists(test_file)
> +
> +    expr = "test -e hostfs - " + test_file
> +    exec_hush_if(uboot_console, expr, False)
> +
> +    try:
> +        with file(test_file, "wb"):
> +            pass
> +        assert os.path.exists(test_file)
> +
> +        expr = "test -e hostfs - " + test_file
> +        exec_hush_if(uboot_console, expr, True)
> +    finally:
> +        os.unlink(test_file)
> +
> +    expr = "test -e hostfs - " + test_file
> +    exec_hush_if(uboot_console, expr, False)
> --
> 2.6.3
>

Following up on our previous discussion the tests become slightly
slower, but not enough to matter, with your new series. Arguably the
tests are a bit harder to adjust, and harder to debug (how do I run
gdb?). Any ideas on that?

Also how will we run the existing command unit tests? There should
definitely be a way to do that.

Regards,
Simon


More information about the U-Boot mailing list