Hi Stephen, On 2 December 2015 at 15:18, Stephen Warren <swar...@wwwdotorg.org> wrote: > From: Stephen Warren <swar...@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 <swar...@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() > + > +@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") > + > +@pytest.mark.buildconfigspec("sys_hush_parser") > +@pytest.mark.parametrize("expr,result", subtests) > +def test_hush_if_test(uboot_console, expr, result): > + exec_hush_if(uboot_console, expr, result) > + > +@pytest.mark.buildconfigspec("sys_hush_parser") > +def test_hush_if_test_teardown(uboot_console): > + uboot_console.run_command("setenv ut_var_exists") > + > +@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. > +@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 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot