RE: [PATCH weston] tests: fix a race condition in ivi-shell tests

2018-02-14 Thread Ucan, Emre (ADITG/ESB)
Hi Pekka,


> -Original Message-
> From: wayland-devel [mailto:wayland-devel-
> boun...@lists.freedesktop.org] On Behalf Of Pekka Paalanen
> Sent: Mittwoch, 14. Februar 2018 12:22
> To: Ucan, Emre (ADITG/ESB)
> Cc: wayland-devel@lists.freedesktop.org
> Subject: Re: [PATCH weston] tests: fix a race condition in ivi-shell tests
> 
> On Wed, 14 Feb 2018 11:06:54 +0100
> Emre Ucan  wrote:
> 
> > ivi-shell tests load their own controller plugin
> > for testing purposes. Tests also uses the generated
> > weston-ivi.in config file, which causes weston to
> > load hmi-controller and its helper client.
> > Existence of hmi-controller and its helper client
> > confuses test plugins. Because they are creating
> > surfaces and layers which are not expected by
> > test plugins.
> >
> > We can start ivi-shell tests without config file
> > to solve this problem. Then, weston will not load
> > hmi-controller plugin.
> >
> > Reported-by: Pekka Paalanen 
> > Signed-off-by: Emre Ucan 
> > ---
> >  Makefile.am| 16 +---
> >  tests/weston-tests-env |  4 ++--
> >  2 files changed, 3 insertions(+), 17 deletions(-)
> >
> > diff --git a/Makefile.am b/Makefile.am
> > index 32c9a0f..c534c56 100644
> > --- a/Makefile.am
> > +++ b/Makefile.am
> > @@ -30,15 +30,6 @@ ivi-shell/weston.ini : $(srcdir)/ivi-shell/weston.ini.in
> > -e 's|@plugin_prefix[@]||g' \
> > $< > $@
> >
> > -tests/weston-ivi.ini : $(srcdir)/ivi-shell/weston.ini.in
> > -   $(AM_V_GEN)$(MKDIR_P) $(dir $@) && $(SED) \
> > -   -e 's|@bindir[@]|$(bindir)|g' \
> > -   -e 's|@abs_top_builddir[@]|$(abs_top_builddir)|g' \
> > -   -e 's|@abs_top_srcdir[@]|$(abs_top_srcdir)|g' \
> > -   -e 's|@libexecdir[@]|$(abs_builddir)|g' \
> > -   -e 's|@plugin_prefix[@]|$(abs_top_builddir)/.libs/|g' \
> > -   $< > $@
> > -
> >  all-local : weston.ini ivi-shell/weston.ini
> >
> >  AM_CFLAGS = $(GCC_CFLAGS)
> > @@ -56,7 +47,6 @@ AM_CPPFLAGS =
>   \
> >
> >  CLEANFILES = weston.ini\
> > ivi-shell/weston.ini\
> > -   tests/weston-ivi.ini\
> > internal-screenshot-00.png  \
> > $(BUILT_SOURCES)
> >
> > @@ -1238,10 +1228,6 @@ weston_tests =
>   \
> > devices.weston  \
> > touch.weston
> >
> > -ivi_tests =
> > -
> > -$(ivi_tests) : $(builddir)/tests/weston-ivi.ini
> > -
> >  AM_TESTS_ENVIRONMENT = \
> > abs_builddir='$(abs_builddir)'; export abs_builddir; \
> > abs_top_srcdir='$(abs_top_srcdir)'; export abs_top_srcdir;
> > @@ -1472,7 +1458,7 @@ nodist_ivi_layout_test_la_SOURCES =
>   \
> > protocol/weston-test-protocol.c \
> > protocol/weston-test-server-protocol.h
> >
> > -ivi_tests +=   \
> > +ivi_tests =\
> > ivi-shell-app.weston
> >
> >  ivi_shell_app_weston_SOURCES = tests/ivi-shell-app-test.c
> > diff --git a/tests/weston-tests-env b/tests/weston-tests-env
> > index f08270e..ac2473f 100755
> > --- a/tests/weston-tests-env
> > +++ b/tests/weston-tests-env
> > @@ -44,7 +44,7 @@ case $TEST_FILE in
> > WESTON_BUILD_DIR=$abs_builddir \
> >
>   WESTON_TEST_REFERENCE_PATH=$abs_top_srcdir/tests/reference
> \
> > $WESTON --backend=$MODDIR/$BACKEND \
> > -   --config=$abs_builddir/tests/weston-ivi.ini \
> > +   --no-config \
> > --shell=$SHELL_PLUGIN \
> > --socket=test-${TEST_NAME} \
> > --
> modules=$TEST_PLUGIN,$MODDIR/${TEST_FILE/.la/.so}\
> > @@ -74,7 +74,7 @@ case $TEST_FILE in
> >
>   WESTON_TEST_REFERENCE_PATH=$abs_top_srcdir/tests/reference
> \
> > WESTON_TEST_CLIENT_PATH=$abs_builddir/$TEST_FILE \
> > $WESTON --backend=$MODDIR/$BACKEND \
> > -   --config=$abs_builddir/tests/weston-ivi.ini \
> > +   --no-config \
> > --shell=$SHELL_PLUGIN \
> > --socket=test-${TEST_NAME} \
> > --modules=$TEST_PLUGIN \
> 
> Hi Emre,
> 
> I didn't even think of that, I thought ivi-shell, hmi-controller, or
> ivi-shell-user-interface 

Re: [PATCH weston] tests: fix a race condition in ivi-shell tests

2018-02-14 Thread Pekka Paalanen
On Wed, 14 Feb 2018 11:06:54 +0100
Emre Ucan  wrote:

> ivi-shell tests load their own controller plugin
> for testing purposes. Tests also uses the generated
> weston-ivi.in config file, which causes weston to
> load hmi-controller and its helper client.
> Existence of hmi-controller and its helper client
> confuses test plugins. Because they are creating
> surfaces and layers which are not expected by
> test plugins.
> 
> We can start ivi-shell tests without config file
> to solve this problem. Then, weston will not load
> hmi-controller plugin.
> 
> Reported-by: Pekka Paalanen 
> Signed-off-by: Emre Ucan 
> ---
>  Makefile.am| 16 +---
>  tests/weston-tests-env |  4 ++--
>  2 files changed, 3 insertions(+), 17 deletions(-)
> 
> diff --git a/Makefile.am b/Makefile.am
> index 32c9a0f..c534c56 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -30,15 +30,6 @@ ivi-shell/weston.ini : $(srcdir)/ivi-shell/weston.ini.in
>   -e 's|@plugin_prefix[@]||g' \
>   $< > $@
>  
> -tests/weston-ivi.ini : $(srcdir)/ivi-shell/weston.ini.in
> - $(AM_V_GEN)$(MKDIR_P) $(dir $@) && $(SED) \
> - -e 's|@bindir[@]|$(bindir)|g' \
> - -e 's|@abs_top_builddir[@]|$(abs_top_builddir)|g' \
> - -e 's|@abs_top_srcdir[@]|$(abs_top_srcdir)|g' \
> - -e 's|@libexecdir[@]|$(abs_builddir)|g' \
> - -e 's|@plugin_prefix[@]|$(abs_top_builddir)/.libs/|g' \
> - $< > $@
> -
>  all-local : weston.ini ivi-shell/weston.ini
>  
>  AM_CFLAGS = $(GCC_CFLAGS)
> @@ -56,7 +47,6 @@ AM_CPPFLAGS =   \
>  
>  CLEANFILES = weston.ini  \
>   ivi-shell/weston.ini\
> - tests/weston-ivi.ini\
>   internal-screenshot-00.png  \
>   $(BUILT_SOURCES)
>  
> @@ -1238,10 +1228,6 @@ weston_tests = \
>   devices.weston  \
>   touch.weston
>  
> -ivi_tests =
> -
> -$(ivi_tests) : $(builddir)/tests/weston-ivi.ini
> -
>  AM_TESTS_ENVIRONMENT = \
>   abs_builddir='$(abs_builddir)'; export abs_builddir; \
>   abs_top_srcdir='$(abs_top_srcdir)'; export abs_top_srcdir;
> @@ -1472,7 +1458,7 @@ nodist_ivi_layout_test_la_SOURCES = \
>   protocol/weston-test-protocol.c \
>   protocol/weston-test-server-protocol.h
>  
> -ivi_tests += \
> +ivi_tests =  \
>   ivi-shell-app.weston
>  
>  ivi_shell_app_weston_SOURCES = tests/ivi-shell-app-test.c
> diff --git a/tests/weston-tests-env b/tests/weston-tests-env
> index f08270e..ac2473f 100755
> --- a/tests/weston-tests-env
> +++ b/tests/weston-tests-env
> @@ -44,7 +44,7 @@ case $TEST_FILE in
>   WESTON_BUILD_DIR=$abs_builddir \
>   WESTON_TEST_REFERENCE_PATH=$abs_top_srcdir/tests/reference \
>   $WESTON --backend=$MODDIR/$BACKEND \
> - --config=$abs_builddir/tests/weston-ivi.ini \
> + --no-config \
>   --shell=$SHELL_PLUGIN \
>   --socket=test-${TEST_NAME} \
>   --modules=$TEST_PLUGIN,$MODDIR/${TEST_FILE/.la/.so}\
> @@ -74,7 +74,7 @@ case $TEST_FILE in
>   WESTON_TEST_REFERENCE_PATH=$abs_top_srcdir/tests/reference \
>   WESTON_TEST_CLIENT_PATH=$abs_builddir/$TEST_FILE \
>   $WESTON --backend=$MODDIR/$BACKEND \
> - --config=$abs_builddir/tests/weston-ivi.ini \
> + --no-config \
>   --shell=$SHELL_PLUGIN \
>   --socket=test-${TEST_NAME} \
>   --modules=$TEST_PLUGIN \

Hi Emre,

I didn't even think of that, I thought ivi-shell, hmi-controller, or
ivi-shell-user-interface would refuse to start without a config, but I
see the defaults are enough to get a black screen at least.

I do wonder about launch_hmi_client_process() calling
weston_client_start() and there strdup() with a NULL argument, but
since nothing loads hmi-controller, that's never hit.

Another thing is that no test ever loads hmi-controller now, which
means it is free to break without notice. I think at least
ivi-shell-app test was supposed to load it to ensure the demo runs, but
obviously as the test still passes, it never tested the helper client
in any way.

All in all, I'm fine with this patch and it does fix the test suite
failure, so R-b me, and Daniel gave A-b on irc, so pushed:
   eba58edc..c6e2942f  master -> master


Thanks,
pq


pgpQBaTJUCP2z.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel