[Fuego] [PATCH] Add new test cases of command that can change permission.

Wang, Mingyu wangmy at cn.fujitsu.com
Fri Jun 8 07:15:56 UTC 2018


Hi Tim:

Thank you very much for pointing out the problems in the scripts for me, and giving me a very detailed explanation. Most of the allegations have already been fixed, you can see them from the latest patch. 

A few points are described as follows:
clear: 
the function of the command is clear the terminal screen, which has no options and output. I didn't think of any other way to test this command.

dd: 
standard error output is not written successfully with 2>&1 >log, so 2>>log is used.

deallocvt: 
Before the execution of the openvt command has ended, the following command deallocvt is executed, which results in deallocvt execution error. Adding sleep 1 to buffer between the two commands can solve this problem. Of course, if there is a better way, please let me know.

by Wangmy

-----Original Message-----
From: Tim.Bird at sony.com [mailto:Tim.Bird at sony.com] 
Sent: Wednesday, June 06, 2018 6:05 AM
To: Wang, Mingyu/王 鸣瑜 <wangmy at cn.fujitsu.com>; fuego at lists.linuxfoundation.org
Subject: RE: [Fuego] [PATCH] Add new test cases of command that can change permission.

Sorry for the slow response.  I was on vacation last week.

Thanks for the tests.  See comments inline below.

> -----Original Message-----
> From: Wang Mingyu
> Signed-off-by: Wang Mingyu <wangmy at cn.fujitsu.com>
> ---
>  .../Functional.busybox/tests/busybox_basename.sh   | 17 ++++++++
>  .../Functional.busybox/tests/busybox_busybox.sh    | 14 +++++++
>  .../Functional.busybox/tests/busybox_bzcat.sh      | 16 +++++++
>  .../tests/Functional.busybox/tests/busybox_cat.sh  | 16 +++++++
>  .../Functional.busybox/tests/busybox_chgrp1.sh     | 25 +++++++++++
>  .../Functional.busybox/tests/busybox_chgrp2.sh     | 31 ++++++++++++++
>  .../Functional.busybox/tests/busybox_chmod1.sh     | 49
> ++++++++++++++++++++++
>  .../Functional.busybox/tests/busybox_chmod2.sh     | 32 ++++++++++++++
>  .../Functional.busybox/tests/busybox_chown1.sh     | 34 +++++++++++++++
>  .../Functional.busybox/tests/busybox_chown2.sh     | 43
> +++++++++++++++++++
>  .../Functional.busybox/tests/busybox_chroot.sh     | 14 +++++++
>  .../tests/Functional.busybox/tests/busybox_chvt.sh | 14 +++++++
>  12 files changed, 305 insertions(+)
>  create mode 100644
> engine/tests/Functional.busybox/tests/busybox_basename.sh
>  create mode 100644
> engine/tests/Functional.busybox/tests/busybox_busybox.sh
>  create mode 100644
> engine/tests/Functional.busybox/tests/busybox_bzcat.sh
>  create mode 100644 
> engine/tests/Functional.busybox/tests/busybox_cat.sh
>  create mode 100644
> engine/tests/Functional.busybox/tests/busybox_chgrp1.sh
>  create mode 100644
> engine/tests/Functional.busybox/tests/busybox_chgrp2.sh
>  create mode 100644
> engine/tests/Functional.busybox/tests/busybox_chmod1.sh
>  create mode 100644
> engine/tests/Functional.busybox/tests/busybox_chmod2.sh
>  create mode 100644
> engine/tests/Functional.busybox/tests/busybox_chown1.sh
>  create mode 100644
> engine/tests/Functional.busybox/tests/busybox_chown2.sh
>  create mode 100644
> engine/tests/Functional.busybox/tests/busybox_chroot.sh
>  create mode 100644
> engine/tests/Functional.busybox/tests/busybox_chvt.sh
> 
> diff --git a/engine/tests/Functional.busybox/tests/busybox_basename.sh
> b/engine/tests/Functional.busybox/tests/busybox_basename.sh
> new file mode 100644
> index 0000000..6fdab06
> --- /dev/null
> +++ b/engine/tests/Functional.busybox/tests/busybox_basename.sh
> @@ -0,0 +1,17 @@
> +#!/bin/sh
> +
> +#  The testscript checks the following options of the command 
> +basename #  1) Option none
> +
> +workdir=$(pwd)
> +test="basename"
> +
> +busybox mkdir -p $workdir/test_dir
> +busybox echo "This is a test file." > ./test_dir/test1

Please remove this creation of directory and file.  The basename function does not require these.  'basename' is purely a string operation.

> +if [ $(busybox basename ./test_dir/test1) = "test1" ] then
> +    echo " -> $test: TEST-PASS"
> +else
> +    echo " -> $test: TEST-FAIL"
> +fi;
> +busybox rm -rf test_dir;
Also eliminate this cleanup.

> diff --git a/engine/tests/Functional.busybox/tests/busybox_busybox.sh
> b/engine/tests/Functional.busybox/tests/busybox_busybox.sh
> new file mode 100644
> index 0000000..22545b3
> --- /dev/null
> +++ b/engine/tests/Functional.busybox/tests/busybox_busybox.sh
> @@ -0,0 +1,14 @@
> +#!/bin/sh
> +
> +#  The testscript checks the following options of the command busybox 
> +#  1) Option none
> +
> +workdir=$(pwd)
> +test="busybox"
> +
> +if [ $(busybox pwd) = ${workdir} ]
I'm not sure I understand this test.  If busybox provides the 'pwd'
command on the system, this just tests that it returns the same thing as itself.

Since this test is named 'busybox', I thought it would test something very basic and intrinsic about busybox - maybe the presence of busybox, or "busybox" with no args?

I suppose "busybox pwd" is one of the more basic commands, but I would assume this would be in a test named "busybox_pwd.sh".

I think I'd rather see the test do something like this:

if busybox | grep ^BusyBox ;

> +then
> +    echo " -> $test: TEST-PASS"
> +else
> +    echo " -> $test: TEST-FAIL"
> +fi;
> diff --git a/engine/tests/Functional.busybox/tests/busybox_bzcat.sh
> b/engine/tests/Functional.busybox/tests/busybox_bzcat.sh
> new file mode 100644
> index 0000000..93ce7f0
> --- /dev/null
> +++ b/engine/tests/Functional.busybox/tests/busybox_bzcat.sh
> @@ -0,0 +1,16 @@
> +#!/bin/sh
> +
> +#  The testscript checks the following options of the command bzcat #  
> +1) Option none
> +
> +test="bzcat"
> +
> +echo "This is a test file">test1
> +bzip2 test1
> +if [ "$(busybox bzcat test1.bz2)" == "This is a test file" ]
Please use single '=' here, for backwards POSIX compatibility.

> +then
> +    echo " -> $test: TEST-PASS"
> +else
> +    echo " -> $test: TEST-FAIL"
> +fi;
> +rm -rf test1.bz2;
> diff --git a/engine/tests/Functional.busybox/tests/busybox_cat.sh
> b/engine/tests/Functional.busybox/tests/busybox_cat.sh
> new file mode 100644
> index 0000000..ee09a8e
> --- /dev/null
> +++ b/engine/tests/Functional.busybox/tests/busybox_cat.sh
> @@ -0,0 +1,16 @@
> +#!/bin/sh
> +
> +#  The testscript checks the following options of the command cat #  
> +1) Option none
> +
> +test="cat"
> +
> +busybox echo "hello" > test1
> +busybox echo "world" > test2
I'd prefer if commands other than the one under test, just used the system default version.  That is, this is a test of busyboxes version of 'cat', so that command should be 'busybox cat'.  But for things like 'echo', 'sed', 'head', 'tail', 'tr', 'cut', 'ls',  and 'touch' these should not be prefixed with 'busybox'.

In other words, please just use 'echo ...' here, instead of 'busybox echo...'

> +if [ "$(busybox cat test1 test2 | sed -n '1p')" = "hello" ] && [ 
> +"$(busybox cat
> test1 test2 | sed -n '2p')" = "world" ]

'sed' is not guaranteed to be on the system.  But Fuego requires 'head' and 'tail'.
So please use those instead, to access the lines in the file.

> +then
> +    echo " -> $test: TEST-PASS"
> +else
> +    echo " -> $test: TEST-FAIL"
> +fi;
> +busybox rm -rf test1 test2;
> diff --git a/engine/tests/Functional.busybox/tests/busybox_chgrp1.sh
> b/engine/tests/Functional.busybox/tests/busybox_chgrp1.sh
> new file mode 100644
> index 0000000..7a72fac
> --- /dev/null
> +++ b/engine/tests/Functional.busybox/tests/busybox_chgrp1.sh
> @@ -0,0 +1,25 @@
> +#!/bin/sh
> +
> +#  The testscript checks the following options of the command chgrp #  
> +1) Option: none
> +
> +test="chgrp1"
> +
> +busybox mkdir test_dir
> +busybox touch test_dir/test1
> +if [ "$(busybox ls -l ./test_dir | busybox tr -s ' ' | busybox cut 
> +-d' ' -f4,9)" =
> "root test1" ]

It's not guaranteed that the test will be running as group 'root'.
Also, the group name might be longer than 8 characters.  On my system, 'busybox ls -l' truncates the group name in the output to 8 chars, while regular (non-busybox) 'ls -l' does not truncate the group name.  (This seems like a pretty serious difference in behavior that I've never noticed before, but this is unrelated to this test - except that you're explicitly using 'busybox ls -l', which means you need to pay attention to the possibility of that command truncating the group name.

Also, the test is using busybox ls, busybox_touch, busybox tr, and busybox cut.
Busybox could conceivably be configured without one or more of these sub-commands.

Instead of using 'busybox touch', please use 'touch'.  Instead of using 'busybox cut', please use 'cut'.  Please do this throughout these tests.

Note that Fuego requires that the DUT have 'touch', so that one you don't
need to check for.   At least one other test uses 'cut', so that one's
probably OK also. Fuego also requires that the DUT have 'id'.

I'd replace your test line with something like the following:

group=$(id -n -g | cut -b -8)
if [ "$(busybox ls -l ./test_dir | tr -s ' ' | cut -d' ' -f4 | cut -b -8)" = "$group" ]

If you could eliminate the use of 'tr' that would be good, but I couldn't figure out any other command that was guaranteed to be on the system and accomplished similar functionality.

If you end up using it, you might want to add 'tr' to the list of things you check for in the test_pre_check (using skip_if_command_unavailable).

> +then
> +    echo " -> $test: Group info display succedded."
> +else
> +    echo " -> $test: FAIL"
> +    busybox rm -rf test_dir;
> +    exit
> +fi;
> +
> +if [ "$(busybox chgrp bin ./test_dir/test1; busybox ls -l ./test_dir 
> +| busybox
> tr -s ' ' | busybox cut -d' ' -f4,9)" = "bin test1" ]
> +then
> +    echo " -> $test: TEST-PASS"
> +else
> +    echo " -> $test: TEST-FAIL"
> +fi;
> +busybox rm -rf test_dir;
> diff --git a/engine/tests/Functional.busybox/tests/busybox_chgrp2.sh
> b/engine/tests/Functional.busybox/tests/busybox_chgrp2.sh
> new file mode 100644
> index 0000000..4b27e76
> --- /dev/null
> +++ b/engine/tests/Functional.busybox/tests/busybox_chgrp2.sh
> @@ -0,0 +1,31 @@
> +#!/bin/sh
> +
> +#  The testscript checks the following options of the command chgrp #  
> +1) Option: -R
> +
> +test="chgrp2"
> +
> +busybox mkdir test_dir
> +busybox touch test_dir/test1
> +busybox touch test_dir/test2
> +busybox ls -l ./test_dir | busybox tr -s ' ' | busybox cut -d' ' 
> +-f4,9 > log1 if [ "$(sed -n '1p' log1)" = "root test1" ] && [  "$(sed 
> +-n '2p' log1)" = "root
> test2" ]

You should check for 'sed' on the system, before using it.
It looks like these are just being used for 'head' and 'tail' operations.
Fuego requires these on a DUT, so please replace these with:
$(head -n 1 log1)
and $(tail -n 1 log1)
respectively.

Also, make sure that the test correctly handles non-root group name, that might be potentially longer than 8 chars.

> +then
> +    echo " -> $test: Group info display succedded."
> +else
> +    echo " -> $test: TEST-FAIL"
> +    busybox rm log1
> +    busybox rm -rf test_dir
> +    exit
> +fi;
> +
> +busybox chgrp -R bin ./test_dir
> +busybox ls -l ./test_dir | busybox tr -s ' ' | busybox cut -d' ' 
> +-f4,9 > log2 if [ "$(sed -n '1p' log2)" = "bin test1" ] && [ "$(sed -n '2p' log2)" = "bin test2"
> ]
Same issues here.

> +then
> +    echo " -> $test: TEST-PASS"
> +else
> +    echo " -> $test: TEST-FAIL"
> +fi;
> +busybox rm log1 log2;
> +busybox rm -rf test_dir;
> diff --git a/engine/tests/Functional.busybox/tests/busybox_chmod1.sh
> b/engine/tests/Functional.busybox/tests/busybox_chmod1.sh
> new file mode 100644
> index 0000000..17f22e6
> --- /dev/null
> +++ b/engine/tests/Functional.busybox/tests/busybox_chmod1.sh
> @@ -0,0 +1,49 @@
> +#!/bin/sh
> +
> +#  The testscript checks the following options of the command chmod #  
> +1) Option: g+x #  2) Option: o+x #  3) Option: a-x
> +
> +test="chmod1"
> +
> +busybox mkdir test_dir
> +busybox touch test_dir/test1
> +
> +if [[ $(busybox ls -l ./test_dir | busybox tr -s ' ' | busybox cut 
> +-d' ' -f1,9) =
> *test1 ]]
[[ with a wildcard is a bash-ism, that may not be supported on the target.

Can you please replace this with more specific 'cut'ing?

> +then
> +    echo " -> $test: test_dir contents verification succeeded."
> +else
> +    echo " -> $test: TEST-FAIL"
> +    busybox rm -rf ./test_dir
> +    exit
> +fi;
> +
> +busybox chmod g+x ./test_dir/test1
> +if [[ $(busybox ls -l ./test_dir|busybox tr -s ' '|busybox cut -d' ' 
> +-f1) = -rw* ]]
[[ with a wildcard is a bash-ism, that may not be supported on the target.

Can you replace this with a cut command, to isolate just the parts you want to test?
e.g.
if [ $(busybox ls -l ./test_dir| cut -b 4-6 ) = "-rw" ]

> +then
> +    echo " -> $test: Changed file permissions verification2 succeeded."
> +else
> +    echo " -> $test: TEST-FAIL"
> +    busybox rm -rf ./test_dir
> +    exit
> +fi;
> +
> +busybox chmod o+x ./test_dir/test1
> +if [[ $(busybox ls -l ./test_dir|busybox tr -s ' '|busybox cut -d' ' 
> +-f1,9) = -
> *x*test1 ]]
Please remove the [[ bashism, and use more intricate cuts, instead of wilcard matching here.

> +then
> +    echo " -> $test: Changed file permissions verification3 succeeded."
> +else
> +    echo " -> $test: TEST-FAIL"
> +    busybox rm -rf ./test_dir
> +    exit
> +fi;
> +
> +busybox chmod a-x ./test_dir/test1
> +if [[ $(busybox ls -l ./test_dir|busybox tr -s ' '|busybox cut -d' ' 
> +-f1,9) = -**-
> **-**-*test1 ]]
Same thing here.

> +then
> +    echo " -> $test: TEST-PASS"
> +else
> +    echo " -> $test: TEST-FAIL"
> +fi;
> +busybox rm -rf ./test_dir;
> diff --git a/engine/tests/Functional.busybox/tests/busybox_chmod2.sh
> b/engine/tests/Functional.busybox/tests/busybox_chmod2.sh
> new file mode 100644
> index 0000000..d82452f
> --- /dev/null
> +++ b/engine/tests/Functional.busybox/tests/busybox_chmod2.sh
> @@ -0,0 +1,32 @@
> +#!/bin/sh
> +
> +#  The testscript checks the following options of the command chmod #  
> +1) Option: -R
> +
> +test="chmod2"
> +
> +busybox mkdir test_dir
> +busybox touch test_dir/test1
> +busybox touch test_dir/test2
> +busybox ls -l ./test_dir | busybox tr -s ' ' | busybox cut -d' ' 
> +-f1,9 > log1
> +
> +if [[ $(sed -n '1p' log1) = *test1 ]] && [[ $(sed -n '2p' log1) = 
> +*test2 ]]

Remove [[ bashism, and use head and tail instead of sed.

> +then
> +    echo " -> $test: test_dir contents verification succeeded."
> +else
> +    echo " -> $test: TEST-FAIL"
> +    busybox rm -rf ./test_dir;
> +    busybox rm log1;
> +    exit
> +fi;
> +
> +busybox chmod -R a-r ./test_dir
> +busybox ls -l ./test_dir | busybox tr -s ' ' | busybox cut -d' ' 
> +-f1,9 > log2 if [[ $(sed -n '1p' log2) = --**-**-*test1 ]] && [[ 
> +$(sed -n '2p' log2) = --**-**-
> *test2 ]]
Remove [[ bashism, and use head and tail instead of sed.

> +then
> +    echo " -> $test: TEST-PASS"
> +else
> +    echo " -> $test: TEST-FAIL"
> +fi;
> +busybox rm log1 log2;
> +busybox rm -rf ./test_dir;
> diff --git a/engine/tests/Functional.busybox/tests/busybox_chown1.sh
> b/engine/tests/Functional.busybox/tests/busybox_chown1.sh
> new file mode 100644
> index 0000000..6adc0f1
> --- /dev/null
> +++ b/engine/tests/Functional.busybox/tests/busybox_chown1.sh
> @@ -0,0 +1,34 @@
> +#!/bin/sh
> +
> +#  The testscript checks the following options of the command chown #  
> +1) Option none
> +
> +test="chown1"
> +
> +busybox mkdir test_dir
> +busybox touch test_dir/test1
> +if [[ "$(busybox ls -l ./test_dir | busybox tr -s ' ' | busybox cut -d' ' -f1,3,4,9)"
> =~ " root root test1" ]]
Remove [[ bashism, and don't assume user and group names  here.

> +then
> +    echo " -> $test: test_dir contents verification succeeded."
> +else
> +    echo " -> $test: TEST-FAIL"
> +    busybox rm -rf ./test_dir;
> +    exit
> +fi;
> +
> +if [[ "$(busybox chown bin ./test_dir/test1; busybox ls -l ./test_dir 
> +|
> busybox tr -s ' ' | busybox cut -d' ' -f1,3,4,9)" =~ " bin root test1" 
> ]]

Remove [[ bashism, and don't assume group name  here.

> +then
> +    echo " -> $test: test_dir contents verification succeeded."
> +else
> +    echo " -> $test: TEST-FAIL"
> +    busybox rm -rf ./test_dir;
> +    exit
> +fi;
> +
> +if [[ "$(busybox chown bin.bin ./test_dir/test1; busybox ls -l 
> +./test_dir |
> busybox tr -s ' ' | busybox cut -d' ' -f1,3,4,9)" =~ " bin bin test1" 
> ]]
Please remove use of [[ here.

> +then
> +    echo " -> $test: TEST-PASS"
> +else
> +    echo " -> $test: TEST-FAIL"
> +fi;
> +busybox rm -rf ./test_dir;
> diff --git a/engine/tests/Functional.busybox/tests/busybox_chown2.sh
> b/engine/tests/Functional.busybox/tests/busybox_chown2.sh
> new file mode 100644
> index 0000000..25e241b
> --- /dev/null
> +++ b/engine/tests/Functional.busybox/tests/busybox_chown2.sh
> @@ -0,0 +1,43 @@
> +#!/bin/sh
> +
> +#  The testscript checks the following options of the command chown #  
> +1) Option: -R
> +
> +test="chown2"
> +
> +busybox mkdir test_dir
> +busybox touch test_dir/test1
> +busybox touch test_dir/test2
> +busybox ls -l ./test_dir | busybox tr -s ' ' | busybox cut -d' ' 
> +-f1,3,4,9 > log1 if [[ "$(sed -n '1p' log1)" =~ " root root test1" ]] 
> +&& [[ "$(sed -n '2p' log1)" =~
> " root root test2" ]]
Please remove use of [[ here, replace 'sed' with head and tail, and don't assume root user and group starting names.

> +then
> +    echo " -> $test: test_dir contents verification succeeded."
> +else
> +    echo " -> $test: TEST-FAIL"
> +    busybox rm log1
> +    busybox rm -rf ./test_dir
> +    exit
> +fi;
> +
> +busybox chown -R bin ./test_dir
> +busybox ls -l ./test_dir | busybox tr -s ' ' | busybox cut -d' ' 
> +-f1,3,4,9 > log2 if [[ "$(sed -n '1p' log2)" =~ " bin root test1" ]] && [[ "$(sed -n '2p' log2)" =~ "
> bin root test2" ]]
Same issues.

> +then
> +    echo " -> $test: test_dir contents verification succeeded."
> +else
> +    echo " -> $test: TEST-FAIL"
> +    busybox rm log1 log2
> +    busybox rm -rf ./test_dir
> +    exit
> +fi;
> +
> +busybox chown -R bin.bin ./test_dir
> +busybox ls -l ./test_dir | busybox tr -s ' ' | busybox cut -d' ' 
> +-f1,3,4,9 > log3 if [[ "$(sed -n '1p' log3)" =~ " bin bin test1" ]] && [[ "$(sed -n '2p' log3)" =~ "
> bin bin test2" ]]
Same issues.

> +then
> +    echo " -> $test: TEST-PASS"
> +else
> +    echo " -> $test: TEST-FAIL"
> +fi;
> +busybox rm log1 log2 log3;
> +busybox rm -rf ./test_dir;
> diff --git a/engine/tests/Functional.busybox/tests/busybox_chroot.sh
> b/engine/tests/Functional.busybox/tests/busybox_chroot.sh
> new file mode 100644
> index 0000000..60fb461
> --- /dev/null
> +++ b/engine/tests/Functional.busybox/tests/busybox_chroot.sh
> @@ -0,0 +1,14 @@
> +#!/bin/sh
> +
> +#  The testscript checks the following options of the command chroot 
> +#  1) Option none
> +
> +test="chroot"
> +
> +busybox chroot / /bin/true
This command tests that the command executed, but doesn't really test any chroot functionality.

It would be better to test some operation that left behind a result in the chrooted environment, like so:
mkdir test_dir
cp /bin/touch test_dir
busybox chroot test_dir /touch foo
ls test_dir/foo

However, when I attempted this, it failed, since  'touch' inside the chroot could not load it's shared libraries.  It might be required to build a static version of touch, or some other program that could leave a trace behind, to get this approach to work.

Let me know what you think.

> +if [ $? = 0 ]
> +then
> +    echo " -> $test: TEST-PASS"
> +else
> +    echo " -> $test: TEST-FAIL"
> +fi;
> diff --git a/engine/tests/Functional.busybox/tests/busybox_chvt.sh
> b/engine/tests/Functional.busybox/tests/busybox_chvt.sh
> new file mode 100644
> index 0000000..a3fdb08
> --- /dev/null
> +++ b/engine/tests/Functional.busybox/tests/busybox_chvt.sh
> @@ -0,0 +1,14 @@
> +#!/bin/sh
> +
> +#  The testscript checks the following options of the command chvt #  
> +1) Option none
> +
> +test="chvt"
> +
> +busybox chvt 10
Is this guaranteed to work on all systems?

Do most embedded devices have 10 vts configured?
If the test starts not on vt 10, does it need to change back?

> +if [ $? = 0 ]
> +then
> +    echo " -> $test: TEST-PASS"
> +else
> +    echo " -> $test: TEST-FAIL"
> +fi;
> --
> 1.8.3.1

Thanks for the submission.  Please address or respond to the comments and re-submit.
 -- Tim







More information about the Fuego mailing list