[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