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

[Ucan, Emre (ADITG/ESB)]

Hi Pekka,


> -----Original Message-----
> From: wayland-devel [mailto:wayland-devel-
> bounces at lists.freedesktop.org] On Behalf Of Pekka Paalanen
> Sent: Mittwoch, 14. Februar 2018 12:22
> To: Ucan, Emre (ADITG/ESB)
> Cc: wayland-devel at 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 <eucan at de.adit-jv.com> 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 <pekka.paalanen at collabora.co.uk>
> > Signed-off-by: Emre Ucan <eucan at de.adit-jv.com>
> > ---
> >  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.

ivi-shell-app only tests ivi_application protocol. It does not have any dependency to hmi-controller.
ivi-layout tests are testing ivi-layout interface. Therefore, they don't require hmi-controller also.

For testing hmi-controller, we have to implement another test application which uses ivi_hmi_controller plugin.
I will take this point to our internal TODO list.

Best Regards,
Emre

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