[PATCH libinput 3/8] test: Add seat slot tests
Peter Hutterer
peter.hutterer at who-t.net
Wed Mar 26 02:32:54 PDT 2014
On Tue, Mar 25, 2014 at 09:45:54PM +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-genric-highres-touch supports
> this so far and only generic artifical test devices should, in order to
> keep emulated devices closer to their originals.
I'm generally happy with the test itself, but the extension to the litest
framework I'm not convinced.
There are a bunch of tests already that need specific devices and they just
create a uinput device. I've got an unfinished series here that adds a
wrapper to make this easier and with less boilerplate.
My main worries with this here is that the deviceid is part of the
litest_device now but meaningless for all other devices. And for this
specific test you're essentially hiding one of the main features in the
generic_highres device - that it has > MAX_SLOT slots. Plus the extra bits
that elevate litest from a "initializes behind the scenes for multiple
devices" to something that almost looks like an API when it really doesn't
give you that much benefit over just creating a uinput device. Finally, we
already have generic devices, you could simply add a generic_toomanyslots
device.
I'll try to clean up the uinput wrapper I mentioned above tomorrow, maybe
that will be enough for this patch.
a few other comments inline
>
> Signed-off-by: Jonas Ådahl <jadahl at gmail.com>
> ---
> test/litest-generic-highres-touch.c | 28 +++++-
> test/litest.c | 33 ++++++-
> test/litest.h | 13 ++-
> test/touch.c | 184 ++++++++++++++++++++++++++++++++++++
> 4 files changed, 251 insertions(+), 7 deletions(-)
>
> diff --git a/test/litest-generic-highres-touch.c b/test/litest-generic-highres-touch.c
> index 68615c3..5f2f023 100644
> --- a/test/litest-generic-highres-touch.c
> +++ b/test/litest-generic-highres-touch.c
> @@ -23,11 +23,22 @@
>
> #include "config.h"
>
> +#include <stdio.h>
> +
> #include "litest.h"
> #include "litest-int.h"
> #include "libinput-util.h"
>
> -void litest_generic_highres_touch_setup(void)
> +static int device_ids = 0;
> +
> +static void
> +litest_generic_highres_touch_destroy(struct litest_device *dev)
> +{
> + device_ids &= ~dev->device_id;
> +}
> +
> +void
> +litest_generic_highres_touch_setup(void)
> {
> struct litest_device *d =
> litest_create_device(LITEST_GENERIC_HIGHRES_TOUCH);
> @@ -96,22 +107,33 @@ litest_create_generic_highres_touch(struct litest_device *d)
> {
> struct libevdev *dev;
> int rc;
> + int device_id;
> + char name[255];
> struct input_absinfo *a;
> struct input_absinfo abs[] = {
> { ABS_X, 0, 32767, 75 },
> { ABS_Y, 0, 32767, 129 },
> - { ABS_MT_SLOT, 0, 1, 0 },
> + { ABS_MT_SLOT, 0, 40, 0 },
> { ABS_MT_POSITION_X, 0, 32767, 0, 0, 10 },
> { ABS_MT_POSITION_Y, 0, 32767, 0, 0, 9 },
> { ABS_MT_TRACKING_ID, 0, 65535, 0 },
> };
>
> d->interface = &interface;
> + d->destroy = litest_generic_highres_touch_destroy;
>
> dev = libevdev_new();
> ck_assert(dev != NULL);
>
> - libevdev_set_name(dev, "Generic emulated highres touch device");
> + 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,
> + "Generic emulated highres touch device (%d)",
> + device_id);
> + libevdev_set_name(dev, name);
> +
> libevdev_set_id_bustype(dev, 0x3);
> libevdev_set_id_vendor(dev, 0xabcd); /* Some random vendor. */
> libevdev_set_id_product(dev, 0x1234); /* Some random product id. */
> diff --git a/test/litest.c b/test/litest.c
> index 9241623..0e7da70 100644
> --- a/test/litest.c
> +++ b/test/litest.c
> @@ -328,7 +328,8 @@ const struct libinput_interface 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;
> @@ -360,7 +361,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);
> @@ -374,6 +375,28 @@ litest_create_device(enum litest_device_type which)
> return d;
> }
>
> +struct libinput *
> +litest_create_context(void)
> +{
> + struct libinput *libinput =
> + libinput_path_create_context(&interface, NULL);
> + ck_assert_notnull(libinput);
> + return libinput;
> +}
> +
> +struct litest_device *
> +litest_create_device(enum litest_device_type which)
> +{
> + struct libinput *libinput;
> + struct litest_device *d;
> +
> + libinput = litest_create_context();
> + d = litest_create_device_for(libinput, which);
> + d->owns_context = true;
> +
> + return d;
> +}
> +
> int
> litest_handle_events(struct litest_device *d)
> {
> @@ -394,8 +417,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 9f0f614..0aa1c99 100644
> --- a/test/litest.h
> +++ b/test/litest.h
> @@ -62,8 +62,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;
I'm not quite sure why you split these additions up. seems like they could
just be appended to existing fields.
> };
>
> void litest_add(const char *name, void *func,
> @@ -72,7 +75,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);
> +
> +struct libinput *litest_create_context(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);
> @@ -100,4 +107,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
wouldn't a simple ck_assert(ptr != NULL) be enough?
> +
> #endif /* LITEST_H */
> diff --git a/test/touch.c b/test/touch.c
> index 6833e10..9354c25 100644
> --- a/test/touch.c
> +++ b/test/touch.c
> @@ -22,6 +22,7 @@
>
> #include <config.h>
>
> +#include <stdio.h>
> #include <check.h>
> #include <errno.h>
> #include <fcntl.h>
> @@ -92,6 +93,187 @@ 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 = litest_create_context();
> +
> + dev1 = litest_create_device_for(libinput, LITEST_GENERIC_HIGHRES_TOUCH);
> + dev2 = litest_create_device_for(libinput, LITEST_GENERIC_HIGHRES_TOUCH);
> +
> + 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);
> + ck_assert_int_ge(seat_slot, 0);
> +
> + 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);
> + }
we re-use seat slots, I think the tests should also trigger that case, not
just the initial touch down. otherwise we may miss a bug where cleaning up
and re-assinging the same touch doesn't do the right thing.
> +
> + ck_assert(has_last_seat_slot);
> + ck_assert_int_ne(seat_slot, last_seat_slot);
> +
> + 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;
> + const int num_devices = 5;
> + struct litest_device *devices[num_devices];
> + int slot;
> + int seat_slot;
> + enum libinput_event_type type;
> + int found = 0;
> + int i;
> + const int num_tps = num_devices * 16;
this would need a comment that 16 == MAX_SLOTS
> + int slot_count = 0;
> + int slot_max_count;
> + int32_t slots[num_tps];
> +
> + libinput = litest_create_context();
> +
> + /* Activate touch points from several devices in order to surpass per
> + * device touch slot limit. */
> + for (i = 0; i < num_devices; ++i) {
> + devices[i] =
> + litest_create_device_for(libinput,
> + LITEST_GENERIC_HIGHRES_TOUCH);
> + }
if you add a litest_drain_events() here then you should be able to abort on
anything that's not a touch event in the while loops.
> +
> + for (i = 0; i < num_devices; ++i) {
> + dev = devices[i];
> + for (slot = 0; slot < num_tps; ++slot)
> + litest_touch_down(dev, slot, 0, 0);
> + }
> +
> + libinput_dispatch(libinput);
> +
> + /* Wait for 200 ms so we'll get all down events first. */
> + usleep(200000);
that seems odd. is this timeout really needed? doesn't the dispatch call
above get all of the touch down events?
> +
> + for (i = 0; i < num_devices; ++i) {
> + dev = devices[i];
> + for (slot = 0; slot < num_tps; ++slot)
> + litest_touch_up(dev, slot);
> + }
> +
> + libinput_dispatch(libinput);
> +
> + /* Expect up to `num_tps`, 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 (slot = 0; slot < slot_count; ++slot)
> + ck_assert_int_ne(seat_slot, slots[slot]);
> +
> + slots[slot_count] = seat_slot;
> + ++slot_count;
> + } else if (type == LIBINPUT_EVENT_TOUCH_UP) {
> + break;
since you have the break here anyway, you could just move the
litest_touch_up() calls to below this loop, then you won't need the usleep.
> + }
> +
> + libinput_dispatch(libinput);
> + }
> +
> + ck_assert_notnull(ev);
> + ck_assert_int_gt(slot_count, 0);
> +
> + slot_max_count = slot_count;
> +
> + /* Make sure order was not shuffled and we received up before all
> + * down, and that we received as many up as down. */
> + do {
> + switch (libinput_event_get_type(ev)) {
hmm, switch here but if/else in the previous loop.
> + case LIBINPUT_EVENT_TOUCH_DOWN:
> + ck_abort();
> + break;
> + case 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 (slot = 0; slot < slot_max_count; ++slot)
> + if (slots[slot] == seat_slot)
> + ++found;
> + ck_assert_int_eq(found, 1);
> + break;
> + default:
> + break;
> + }
> +
> + libinput_dispatch(libinput);
> +
> + if (slot_count == 0)
> + break;
> + } while ((ev = libinput_get_event(libinput)));
> +
> + /* 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);
> + }
there shouldn't be any event in the queue now, right? you haven't killed the
devices, so any other events are probably not good.
> +
> + ck_assert_int_eq(slot_count, 0);
> +
> + for (i = 0; i < num_devices; ++i)
> + litest_delete_device(devices[i]);
> +
> + libinput_destroy(libinput);
> +
> + fprintf(stderr,
> + "libinput supports %s%d simultaneous touch seat slots\n",
> + slot_max_count == num_tps ? ">=" : "", slot_max_count);
is this debugging leftover? If not, this seems like something we could just
export elsewhere for general use instead of printing it on every test run.
(./tools/libinput-capabilities or so, though I do wonder who'd really need it)
Cheers,
Peter
> +}
> +END_TEST
>
> int
> main(int argc, char **argv)
> @@ -99,6 +281,8 @@ 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_no_device("touch:seat-slot-drop", touch_seat_slot_drop);
>
> return litest_run(argc, argv);
> }
> --
> 1.8.3.2
>
More information about the wayland-devel
mailing list