[Fuego] [PATCH] busybox: add support for command:

Tim Bird tbird20d at gmail.com
Fri Aug 17 23:04:41 UTC 2018


Comments inline below.

To shorten the review cycle, I plan to accept the patch, and then
do a fixup patch myself for any issues detected.  (I need to plow through
my patch backlog.)

If you disagree with any items, or I state that I'd like you to fix them, please
examine them, and submit another patch on top of this one.

On Sun, Jul 29, 2018 at 10:49 PM, Wang Mingyu <wangmy at cn.fujitsu.com> wrote:
> kill/killall/ln/logger/ls/mkdir/mknod/mktemp/more/mount/mv
>
> Signed-off-by: Wang Mingyu <wangmy at cn.fujitsu.com>
> ---
>  .../tests/Functional.busybox/tests/busybox_kill.sh | 24 +++++++++++++++++
>  .../Functional.busybox/tests/busybox_killall.sh    | 26 ++++++++++++++++++
>  .../tests/Functional.busybox/tests/busybox_ln.sh   | 31 ++++++++++++++++++++++
>  .../Functional.busybox/tests/busybox_logger.sh     | 14 ++++++++++
>  .../tests/Functional.busybox/tests/busybox_ls.sh   | 18 +++++++++++++
>  .../Functional.busybox/tests/busybox_mkdir.sh      | 15 +++++++++++
>  .../Functional.busybox/tests/busybox_mknod.sh      | 16 +++++++++++
>  .../Functional.busybox/tests/busybox_mktemp.sh     | 25 +++++++++++++++++
>  .../tests/Functional.busybox/tests/busybox_more.sh | 16 +++++++++++
>  .../Functional.busybox/tests/busybox_mount.sh      | 13 +++++++++
>  .../tests/Functional.busybox/tests/busybox_mv.sh   | 27 +++++++++++++++++++
>  11 files changed, 225 insertions(+)
>  create mode 100644 engine/tests/Functional.busybox/tests/busybox_kill.sh
>  create mode 100644 engine/tests/Functional.busybox/tests/busybox_killall.sh
>  create mode 100644 engine/tests/Functional.busybox/tests/busybox_ln.sh
>  create mode 100644 engine/tests/Functional.busybox/tests/busybox_logger.sh
>  create mode 100644 engine/tests/Functional.busybox/tests/busybox_ls.sh
>  create mode 100644 engine/tests/Functional.busybox/tests/busybox_mkdir.sh
>  create mode 100644 engine/tests/Functional.busybox/tests/busybox_mknod.sh
>  create mode 100644 engine/tests/Functional.busybox/tests/busybox_mktemp.sh
>  create mode 100644 engine/tests/Functional.busybox/tests/busybox_more.sh
>  create mode 100644 engine/tests/Functional.busybox/tests/busybox_mount.sh
>  create mode 100644 engine/tests/Functional.busybox/tests/busybox_mv.sh
>
> diff --git a/engine/tests/Functional.busybox/tests/busybox_kill.sh b/engine/tests/Functional.busybox/tests/busybox_kill.sh
> new file mode 100644
> index 0000000..954c694
> --- /dev/null
> +++ b/engine/tests/Functional.busybox/tests/busybox_kill.sh
> @@ -0,0 +1,24 @@
> +#!/bin/sh
> +
> +#  The testscript checks the following options of the command kill
> +#  1) Option none
> +
> +test="kill"
> +
> +sleep 2000 & pid=$!
Should use a weird number and include that in the grep, to avoid a false
positive with another sleep on the system.  Also, shorten the sleep so it
doesn't hang around for too long, if the test fails for some reason.

e.g. change to
sleep  283 & pid=$!

> +if [ $(ps | grep sleep | wc -l) = 1 ]
Use patterns to avoid catching 'grep' in ps.
Use 'ps a' to get arguments (not just the commands)
Make sure to match just the sleep I started.

if [ $(ps a | grep "[s]leep 283" | wc -l) = 1 ]

> +then
> +    echo " -> $test: sleep 2000 succeeded."
> +else
> +    echo " -> $test: sleep 2000 failed."
Change these lines to match the seconds actually used.

> +    echo " -> $test: TEST-FAIL"
> +    exit
> +fi;
> +
> +busybox kill -9 $pid
> +if [ $(ps | grep sleep | wc -l) = 0 ]
Alter this line as above.

> +then
> +    echo " -> $test: TEST-PASS"
> +else
> +    echo " -> $test: TEST-FAIL"
> +fi;
> diff --git a/engine/tests/Functional.busybox/tests/busybox_killall.sh b/engine/tests/Functional.busybox/tests/busybox_killall.sh
> new file mode 100644
> index 0000000..7ccff75
> --- /dev/null
> +++ b/engine/tests/Functional.busybox/tests/busybox_killall.sh
> @@ -0,0 +1,26 @@
> +#!/bin/sh
> +
> +#  The testscript checks the following options of the command killall
> +#  1) Option none
> +
> +test="killall"
> +
> +sleep 1500 &
> +sleep 1500 &
Same issue as above - use a unique 'seconds' argument.

> +
> +if ps | grep sleep
change greps to find only this test's sleeps.

> +then
> +    echo " -> $test: sleep successed."
succeeded

> +else
> +    echo " -> $test: sleep failed."
> +    echo " -> $test: TEST-FAIL"
> +    exit
> +fi
> +
> +busybox killall sleep
> +if ps | grep sleep
change grep to find only this test's sleeps.

> +then
> +    echo " -> $test: TEST-FAIL"
> +else
> +    echo " -> $test: TEST-PASS"
> +fi;
> diff --git a/engine/tests/Functional.busybox/tests/busybox_ln.sh b/engine/tests/Functional.busybox/tests/busybox_ln.sh
> new file mode 100644
> index 0000000..291a224
> --- /dev/null
> +++ b/engine/tests/Functional.busybox/tests/busybox_ln.sh
> @@ -0,0 +1,31 @@
> +#!/bin/sh
> +
> +#  The testscript checks the following options of the command ln
> +#  1) Option: -s
> +
> +test="ln"
> +
> +mkdir test_dir
> +echo "Hello world." > ./test_dir/test1
> +busybox ln -s test1 ./test_dir/link_to_test1
> +busybox ls -l ./test_dir > log
should use just 'ls' here, not busybox ls.

> +if head -n 1 log | grep "link_to_test1" && tail -n 1 log | grep "test1"

This can be simplified.  Some ls-es don't sort properly, so you
shouldn't count on the order.  There's no need to test for
'test1'.  I simplified this to:
if grep link_to_other_file log

> +then
> +    echo " -> $test: Sym Link to file created."
'test1' can match either the file 'test1' or the link 'link_to_test1'
Should change this so the filename and the link don't use
matching strings.  (I used 'link_to_other_file')

> +else
> +    echo " -> $test: Sym Link to file creation failure."
> +    echo " -> $test: TEST-FAIL"
> +    rm log
> +    rm -rf test_dir
> +    exit
> +fi;
> +
> +echo "changed text." > ./test_dir/test1
> +if [ "$(busybox more ./test_dir/link_to_test1)" = "changed text." ]
use of 'more' here is not needed.  'cat' will do.

> +then
> +    echo " -> $test: TEST-PASS"
> +else
> +    echo " -> $test: TEST-FAIL"
> +fi;
> +rm log;
> +rm -rf test_dir;
> diff --git a/engine/tests/Functional.busybox/tests/busybox_logger.sh b/engine/tests/Functional.busybox/tests/busybox_logger.sh
> new file mode 100644
> index 0000000..bc3ae5b
> --- /dev/null
> +++ b/engine/tests/Functional.busybox/tests/busybox_logger.sh
> @@ -0,0 +1,14 @@
> +#!/bin/sh
> +
> +#  The testscript checks the following options of the command logger
> +#  1) Option none
> +
> +test="logger"
> +
> +busybox logger "This is a test message $(date)"
should save the $date string here, and check for it with grep, to
avoid a false positive on a string from a previous run of this test
already in the log.

> +if  cat /var/log/syslog | grep "This is a test message"
Can use grep without cat here.

> +then
> +    echo " -> $test: TEST-PASS"
> +else
> +    echo " -> $test: TEST-FAIL"
> +fi;


> diff --git a/engine/tests/Functional.busybox/tests/busybox_ls.sh b/engine/tests/Functional.busybox/tests/busybox_ls.sh
> new file mode 100644
> index 0000000..66561cd
> --- /dev/null
> +++ b/engine/tests/Functional.busybox/tests/busybox_ls.sh
> @@ -0,0 +1,18 @@
> +#!/bin/sh
> +
> +#  The testscript checks the following options of the command ls
> +#  1) Option: -l
> +
> +test="ls"
> +
> +mkdir test_dir
> +touch test_dir/test1 test_dir/test2 test_dir/test3
> +busybox ls -l ./test_dir > log
> +if head -n 1 log | grep "test1" && head -n 2 log | tail -n 1 | grep "test2" && tail -n 1 log | grep "test3"
Some ls-es don't sort.  You shouldn't count on the order of the items
in log here.
This simplifies this to:
if grep "test1" log && grep "test2" log && grep "test3" log

> +then
> +    echo " -> $test: TEST-PASS"
> +else
> +    echo " -> $test: TEST-FAIL"
> +fi;
> +rm log;
> +rm -rf test_dir;

Would be nice to validate some of the other fields produced by ls -l here.

> diff --git a/engine/tests/Functional.busybox/tests/busybox_mkdir.sh b/engine/tests/Functional.busybox/tests/busybox_mkdir.sh
> new file mode 100644
> index 0000000..d7cdc44
> --- /dev/null
> +++ b/engine/tests/Functional.busybox/tests/busybox_mkdir.sh
> @@ -0,0 +1,15 @@
> +#!/bin/sh
> +
> +#  The testscript checks the following options of the command mkdir
> +#  1) Option none
> +
> +test="mkdir"
> +
> +busybox mkdir test_dir
> +if ls -l | grep test_dir
> +then
> +    echo " -> $test: TEST-PASS"
> +else
> +    echo " -> $test: TEST-FAIL"
> +fi;
> +rmdir test_dir;
> diff --git a/engine/tests/Functional.busybox/tests/busybox_mknod.sh b/engine/tests/Functional.busybox/tests/busybox_mknod.sh
> new file mode 100644
> index 0000000..80d5d4c
> --- /dev/null
> +++ b/engine/tests/Functional.busybox/tests/busybox_mknod.sh
> @@ -0,0 +1,16 @@
> +#!/bin/sh
> +
> +#  The testscript checks the following options of the command mknod
> +#  1) Option: -m
> +
> +test="mknod"
> +
> +mkdir test_dir
> +busybox mknod -m 666 test_dir/test_pipe p
> +if ls -l test_dir | grep "prw\-rw\-rw\-.*test_pipe"
This line had a trailing space in the patch.  Please check your scripts
with this: grep " $" <script>, before committing them.

> +then
> +    echo " -> $test: TEST-PASS"
> +else
> +    echo " -> $test: TEST-FAIL"
> +fi;
> +rm -rf test_dir;
> diff --git a/engine/tests/Functional.busybox/tests/busybox_mktemp.sh b/engine/tests/Functional.busybox/tests/busybox_mktemp.sh
> new file mode 100644
> index 0000000..080ebd8
> --- /dev/null
> +++ b/engine/tests/Functional.busybox/tests/busybox_mktemp.sh
> @@ -0,0 +1,25 @@
> +#!/bin/sh
> +
> +#  The testscript checks the following options of the command mktemp
> +#  1) Option none
> +
> +test="mktemp"
> +
> +mkdir test_dir
> +if [ "$(busybox mktemp test_dir/test_file.XXXXXX | grep "test_dir\/test_file\.[a-zA-Z0-9]\{6\}")" ]
> +then
> +    echo " -> $test: mktemp command succeeded."
> +else
> +    echo " -> $test: mktemp command failed."
> +    echo " -> $test: TEST-FAIL"
> +    rm -rf test_dir;
> +    exit
> +fi;
> +
> +if ls -l test_dir | grep "\-rw[\-]\{7\}.*test_file\.[a-zA-Z0-9]\{6\}"
> +then
> +    echo " -> $test: TEST-PASS"
> +else
> +    echo " -> $test: TEST-FAIL"
> +fi;
> +rm -rf test_dir;
> diff --git a/engine/tests/Functional.busybox/tests/busybox_more.sh b/engine/tests/Functional.busybox/tests/busybox_more.sh
> new file mode 100644
> index 0000000..d9d06cf
> --- /dev/null
> +++ b/engine/tests/Functional.busybox/tests/busybox_more.sh
> @@ -0,0 +1,16 @@
> +#!/bin/sh
> +
> +#  The testscript checks the following options of the command more
> +#  1) Option none
> +
> +test="more"
> +
> +mkdir test_dir
> +echo "This is a file" > test_dir/test1
> +if [ "$(busybox more ./test_dir/test1)" = "This is a file" ]
> +then
> +    echo " -> $test: TEST-PASS"
> +else
> +    echo " -> $test: TEST-FAIL"
> +fi;
> +rm -rf ./test_dir;
> diff --git a/engine/tests/Functional.busybox/tests/busybox_mount.sh b/engine/tests/Functional.busybox/tests/busybox_mount.sh
> new file mode 100644
> index 0000000..247c894
> --- /dev/null
> +++ b/engine/tests/Functional.busybox/tests/busybox_mount.sh
> @@ -0,0 +1,13 @@
> +#!/bin/sh
> +
> +#  The testscript checks the following options of the command mount
> +#  1) Option none
> +
> +test="mount"
> +
> +if busybox mount
> +then
> +    echo " -> $test: TEST-PASS"
> +else
> +    echo " -> $test: TEST-FAIL"
> +fi;

Not much test of mount functionality here.  Maybe should at least
check for output of mount information
for '/' (which should be present on all systems).

e.g. if busybox mount | grep "/ "


> diff --git a/engine/tests/Functional.busybox/tests/busybox_mv.sh b/engine/tests/Functional.busybox/tests/busybox_mv.sh
> new file mode 100644
> index 0000000..3cff164
> --- /dev/null
> +++ b/engine/tests/Functional.busybox/tests/busybox_mv.sh
> @@ -0,0 +1,27 @@
> +#!/bin/sh
> +
> +#  The testscript checks the following options of the command mv
> +#  1) Option none
> +
> +test="mv"
> +
> +mkdir test_dir
> +touch test_dir/test1
> +if [ "$(ls test_dir)" = "test1" ]
> +then
> +    echo " -> $test: ls test_dir succeeded."
> +else
> +    echo " -> $test: ls test_dir failed."
> +    echo " -> $test: TEST-FAIL"
> +    rm -rf test_dir
> +    exit
> +fi
> +
> +busybox mv test_dir/test1 test_dir/test2
> +if [ "$(ls test_dir)" = "test2" ]
> +then
> +    echo " -> $test: TEST-PASS"
> +else
> +    echo " -> $test: TEST-FAIL"
> +fi;
> +rm -rf test_dir
> --
> 1.8.3.1
>


Thanks.  Please check my master branch and see if I've broken
anything for you with my changes.
 -- Tim
Senior Staff Software Engineer, Sony Corporation
Architecture Group Chair, Core Embedded Linux Project, Linux Foundation


More information about the Fuego mailing list