[PATCH libinput] test: Add seat slot tests

Peter Hutterer peter.hutterer at who-t.net
Sun Feb 23 16:52:38 PST 2014


On Sat, Feb 22, 2014 at 03:38:28PM +0100, Jonas Ådahl wrote:
> Add one test that checks uniqueness of seat slots when having multiple
> devices with active touch points.
> 
> Add one test that checks that libinput drops touch points when it could
> not represent them with a seat wide slot.
> 
> This commit also adds support for from a test case add test devices to
> an existing libinput context. Only litest-wacom-touch supports this so
> far.
> 
> Signed-off-by: Jonas Ådahl <jadahl at gmail.com>
> ---
> 
> Needs to be applied after 'Split up the touch event into the different
> touch types'.
> 
>  test/litest-wacom-touch.c |  24 ++++++-
>  test/litest.c             |  30 ++++++++-
>  test/litest.h             |  13 +++-
>  test/touch.c              | 156 ++++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 217 insertions(+), 6 deletions(-)
> 
> diff --git a/test/litest-wacom-touch.c b/test/litest-wacom-touch.c
> index 464d541..a6c22ef 100644
> --- a/test/litest-wacom-touch.c
> +++ b/test/litest-wacom-touch.c
> @@ -24,11 +24,22 @@
>  #include "config.h"
>  #endif
>  
> +#include <stdio.h>
> +
>  #include "litest.h"
>  #include "litest-int.h"
>  #include "libinput-util.h"
>  
> -void litest_wacom_touch_setup(void)
> +static int device_ids = 0;
> +
> +static void
> +litest_wacom_touch_destroy(struct litest_device *dev)
> +{
> +	device_ids &= ~dev->device_id;
> +}
> +
> +void
> +litest_wacom_touch_setup(void)
>  {
>  	struct litest_device *d = litest_create_device(LITEST_WACOM_TOUCH);
>  	litest_set_current_device(d);
> @@ -104,14 +115,23 @@ litest_create_wacom_touch(struct litest_device *d)
>  		{ ABS_MT_TRACKING_ID, 0, 65535, 0 },
>  	};
>  	struct input_absinfo *a;
> +	char name[256];
> +	int device_id;
>  	int rc;
>  
>  	d->interface = &interface;
> +	d->destroy = litest_wacom_touch_destroy;
>  
>  	dev = libevdev_new();
>  	ck_assert(dev != NULL);
>  
> -	libevdev_set_name(dev, "Wacom ISDv4 E6 Finger");
> +	device_id = ffs(~device_ids) - 1;
> +	ck_assert_int_ge(device_id, 0);
> +	device_ids |= 1 << device_id;
> +	d->device_id = device_id;
> +	snprintf(name, sizeof name, "Wacom ISDv4 E6 Finger (%d)", device_id);
> +	libevdev_set_name(dev, name);
> +

IMO the actual devices should stay as close to the real thing as possible.
That leaves us, for this test, with two options: put a 1.5s sleep in to
avoid the uinput duplicate issue, or push this code into the generic touch
device that you created for the scaling overflow issue. with that we have
more freedom of messing around with the device name.


>  	libevdev_set_id_bustype(dev, 0x3);
>  	libevdev_set_id_vendor(dev, 0x56a);
>  	libevdev_set_id_product(dev, 0xe6);
> diff --git a/test/litest.c b/test/litest.c
> index 78a0472..e69f354 100644
> --- a/test/litest.c
> +++ b/test/litest.c
> @@ -325,8 +325,15 @@ const struct libinput_interface interface = {
>  	.close_restricted = close_restricted,
>  };
>  
> +const struct libinput_interface *
> +litest_get_libinput_interface()
> +{
> +	return &interface;
> +}
> +
>  struct litest_device *
> -litest_create_device(enum litest_device_type which)
> +litest_create_device_for(struct libinput *libinput,
> +			 enum litest_device_type which)
>  {
>  	struct litest_device *d = zalloc(sizeof(*d));
>  	int fd;
> @@ -358,7 +365,7 @@ litest_create_device(enum litest_device_type which)
>  	rc = libevdev_new_from_fd(fd, &d->evdev);
>  	ck_assert_int_eq(rc, 0);
>  
> -	d->libinput = libinput_path_create_context(&interface, NULL);
> +	d->libinput = libinput;
>  	ck_assert(d->libinput != NULL);
>  
>  	d->libinput_device = libinput_path_add_device(d->libinput, path);
> @@ -372,6 +379,19 @@ litest_create_device(enum litest_device_type which)
>  	return d;
>  }
>  
> +struct litest_device *
> +litest_create_device(enum litest_device_type which)
> +{
> +	struct libinput *libinput;
> +	struct litest_device *d;
> +
> +	libinput = libinput_path_create_context(&interface, NULL);
> +	d = litest_create_device_for(libinput, which);
> +	d->owns_context = true;
> +
> +	return d;
> +}
> +
>  int
>  litest_handle_events(struct litest_device *d)
>  {
> @@ -392,8 +412,12 @@ litest_delete_device(struct litest_device *d)
>  	if (!d)
>  		return;
>  
> +	if (d->destroy)
> +		d->destroy(d);
> +
>  	libinput_device_unref(d->libinput_device);
> -	libinput_destroy(d->libinput);
> +	if (d->owns_context)
> +		libinput_destroy(d->libinput);
>  	libevdev_free(d->evdev);
>  	libevdev_uinput_destroy(d->uinput);
>  	memset(d,0, sizeof(*d));
> diff --git a/test/litest.h b/test/litest.h
> index 9cc0ff5..fef051d 100644
> --- a/test/litest.h
> +++ b/test/litest.h
> @@ -60,8 +60,11 @@ struct litest_device {
>  	struct libevdev *evdev;
>  	struct libevdev_uinput *uinput;
>  	struct libinput *libinput;
> +	void (*destroy)(struct litest_device *d);
> +	bool owns_context;
>  	struct libinput_device *libinput_device;
>  	struct litest_device_interface *interface;
> +	int device_id;

while overkill for now, is this something that we should consider adding a
void *device_private for?

>  };
>  
>  void litest_add(const char *name, void *func,
> @@ -70,7 +73,11 @@ void litest_add(const char *name, void *func,
>  void litest_add_no_device(const char *name, void *func);
>  
>  int litest_run(int argc, char **argv);
> -struct litest_device * litest_create_device(enum litest_device_type which);
> +
> +const struct libinput_interface *litest_get_libinput_interface(void);
> +struct litest_device *litest_create_device(enum litest_device_type which);
> +struct litest_device *litest_create_device_for(struct libinput *libinput,
> +					       enum litest_device_type which);
>  struct litest_device *litest_current_device(void);
>  void litest_delete_device(struct litest_device *d);
>  int litest_handle_events(struct litest_device *d);
> @@ -98,4 +105,8 @@ void litest_button_click(struct litest_device *d,
>  			 bool is_press);
>  void litest_drain_events(struct libinput *li);
>  
> +#ifndef ck_assert_notnull
> +#define ck_assert_notnull(ptr) ck_assert_ptr_ne(ptr, NULL)
> +#endif
> +
>  #endif /* LITEST_H */
> diff --git a/test/touch.c b/test/touch.c
> index 6833e10..661e295 100644
> --- a/test/touch.c
> +++ b/test/touch.c
> @@ -92,6 +92,159 @@ START_TEST(touch_abs_transform)
>  }
>  END_TEST
>  
> +START_TEST(touch_seat_slots)
> +{
> +
> +	struct libinput *libinput;
> +	struct litest_device *dev1;
> +	struct litest_device *dev2;
> +	struct libinput_event *ev;
> +	struct libinput_event_touch *tev;
> +	int32_t seat_slot;
> +	int32_t last_seat_slot = -1;
> +	bool has_last_seat_slot = false;
> +
> +	libinput = libinput_path_create_context(litest_get_libinput_interface(),
> +						NULL);
> +	ck_assert_notnull(libinput);
> +
> +	dev1 = litest_create_device_for(libinput, LITEST_WACOM_TOUCH);
> +	dev2 = litest_create_device_for(libinput, LITEST_WACOM_TOUCH);

just looking at the code here, I wonder if we should wrap the
create_context call as well:

libinput = litest_create_context()
dev1 = litest_create_device_for_()
dev1 = litest_create_device_for_()

this way we don't need to expose the interface, and we can put a assert
into the litest function to save us all the extra asserts in the tests
(which we already do for the create_device function, so the checks
immediately below are superfluous)


> +	ck_assert_notnull(dev1);
> +	ck_assert_notnull(dev2);



> +
> +	litest_touch_down(dev1, 0, 0, 0);
> +	litest_touch_down(dev2, 0, 0, 0);
> +
> +	libinput_dispatch(libinput);
> +
> +	/*
> +	 * Read the two touch down events and ensure their seat slot are
> +	 * unique.
> +	 */
> +	while ((ev = libinput_get_event(libinput))) {
> +		if (libinput_event_get_type(ev) != LIBINPUT_EVENT_TOUCH_DOWN)
> +			continue;
> +
> +		tev = libinput_event_get_touch_event(ev);
> +
> +		ck_assert_int_eq(libinput_event_touch_get_slot(tev), 0);
> +		seat_slot = libinput_event_touch_get_seat_slot(tev);
> +

add a ck_assert_int_ge(seat_slot, 0); here too

> +		if (!has_last_seat_slot) {
> +			has_last_seat_slot = true;
> +			last_seat_slot = seat_slot;
> +			continue;
> +		}
> +
> +		ck_assert_int_ne(seat_slot, last_seat_slot);
> +
> +		libinput_dispatch(libinput);
> +	}
> +
> +	ck_assert(has_last_seat_slot);
> +	ck_assert_int_ne(seat_slot, last_seat_slot);

this seems like a superfluous test, can only happen if has_last_seat_slot is
false, so we'd abort earlier.

> +
> +	litest_delete_device(dev1);
> +	litest_delete_device(dev2);
> +
> +	libinput_destroy(libinput);
> +}
> +END_TEST
> +
> +START_TEST(touch_seat_slot_drop)
> +{
> +	struct libinput *libinput;
> +	struct litest_device *dev;
> +	struct libinput_event *ev;
> +	struct libinput_event_touch *tev;
> +	struct libinput_device *ldev;
> +	int32_t slot;
> +	int32_t seat_slot;
> +	enum libinput_event_type type;
> +	int found = 0;
> +	size_t i;
> +	const size_t num_tps = 40;
> +	size_t slot_count = 0;
> +	int32_t slots[num_tps];
> +
> +	dev = litest_current_device();
> +	libinput = dev->libinput;
> +
> +	for (slot = 0; i < num_tps; ++i)
> +		litest_touch_down(dev, slot, 0, 0);
> +	for (slot = 0; i < num_tps; ++i)
> +		litest_touch_up(dev, slot);
> +
> +	libinput_dispatch(libinput);
> +
> +	/* Expect up to 40, but allow fewer. */
> +	memset(slots, 0, sizeof slots);
> +	while ((ev = libinput_get_event(libinput))) {
> +		type = libinput_event_get_type(ev);
> +		if (type == LIBINPUT_EVENT_TOUCH_DOWN) {
> +			tev = libinput_event_get_touch_event(ev);
> +			seat_slot = libinput_event_touch_get_seat_slot(tev);
> +
> +			ck_assert_int_ge(seat_slot, 0);
> +			ck_assert_int_lt(slot_count, num_tps);
> +			ck_assert_int_eq(slots[slot_count], 0);
> +
> +			/* Assert uniqueness */
> +			for (i = 0; i < slot_count; ++i)
> +				ck_assert_int_ne(seat_slot, slots[i]);
> +
> +			slots[slot_count] = seat_slot;
> +			++slot_count;
> +		} else if (type == LIBINPUT_EVENT_TOUCH_UP) {
> +			tev = libinput_event_get_touch_event(ev);
> +			seat_slot = libinput_event_touch_get_seat_slot(tev);
> +
> +			--slot_count;
> +			found = 0;
> +			for (i = 0; i < slot_count; ++i)
> +				if (slots[i] == seat_slot)
> +					++found;
> +			ck_assert_int_eq(found, 1);
> +			break;
> +		}
> +
> +		libinput_dispatch(libinput);
> +	}
> +
> +	/* Make sure order was not shuffled and we received up before all
> +	 * down, and that we received as many up as down. */
> +	while ((ev = libinput_get_event(libinput))) {
> +		switch (libinput_event_get_type(ev)) {
> +		case LIBINPUT_EVENT_TOUCH_DOWN:
> +			ck_assert(0);

there's a ck_abort() call for this.

> +			break;
> +		case LIBINPUT_EVENT_TOUCH_UP:
> +			--slot_count;
> +			found = 0;
> +			for (i = 0; i < slot_count; ++i)
> +				if (slots[i] == seat_slot)

you're not getting the seat_slot from the event here. Which brings me to the
comment that I wanted to write anyway: I think it's better if you write this
test as two loops:

 while () {
    if (touch down)
        handle event
    else if (touch up)
        break;
 } 

 do  {
    if (touch down)
        abort()
    else if (touch up)
        handle event
 } while (...);


this way you only have one block that handles touch down events. on the
first run, the event is already set from the previous while loop (though you
need some checks that the loop didn't end on its own account).


> +					++found;
> +			ck_assert_int_eq(found, 1);
> +			break;
> +		default:
> +			break;
> +		}
> +
> +		if (slot_count == 0)
> +			break;
> +
> +		libinput_dispatch(libinput);
> +	}
> +

maybe add a libinput_dispatch() here, just in case.

> +	/* Make sure we didn't receive too many up. */
> +	while ((ev = libinput_get_event(libinput))) {
> +		type = libinput_event_get_type(ev);
> +		ck_assert_int_ne(type, LIBINPUT_EVENT_TOUCH_DOWN);
> +		ck_assert_int_ne(type, LIBINPUT_EVENT_TOUCH_UP);
> +	}

I think you still need a ck_assert_eq(slot_count, 0) here to make sure you
did get all up events and no slot is still pending.

Cheers,
   Peter

> +}
> +END_TEST
>  
>  int
>  main(int argc, char **argv)
> @@ -99,6 +252,9 @@ main(int argc, char **argv)
>  	litest_add("touch:frame", touch_frame_events, LITEST_TOUCH, LITEST_ANY);
>  	litest_add("touch:abs-transform", touch_abs_transform,
>  		   LITEST_TOUCH, LITEST_ANY);
> +	litest_add_no_device("touch:seat-slot", touch_seat_slots);
> +	litest_add("touch:seat-slot-drop", touch_seat_slot_drop,
> +		   LITEST_TOUCH, LITEST_ANY);
>  
>  	return litest_run(argc, argv);
>  }
> -- 
> 1.8.3.2
> 
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
> 


More information about the wayland-devel mailing list