[Fuego] [PATCH] Add command false, fgrep and find to testset of busybox.

Tim.Bird at sony.com Tim.Bird at sony.com
Wed Jun 27 20:17:26 UTC 2018


Wang,

Sorry for the slow response.  It was a pleasure to meet you face-to-face
at the Fuego Jamboree.  Here is some feedback on this patch, which 
I apologize for taking so long to review.

Feedback inline below...

> -----Original Message-----
> From: Wang Mingyu
> Subject: [Fuego] [PATCH] Add command false, fgrep and find to testset of
> busybox.
> 
> Signed-off-by: Wang Mingyu <wangmy at cn.fujitsu.com>
> ---
>  .../Functional.busybox/tests/busybox_false.sh      | 14 ++++++++++++++
>  .../Functional.busybox/tests/busybox_fgrep.sh      | 22
> ++++++++++++++++++++++
>  .../tests/Functional.busybox/tests/busybox_find.sh | 15 +++++++++++++++
>  3 files changed, 51 insertions(+)
>  create mode 100644
> engine/tests/Functional.busybox/tests/busybox_false.sh
>  create mode 100644
> engine/tests/Functional.busybox/tests/busybox_fgrep.sh
>  create mode 100644
> engine/tests/Functional.busybox/tests/busybox_find.sh
> 
> diff --git a/engine/tests/Functional.busybox/tests/busybox_false.sh
> b/engine/tests/Functional.busybox/tests/busybox_false.sh
> new file mode 100644
> index 0000000..077ab7d
> --- /dev/null
> +++ b/engine/tests/Functional.busybox/tests/busybox_false.sh
> @@ -0,0 +1,14 @@
> +#!/bin/sh
> +
> +#  The testscript checks the following options of the command false
> +#  1) Option none
> +
> +test="false"
> +
> +busybox false
> +if echo $?
I think this ends up just testing that 'echo' worked.

I was expecting something like:
if [ "$?" = "1" ] ; then

But, this is a bit fragile.  $? tends to get overwritten very quickly in
a shell environment.  If someone inserts a statement in-between
the command that is executed, and the use of $?, it results in a wrong
value, and much confusion.

This is why you often see the capture of $? as a statement immediately following
the command, like so:   "some_program ; RESULT=$?"

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

I think this would be better structured as:
if busybox false
then
    echo "-> $test: TEST-FAIL"
else
    echo "-> $test: TEST-PASS"
fi

The whole point of 'false', is that it returns non-true, which can be
tested directly with the if.

> diff --git a/engine/tests/Functional.busybox/tests/busybox_fgrep.sh
> b/engine/tests/Functional.busybox/tests/busybox_fgrep.sh
> new file mode 100644
> index 0000000..3dc48f9
> --- /dev/null
> +++ b/engine/tests/Functional.busybox/tests/busybox_fgrep.sh
> @@ -0,0 +1,22 @@
> +#!/bin/sh
> +
> +#  The testscript checks the following options of the command fgrep
> +#  1) Option: -i
> +
> +test="fgrep"
> +
> +mkdir test_dir
> +echo "This is a test file." > ./test_dir/test1
> +echo "It contains data to test the fgrep function." >> ./test_dir/test1
> +echo "fgrep is the same as grep -F function." >> ./test_dir/test1
> +echo "It interprets the pattern as fixed string." >> ./test_dir/test1
> +echo "In the test we shall look for a fixed pattern." >> ./test_dir/test1
> +
> +busybox fgrep in ./test_dir/test1 > log
> +if [ "$(head -n 1 log)" = "It contains data to test the fgrep function." ] && [
> "$(tail -n 1 log)" = "It interprets the pattern as fixed string." ]
> +then
> +    echo " -> $test: TEST-PASS"
> +else
> +    echo " -> $test: TEST-FAIL"
> +fi;
> +rm -rf test_dir
> diff --git a/engine/tests/Functional.busybox/tests/busybox_find.sh
> b/engine/tests/Functional.busybox/tests/busybox_find.sh
> new file mode 100644
> index 0000000..7883a97
> --- /dev/null
> +++ b/engine/tests/Functional.busybox/tests/busybox_find.sh
> @@ -0,0 +1,15 @@
> +#!/bin/sh
> +
> +#  The testscript checks the following options of the command find
> +#  1) Option: -name
> +
> +test="find"
> +
> +mkdir test_find_dir
> +if [ "$(busybox find . -name test_find_dir)" = "./test_find_dir" ]
> +then
> +    echo " -> $test: TEST-PASS"
> +else
> +    echo " -> $test: TEST-FAIL"
> +fi;
> +rm -rf test_find_dir
> --
> 1.8.3.1


These look OK.

If you want, I can change the patch as I described and commit it.  Or you can re-submit
it if you'd like to make a different change.

Let me know which you prefer.

Thanks very much for the continuing contributions to Fuego.
 -- Tim


More information about the Fuego mailing list