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

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

[Wang Mingyu]

Signed-off-by: Wang Mingyu <wangmy@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 $?
+then
+    echo " -> $test: TEST-PASS"
+else
+    echo " -> $test: TEST-FAIL"
+fi;
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

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

[Tim.Bird]

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@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

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

[Wang, Mingyu]

Hi Tim:

Thank you very much for the advice. I will also reflect these comments on future contributions. As for the modification of these test sets, I think your opinion is no problem, you can modify it directly.

by Wangmy

-----Original Message-----
From: Tim.Bird@sony.com [mailto:Tim.Bird@sony.com] 
Sent: Thursday, June 28, 2018 4:17 AM
To: Wang, Mingyu/王 鸣瑜 <wangmy@cn.fujitsu.com>; fuego@lists.linuxfoundation.org
Subject: RE: [Fuego] [PATCH] Add command false, fgrep and find to testset of busybox.

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@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

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

[Tim.Bird]

> -----Original Message-----
> From: Wang, Mingyu 
> 
> Hi Tim:
> 
> Thank you very much for the advice. I will also reflect these comments on
> future contributions. As for the modification of these test sets, I think your
> opinion is no problem, you can modify it directly.

The patch has been applied and modified as discussed.

Thanks!
 -- Tim

> -----Original Message-----
> From: Tim.Bird@sony.com [mailto:Tim.Bird@sony.com]
> Sent: Thursday, June 28, 2018 4:17 AM
> To: Wang, Mingyu/王 鸣瑜 <wangmy@cn.fujitsu.com>;
> fuego@lists.linuxfoundation.org
> Subject: RE: [Fuego] [PATCH] Add command false, fgrep and find to testset
> of busybox.
> 
> 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@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
> 
> 
>