[Fuego] [PATCH 3/6] commonAPI_Dbus: using common script to generate dbus test program

Bird, Timothy Tim.Bird at sony.com
Tue Nov 7 23:54:44 UTC 2017


See comments inline below.

> -----Original Message-----
> From: Liu Wenlong on Monday, November 06, 2017 7:43 AM
>
> Signed-off-by: Liu Wenlong <liuwl.fnst at cn.fujitsu.com>
> ---
>  .../commonAPI_Dbus.tar.gz                          | Bin 6320 -> 0 bytes
>  .../tests/Functional.commonAPI_Dbus/fuego_test.sh  |  34
> +++++++++++++--------
>  2 files changed, 22 insertions(+), 12 deletions(-)
>  delete mode 100755
> engine/tests/Functional.commonAPI_Dbus/commonAPI_Dbus.tar.gz
> 
> diff --git
> a/engine/tests/Functional.commonAPI_Dbus/commonAPI_Dbus.tar.gz
> b/engine/tests/Functional.commonAPI_Dbus/commonAPI_Dbus.tar.gz
> deleted file mode 100755
> index
> 7f1b491803d44b4fb25b16ea50294e9bd1a4b6b7..0000000000000000000000000
> 000000000000000
> GIT binary patch
> literal 0
> HcmV?d00001
> 
> literal 6320
> zcmV;h7*FRPiwFQyY=l?<1MNNiQ{y<Y^DFr)wAm^y at R)o<?gE=v49xCQ#mrE
> ^-0r?N
  [binary stuff omitted]
> mNJT1Ak&0BLA{D7fMJiH}id3W`6{$#nzVvT4*9}Ji$N&I8dw_fZ
> 
> diff --git a/engine/tests/Functional.commonAPI_Dbus/fuego_test.sh
> b/engine/tests/Functional.commonAPI_Dbus/fuego_test.sh
> index 485d279..97ae6c9 100755
> --- a/engine/tests/Functional.commonAPI_Dbus/fuego_test.sh
> +++ b/engine/tests/Functional.commonAPI_Dbus/fuego_test.sh
> @@ -1,25 +1,35 @@
> -tarball=commonAPI_Dbus.tar.gz
> +#!/bin/bash

I don't think we should use this bash invocation line.
I see that some scripts have this bash header line, but I don't
think, in general, that this makes sense. It looks like I've even
added this line into some of the tests that I wrote.  I probably
copy and pasted this without thinking about it.   However, now
that I think about it I would like to move away from using it.

Although fuego_test.sh looks like a regular shell script, it really
is not one. fuego_test.sh relies on a lot of external environment setup,
and only includes certain explicitly supported variables
and shell functions.  It is not intended to be executed as a standalone
script, but always be source'ed by main.sh (or loaded and processed
by ftc or the overlay generator).

I think we should instead move the other way, and remove
#!/bin/bash from all fuego_test.sh scripts.
(In fact I just did so, with a commit to remove this from all remaining
scripts where it is used.)

It does no harm, since it is just a shell comment in our usage of 
fuego_test.sh, but I believe it's confusing.  So I think it's better
to get rid of it.

> 
> -function test_build {
> -    $CXX -std=gnu++11 HelloWorldService.cpp HelloWorldStubImpl.cpp
> HelloWorldDBusStubAdapter.cpp HelloWorldStubDefault.cpp
> HelloWorldDBusDeployment.cpp -rdynamic -lCommonAPI -lCommonAPI-
> DBus -ldbus-1 -I $SDKTARGETSYSROOT/usr/include/CommonAPI-3.1/ -I
> $SDKTARGETSYSROOT/usr/include/dbus-1.0/ -I
> $SDKTARGETSYSROOT/usr/lib/dbus-1.0/include/ -o HelloWorldService
> -    $CXX -std=gnu++11 HelloWorldClient.cpp HelloWorldDBusProxy.cpp
> HelloWorldDBusDeployment.cpp -rdynamic -lCommonAPI -lCommonAPI-
> DBus -ldbus-1 -I $SDKTARGETSYSROOT/usr/include/CommonAPI-3.1/ -I
> $SDKTARGETSYSROOT/usr/include/dbus-1.0/ -I
> $SDKTARGETSYSROOT/usr/lib/dbus-1.0/include/ -o HelloWorldClient
> +
> +tarball=../commonAPI/commonapi.tar.gz
> +source $FUEGO_CORE/engine/tests/commonAPI/common_api.sh

OK - this is something that I think need discussing and evaluation.
There are two issues:
 1) using source in common with another test
 2) using test functions in common with other tests

I know this is how netperf and OpenSSL handle this, but I'm not
sure I like this method and want to proliferate it into more tests.
We might end up just proceeding with this method of sharing items
between tests, but I'd like to discuss it first before moving ahead with
this patch.

The shared portion of this solution is in a patch which is off-list,
which makes it hard to discuss, but I'm going to try anyway.

Sorry for the long train of thought.  I'm kind of rambling through
my thought process here...

One reason I find this problematical is that it makes it hard to
package a test.  For most other tests, all the materials for the test
reside in the test's "home" directory.  And so a test can be packaged
up and sent to another user somewhat easily.  This puts material
needed for the test outside the test's home directory.  I don't mind
putting source somewhere else, but when we share source, we should
probably come up with a more formal mechanism for doing that
(e.g. integrated with a source cache, or something like that, that
could also be used to provide offline access to gitrepos).

---
The intent is to share the source and the test_build function between
the tests: Functional.commonAPI_C++, Functional.commonAPI_Dbus,
and Functional.commonAPI_SomeIp.

The proposed solution does in fact do this, but does not end up sharing
a build directory (and neither do the netperf and OpenSSL versions of
this solution either).  I think we may want to have other cases, like
sub-tests of LTP (posix, realtime, general, stress, etc.).  We currently
put all these under a single Functional.LTP Fuego test, with multiple specs.
In the future, if we have a good method of sharing source and 
build directories, it might be good to split these out into
different Fuego tests.

Also, the setup for the build phase of this test is quite complicated, 
and there are special cases inside the test_build function in
common_api.sh (in the off-list patch).  The special cases
essentially pull in whole other build scripts, for the dbus and someip
cases, which makes me wonder why they should be in common.


> +
> +function pre_check {
> +    is_on_target libCommonAPI.so COMMON_API /lib:/usr/lib:/usr/local/lib
> +    assert_define COMMON_API
> +    is_on_target libCommonAPI-DBus.so COMMON_API_DBUS
> /lib:/usr/lib:/usr/local/lib
> +    assert_define COMMON_API_DBUS
>  }
This looks OK.

> 
> +
>  function test_deploy {
> -	put HelloWorldService $BOARD_TESTDIR/fuego.$TESTDIR/
> -	put HelloWorldClient $BOARD_TESTDIR/fuego.$TESTDIR/
> +    put build/project/commonAPI_Dbus/HelloWorldService
> $BOARD_TESTDIR/fuego.$TESTDIR/
> +    put build/project/commonAPI_Dbus/HelloWorldClient
> $BOARD_TESTDIR/fuego.$TESTDIR/
> +    put `find build -name 'libdbus-1.so*'` $BOARD_TESTDIR/fuego.$TESTDIR/
>  }
Using a 'find' to create the arguments to the put function call is a bit different.
Is the location of libdbus-1.so not known?  Does it depend on the Fuego 
PLATFORM?

> 
>  function test_run {
> -	report "cd $BOARD_TESTDIR/fuego.$TESTDIR; export
> PATH=$BOARD_TESTDIR/fuego.$TESTDIR:$PATH; chmod 777 *;
> -	if (./HelloWorldService &); then echo 'TEST-1 OK'; else echo 'TEST-1
> FAILED'; fi;\
> -	if ./HelloWorldClient; then echo 'TEST-2 OK'; else echo 'TEST-2
> FAILED'; fi;
> -	pkill HelloWorldServi"
> +    report "cd $BOARD_TESTDIR/fuego.$TESTDIR; \
> +        export PATH=$BOARD_TESTDIR/fuego.$TESTDIR:\$PATH; \
> +        export LD_LIBRARY_PATH=\`pwd\`:\$LD_LIBRARY_PATH; chmod 777 *;\
> +        if (./HelloWorldService &); then echo 'TEST-1 OK'; else echo 'TEST-1
> FAILED'; fi;\
> +            if ./HelloWorldClient; then echo 'TEST-2 OK'; else echo 'TEST-2 FAILED';
> fi; \
> +                pkill HelloWorldServi"
This looks OK.
>  }
> 
>  function test_processing {
> -	log_compare "$TESTDIR" "2" "^TEST.*OK" "p"
> -    	log_compare "$TESTDIR" "0" "^TEST.*FAILED" "n"
> +    log_compare "$TESTDIR" "2" "^TEST.*OK" "p"
> +    log_compare "$TESTDIR" "0" "^TEST.*FAILED" "n"
The second log_compare is not needed.  The test only
has two testcases.  If we get 2 OKs, then we know there
are no failures.

Overall - some items are OK, but the general approach is quite complicated.
I'd like to do some testing, but I'm hampered by the fact that I'm missing the shared
libs on my target.  I think I should probably install AGL so I can test this properly.
Let me continue looking at it (but please respond to my questions above).

Thanks,
 -- Tim



More information about the Fuego mailing list