[Fuego] [PATCH] Add test cases for commands of busybox as follows: ash bunzip2 free wget

Sat May 19 01:57:23 UTC 2018

> -----Original Message-----
> From: Wang, Mingyu
>
> I'm very sorry, there is a problem with the previous patch, please ignore it.
> The modified patch has been sent as an attachment. Please confirm it.

The attached patch only contains busybox.tar.gz.  The original message contained
busybox.tar.gz, as well as 4 test scripts.  I thought the new patch (the attachment)
would be a replacement.  In the attachment, the patch is inside a directory
busybox-scripts, which is not referenced in fuego_test.sh.  I think there is something
wrong here.  I would recommend getting rid of busybox.tar.gz, and not putting
busybox_test.sh in a sub-directory.

>  Best regards.
> 
> -----Original Message-----
> From: Wang, Mingyu/王 鸣瑜
> Sent: Friday, May 18, 2018 10:41 PM
> To: fuego at lists.linuxfoundation.org
> Cc: Wang, Mingyu/王 鸣瑜 <wangmy at cn.fujitsu.com>
> Subject: [PATCH] Add test cases for commands of busybox as follows: ash
> bunzip2 free wget
> 
> Signed-off-by: Wang Mingyu <wangmy at cn.fujitsu.com>
> ---
>  engine/tests/Functional.busybox/busybox.tar.gz     | Bin 0 -> 171 bytes
>  .../Functional.busybox/testsuits/busybox_ash.sh    |  15 ++++++++++++++
>  .../testsuits/busybox_bunzip2.sh                   |  12 +++++++++++
>  .../Functional.busybox/testsuits/busybox_free.sh   |  10 ++++++++++
>  .../Functional.busybox/testsuits/busybox_wget.sh   |  22
> +++++++++++++++++++++
>  5 files changed, 59 insertions(+)
>  create mode 100644 engine/tests/Functional.busybox/busybox.tar.gz
>  create mode 100644
> engine/tests/Functional.busybox/testsuits/busybox_ash.sh
>  create mode 100644
> engine/tests/Functional.busybox/testsuits/busybox_bunzip2.sh
>  create mode 100644
> engine/tests/Functional.busybox/testsuits/busybox_free.sh
>  create mode 100644
> engine/tests/Functional.busybox/testsuits/busybox_wget.sh
> 
> diff --git a/engine/tests/Functional.busybox/busybox.tar.gz
> b/engine/tests/Functional.busybox/busybox.tar.gz
> new file mode 100644
> index
> 0000000000000000000000000000000000000000..0bdac1768d26a5f9ec1583e93
> a3632d978808960
> GIT binary patch
> literal 171
> zcmb2|=3qE<@m~}J^V>_-e1{AKS`x3 at 9aRbp6nO3!Qy?Lt?N})6y;1q#Tf-
> @u3%Udz
> z{hy_o)~$I>_v5=mZzr|Q=qn6~Hi*1!vi{7qnck~U?b`G$B=YjLQ&U23PxCsXmK`
> >2
> z#;!=0vx-Z~%jf at +->N-lKG*f5f$@iR;{K?dkMHRA-
> )Qk<x`ns=mXyLNd#~5dU|YN2
> UR+<4B{JY2O#?pI`L4$z-0L$P at YXATM
> 
> literal 0
> HcmV?d00001
> 
> diff --git a/engine/tests/Functional.busybox/testsuits/busybox_ash.sh
> b/engine/tests/Functional.busybox/testsuits/busybox_ash.sh
> new file mode 100644
> index 0000000..47cea2d
> --- /dev/null
> +++ b/engine/tests/Functional.busybox/testsuits/busybox_ash.sh
> @@ -0,0 +1,15 @@
> +#!/bin/bash
> +workdir=$(pwd)
> +test="ash"
> +
> +expect <<-EOF
This test requires the tool 'expect', which a board may not have.

Please add a test_pre_check function to your fuego_test.sh, with a
dependency check for this.

Like so:
function test_pre_check {
    is_on_target_path expect PROGRAM_EXPECT
    assert_define PROGRAM_EXPECT "Missing required program 'expect' on board"
}

> +spawn busybox ash
> +expect "*# "
> +send_user " -> $test: Opened ash shell succeeded.\n"
> +send "busybox pwd\r"
> +expect "$workdir"
> +send_user " -> $test: Executed pwd command ash shell succeeded.\n"
> +send "exit\r"
> +send_user " -> $test: Closing ash shell succeeded.\n pass $test\n"
> +expect eof
> +EOF
> diff --git a/engine/tests/Functional.busybox/testsuits/busybox_bunzip2.sh
> b/engine/tests/Functional.busybox/testsuits/busybox_bunzip2.sh
> new file mode 100644
> index 0000000..0704b9a
> --- /dev/null
> +++ b/engine/tests/Functional.busybox/testsuits/busybox_bunzip2.sh
> @@ -0,0 +1,12 @@
> +#!/bin/bash
> +test="bunzip2"
> +
> +echo "This is a test file">test1
> +bzip2 test1
> +if [ "`busybox bunzip2 -c test1.bz2`" == "This is a test file" ] then
> +    echo "$test: test cases succeeded.\n pass $test"
> +else
> +    echo "$test: test cases failed."
> +fi;
> +rm -rf test1.bz2;
> diff --git a/engine/tests/Functional.busybox/testsuits/busybox_free.sh
> b/engine/tests/Functional.busybox/testsuits/busybox_free.sh
> new file mode 100644
> index 0000000..4b5b192
> --- /dev/null
> +++ b/engine/tests/Functional.busybox/testsuits/busybox_free.sh
> @@ -0,0 +1,10 @@
> +#!/bin/bash
> +test="free"
> +
> +busybox free
> +if [ `echo $?` == 0 ]
> +then
> +    echo "$test: test cases succeeded.\n pass $test"
> +else
> +    echo "$test: test cases failed."
> +fi;
> diff --git a/engine/tests/Functional.busybox/testsuits/busybox_wget.sh
> b/engine/tests/Functional.busybox/testsuits/busybox_wget.sh
> new file mode 100644
> index 0000000..90fa83d
> --- /dev/null
> +++ b/engine/tests/Functional.busybox/testsuits/busybox_wget.sh
> @@ -0,0 +1,22 @@
> +#!/bin/bash
> +test="wget"
> +
> +if [ -f /home/test/readme.txt ]; then rm -f /home/test/readme.txt ; fi
You should not use a hardcoded path for this: /home/test.  It would be
better to use a relative path, based on the current working directory.
Different boards will have different BOARD_TESTDIR directories, where
the test will run.  Not every board will have a /home/test directory.
Or, a board might have a /home/test directory that is unrelated to Fuego,
and we should not disturb it.

> +busybox wget
> http://$test_tcpip_host:$test_port_host/$httproot_dir_host
httproot_dir_host is only used once.  (maybe it is used in other scripts
you haven't submitted yet).  I think using the path here would be better.
(that is, use "fuego/userContent/readme.txt" instead of the variable
httproot_dir_host.

I can't find anywhere that SRV_PORT is set on any of my board, or in
Fuego.  Am I missing something?  Is this a new required board variable?

> +-P /home/test if ls  /home/test/ | grep ".*readme.*"
> +then
> +    echo "$test: ls  /home/test/ executed."
> +else
> +    echo "$test: ls  /home/test/ failed."
> +fi;
> +if cat /home/test/readme.txt | grep ".*jenkins.*"
> +then
> +    echo "$test: test cases succeeded.\n pass $test"
> +else
> +    echo "$test: test cases failed."
> +fi;
> +rm /home/test/readme.txt
> +ls /
What is this final 'ls' for?

> +
> +
> +
Why are there extra lines here?

Overall, pretty good.  I think I would prefer something more unique to recognize
a passing test, than the string "pass".  This could end up matching something
like "password", or "passing", in test output.  (It is used in log_compare in your
fuego_test.sh script).   I would recommend something
that is unlikely to appear in test output - maybe TPASS, or something like that.

In general, I think the output could be improved a bit.  We have started adding
the ability to split the test output into testcase results.  It is helpful if a testcase
identifier is available in the line used to report the result for each testcase.
Many tests use numbers, but my personal preference is to use a descriptive
string, that is unique throughout the test suite.

I can discuss this more if you'd like.  I gave a presentation which included some tips
in this area. You can find a link to it here:
http://fuegotest.org/wiki/Presentations

I think it would be good to review the output you are presenting, modify it a bit,
and write a parser.py for your output, so you could get more detailed results
(per-testcase) from the Jenkins interface.  I can help you with this if you'd like.
 -- Tim