[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