[PATCH weston 8/8] tests: Add test for seat destruction and creation

[Alexandros Frantzis]

On Thu, Feb 01, 2018 at 12:46:13PM +0200, Pekka Paalanen wrote:
> On Fri, 26 Jan 2018 18:48:02 +0200
> Alexandros Frantzis <alexandros.frantzis at collabora.com> wrote:
> 
> > Add a test to check that we can destroy and create the test seat. Since
> > after test seat destruction the test client releases any associated
> > input resources, this test also checks that libweston properly handles
> > release requests for inert input resources.
> > 
> > The test is placed in its own file for the time being, so it can run
> > independently. This is needed because the desktop shell, which is used
> > when running tests, doesn't deal well with seat destruction and creation
> > at the moment and may causes crashes in other tests. When this is fixed
> > we can merge this test into devices-test.c.
> > 
> > Signed-off-by: Alexandros Frantzis <alexandros.frantzis at collabora.com>
> > ---
> >  Makefile.am               |  5 +++++
> >  tests/devices-seat-test.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 58 insertions(+)
> >  create mode 100644 tests/devices-seat-test.c
> > 
> > diff --git a/Makefile.am b/Makefile.am
> > index e224d606..f0370973 100644
> > --- a/Makefile.am
> > +++ b/Makefile.am
> > @@ -1234,6 +1234,7 @@ weston_tests =					\
> >  	subsurface.weston			\
> >  	subsurface-shot.weston			\
> >  	devices.weston				\
> > +	devices-seat.weston			\
> >  	touch.weston
> >  
> >  ivi_tests =
> > @@ -1392,6 +1393,10 @@ devices_weston_SOURCES = tests/devices-test.c
> >  devices_weston_CFLAGS = $(AM_CFLAGS) $(TEST_CLIENT_CFLAGS)
> >  devices_weston_LDADD = libtest-client.la
> >  
> > +devices_seat_weston_SOURCES = tests/devices-seat-test.c
> > +devices_seat_weston_CFLAGS = $(AM_CFLAGS) $(TEST_CLIENT_CFLAGS)
> > +devices_seat_weston_LDADD = libtest-client.la
> > +
> >  text_weston_SOURCES = tests/text-test.c
> >  nodist_text_weston_SOURCES =			\
> >  	protocol/text-input-unstable-v1-protocol.c		\
> > diff --git a/tests/devices-seat-test.c b/tests/devices-seat-test.c
> > new file mode 100644
> > index 00000000..182df1d5
> > --- /dev/null
> > +++ b/tests/devices-seat-test.c
> > @@ -0,0 +1,53 @@
> > +/*
> > + * Copyright © 2018 Collabora, Ltd.
> > + *
> > + * Permission is hereby granted, free of charge, to any person obtaining
> > + * a copy of this software and associated documentation files (the
> > + * "Software"), to deal in the Software without restriction, including
> > + * without limitation the rights to use, copy, modify, merge, publish,
> > + * distribute, sublicense, and/or sell copies of the Software, and to
> > + * permit persons to whom the Software is furnished to do so, subject to
> > + * the following conditions:
> > + *
> > + * The above copyright notice and this permission notice (including the
> > + * next paragraph) shall be included in all copies or substantial
> > + * portions of the Software.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> > + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> > + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> > + * NONINFRINGEMENT.  IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
> > + * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
> > + * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
> > + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> > + * SOFTWARE.
> > + */
> > +
> > +#include "config.h"
> > +
> > +#include "weston-test-client-helper.h"
> > +
> > +/**
> > + * Test destroying/recreating seats
> > + *
> > + * The seat destroy/recreate test is placed in its own file for the time
> > + * being, so it can run independently. This is needed because the desktop
> > + * shell, which is used when running tests, doesn't deal well with seat
> > + * destruction and recreation at the moment and may causes crashes in other
> > + * tests. When this is fixed we can merge this test into devices-test.c.
> > + */
> 
> Hi,
> 
> this is suspicious. It could cause random crashes just in this test as
> well, could it not?

In practice I have found that this causes problems only in subsequent
tests when using the same server (i.e., tests in the same test file) and
only with desktop-shell, but you are correct that since we don't know
the exact cause of the issue we can't be 100% sure.

> Seat removal really is very untested path, it's good to have some
> excercise there.
> 
> > +
> > +TEST(seat_destroy_and_recreate)
> > +{
> > +	struct client *cl = create_client_and_test_surface(100, 100, 100, 100);
> > +
> > +	weston_test_device_release(cl->test->weston_test, "seat");
> > +	/* Roundtrip to receive and handle the seat global removal event */
> > +	client_roundtrip(cl);
> 
> Assert it is actually gone.
> 
> > +
> > +	weston_test_device_add(cl->test->weston_test, "seat");
> > +	/* First roundtrip to send request and receive new seat global */
> > +	client_roundtrip(cl);
> > +	/* Second roundtrip to handle seat events and set up input devices */
> > +	client_roundtrip(cl);
> 
> Assert it came back.
> 
> > +}
> 
> I would prefer to have the seen crashes dealt with first, because a
> randomly failing test suite is really annoying when developing long
> patch series.

My plan was to have some kind of test (even if not ideal) in place for
the changes introduced in this patchset and continue with further
changes (e.g. fixing desktop-shell) in a different patchset, to keep
this patchset contained. I will now focus on debugging further and
fixing the desktop-shell issue first.

Thanks,
Alexandros