[Fuego] [PATCH] Add a new test suite called Functional.busybox.

Tim.Bird at sony.com Tim.Bird at sony.com
Sat May 19 01:23:05 UTC 2018


Thanks for the patches!  We always appreciate contributions.
Please see my comments inline below.

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

It sounded like the new patch would be a complete replacement of the earlier
patch, but the patch that was attached was just a change of part of the commit.
It did not apply by itself.

In any event, here is some feedback...

> -----Original Message-----
> From: Wang, Mingyu/王 鸣瑜
> Sent: Friday, May 18, 2018 10:39 PM
> To: fuego at lists.linuxfoundation.org
> Cc: Wang, Mingyu/王 鸣瑜 <wangmy at cn.fujitsu.com>
> Subject: [PATCH] Add a new test suite called Functional.busybox.
> 
> 
> 
> Signed-off-by: Wang Mingyu <wangmy at cn.fujitsu.com>
> ---
>  engine/tests/Functional.busybox/fuego_test.sh | 17 +++++++++++++++++
>  engine/tests/Functional.busybox/spec.json     |  7 +++++++
>  2 files changed, 24 insertions(+)
>  create mode 100755 engine/tests/Functional.busybox/fuego_test.sh
>  create mode 100644 engine/tests/Functional.busybox/spec.json
> 
> diff --git a/engine/tests/Functional.busybox/fuego_test.sh
> b/engine/tests/Functional.busybox/fuego_test.sh
> new file mode 100755
> index 0000000..3cc2b85
> --- /dev/null
> +++ b/engine/tests/Functional.busybox/fuego_test.sh
> @@ -0,0 +1,17 @@
> +tarball=busybox.tar.gz
I looked at the contents of the tarball.  This particular tarball only holds
a single shell script.  In this case, it is better to just have it in the $TEST_HOME
directory, and not use a tarball at all.  My recommendation would be to 
add busybox_test.sh to $TESTHOME, and remove this tarball line.

> +
> +
(minor) my preference would be only 1 blank line here.

> +function test_deploy {
> +    put $TEST_HOME/busybox_test.sh $BOARD_TESTDIR/fuego.$TESTDIR/
I'd leave this as is (not change it like your followup patch)

> +    put -r $TEST_HOME/testsuits $BOARD_TESTDIR/fuego.$TESTDIR/ }
> +
> +function test_run {
> +    report "cd $BOARD_TESTDIR/fuego.$TESTDIR; export
> test_tcpip_host=$SRV_IP; export test_port_host=$SRV_PORT; export
> httproot_dir_host=fuego/userContent/readme.txt; sh -v busybox_test.sh"
Some line continuations would be good here to keep this from getting too long.
e.g.
     report "cd $BOARD_TESTDIR/fuego.$TESTDIR; \
        export test_tcpip_host=$SRV_IP ; \
        export test_port_host=$SRV_PORT; \
        export httproot_dir_host=fuego/userContent/readme.txt; \
        sh -v busybox_test.sh"

I know other tests have some pretty long lines, but I'd like for new tests to
be better at having shorter lines.

> +}
> +
> +function test_processing {
> +    log_compare "$TESTDIR" "89" "pass" "p"
Is 89 the right number for the tests you have submitted so far?

> +}
> +
> +
> diff --git a/engine/tests/Functional.busybox/spec.json
> b/engine/tests/Functional.busybox/spec.json
> new file mode 100644
> index 0000000..a71218a
> --- /dev/null
> +++ b/engine/tests/Functional.busybox/spec.json
> @@ -0,0 +1,7 @@
> +{
> +    "testName": "Functional.busybox",
> +    "specs": {
> +        "default": {}
> +    }
> +}
> +
> --
> 1.8.3.1
> 

Looks good.

Did you want me to apply this, and fix it up myself, or do you want to fix it
and send new patches?
 -- Tim




More information about the Fuego mailing list