[PATCH libinput] test: add per-device udev rule support
Hans de Goede
hdegoede at redhat.com
Mon Feb 2 01:50:39 PST 2015
Hi,
On 02-02-15 04:55, Peter Hutterer wrote:
> Don't rely on a magic version tag, instead let a device define a udev rule and
> drop that into the udev runtime directory before the device is created.
>
> There are a couple of caveats with this approach: first, since this changes
> system-wide state it may cause issues on the device the test suite is run on.
> This can be avoided if the udev rules have filter patterns that ensure only
> test devices are affected.
>
> Second, the check test suite aborts but it doesn't run the teardown() function
> if a test fails. So far this wasn't a problem since uinput devices disappear
> whenever we exit. The rules files will hang around though, so an unchecked
> fixture was added to delete all litest-foo.rules files before and after a test
> case starts. Unchecked fixtures are run regardless of the exit status of the
> test but run in the same address space - i.e. no ck_assert() usage.
>
> Also unchecked fixtures are only run once per test-case, not once per test
> function. For us, that means they're only run once per device (we use the
> devices as test case), i.e. if a test fails and the udev rule isn't tidied up,
> the next test may be unpredictable. This shouldn't matter too much though.
>
> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
Looks good, thanks for working on this:
Reviewed-by: Hans de Goede <hdegoede at redhat.com>
Regards,
Hans
> ---
> configure.ac | 1 +
> src/evdev-mt-touchpad.c | 6 --
> test/litest-int.h | 2 +
> test/litest-synaptics-x1-carbon-3rd.c | 14 ++++-
> test/litest.c | 106 +++++++++++++++++++++++++++++++++-
> test/litest.h | 2 +
> 6 files changed, 122 insertions(+), 9 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index 39e42d0..803d573 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -34,6 +34,7 @@ LIBINPUT_LT_VERSION=8:0:1
> AC_SUBST(LIBINPUT_LT_VERSION)
>
> AM_SILENT_RULES([yes])
> +AC_USE_SYSTEM_EXTENSIONS
>
> # Check for programs
> AC_PROG_CC_C99
> diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c
> index 94cbc13..ae37ab1 100644
> --- a/src/evdev-mt-touchpad.c
> +++ b/src/evdev-mt-touchpad.c
> @@ -1107,12 +1107,6 @@ tp_tag_device(struct evdev_device *device,
> if (udev_device_get_property_value(udev_device,
> "TOUCHPAD_HAS_TRACKPOINT_BUTTONS"))
> device->tags |= EVDEV_TAG_TOUCHPAD_TRACKPOINT;
> -
> - /* Magic version tag: used by the litest device. Should never be set
> - * in real life but allows us to test for these features without
> - * requiring custom udev rules during make check */
> - if (libevdev_get_id_version(device->evdev) == 0xfffa)
> - device->tags |= EVDEV_TAG_TOUCHPAD_TRACKPOINT;
> }
>
> static struct evdev_dispatch_interface tp_interface = {
> diff --git a/test/litest-int.h b/test/litest-int.h
> index 95bc248..12746f1 100644
> --- a/test/litest-int.h
> +++ b/test/litest-int.h
> @@ -69,6 +69,8 @@ struct litest_test_device {
> */
> struct input_absinfo *absinfo;
> struct litest_device_interface *interface;
> +
> + const char *udev_rule;
> };
>
> struct litest_device_interface {
> diff --git a/test/litest-synaptics-x1-carbon-3rd.c b/test/litest-synaptics-x1-carbon-3rd.c
> index 8be29e0..67d6f46 100644
> --- a/test/litest-synaptics-x1-carbon-3rd.c
> +++ b/test/litest-synaptics-x1-carbon-3rd.c
> @@ -65,7 +65,6 @@ static struct input_id input_id = {
> .bustype = 0x11,
> .vendor = 0x2,
> .product = 0x7,
> - .version = 0xfffa, /* Magic value, used to detect this test device */
> };
>
> static int events[] = {
> @@ -97,6 +96,16 @@ static struct input_absinfo absinfo[] = {
> { .value = -1 }
> };
>
> +static const char udev_rule[] =
> +"ACTION==\"remove\", GOTO=\"touchpad_end\"\n"
> +"KERNEL!=\"event*\", GOTO=\"touchpad_end\"\n"
> +"ENV{ID_INPUT_TOUCHPAD}==\"\", GOTO=\"touchpad_end\"\n"
> +"\n"
> +"ATTRS{name}==\"litest*X1C3rd*\",\\\n"
> +" ENV{TOUCHPAD_HAS_TRACKPOINT_BUTTONS}=\"1\"\n"
> +"\n"
> +"LABEL=\"touchpad_end\"";
> +
> struct litest_test_device litest_synaptics_carbon3rd_device = {
> .type = LITEST_SYNAPTICS_TRACKPOINT_BUTTONS,
> .features = LITEST_TOUCHPAD | LITEST_CLICKPAD | LITEST_BUTTON,
> @@ -104,8 +113,9 @@ struct litest_test_device litest_synaptics_carbon3rd_device = {
> .setup = litest_synaptics_carbon3rd_setup,
> .interface = &interface,
>
> - .name = "SynPS/2 Synaptics TouchPad",
> + .name = "SynPS/2 Synaptics TouchPad X1C3rd",
> .id = &input_id,
> .events = events,
> .absinfo = absinfo,
> + .udev_rule = udev_rule,
> };
> diff --git a/test/litest.c b/test/litest.c
> index 2a336e9..7f55d47 100644
> --- a/test/litest.c
> +++ b/test/litest.c
> @@ -26,6 +26,7 @@
>
> #include <assert.h>
> #include <check.h>
> +#include <dirent.h>
> #include <errno.h>
> #include <fcntl.h>
> #include <getopt.h>
> @@ -44,6 +45,9 @@
> #include "litest-int.h"
> #include "libinput-util.h"
>
> +#define UDEV_RULES_D "/run/udev/rules.d"
> +#define UDEV_RULE_PREFIX "99-litest-"
> +
> static int in_debugger = -1;
> static int verbose = 0;
>
> @@ -115,6 +119,55 @@ struct litest_test_device* devices[] = {
> static struct list all_tests;
>
> static void
> +litest_reload_udev_rules(void)
> +{
> + system("udevadm control --reload-rules");
> +}
> +
> +static int
> +litest_udev_rule_filter(const struct dirent *entry)
> +{
> + return strncmp(entry->d_name,
> + UDEV_RULE_PREFIX,
> + strlen(UDEV_RULE_PREFIX)) == 0;
> +}
> +
> +static void
> +litest_drop_udev_rules(void)
> +{
> + int n;
> + int rc;
> + struct dirent **entries;
> + char path[PATH_MAX];
> +
> + n = scandir(UDEV_RULES_D,
> + &entries,
> + litest_udev_rule_filter,
> + alphasort);
> + if (n < 0)
> + return;
> +
> + while (n--) {
> + rc = snprintf(path, sizeof(path),
> + "%s/%s",
> + UDEV_RULES_D,
> + entries[n]->d_name);
> + if (rc > 0 &&
> + (size_t)rc == strlen(UDEV_RULES_D) +
> + strlen(entries[n]->d_name) + 1)
> + unlink(path);
> + else
> + fprintf(stderr,
> + "Failed to delete %s. Remaining tests are unreliable\n",
> + entries[n]->d_name);
> + free(entries[n]);
> + }
> + free(entries);
> +
> + litest_reload_udev_rules();
> +}
> +
> +static void
> litest_add_tcase_for_device(struct suite *suite,
> void *func,
> const struct litest_test_device *dev)
> @@ -134,6 +187,13 @@ litest_add_tcase_for_device(struct suite *suite,
> t->name = strdup(test_name);
> t->tc = tcase_create(test_name);
> list_insert(&suite->tests, &t->node);
> + /* we can't guarantee that we clean up properly if a test fails, the
> + udev rules used for a previous test may still be in place. Add an
> + unchecked fixture to always clean up all rules before/after a
> + test case completes */
> + tcase_add_unchecked_fixture(t->tc,
> + litest_drop_udev_rules,
> + litest_drop_udev_rules);
> tcase_add_checked_fixture(t->tc, dev->setup,
> dev->teardown ? dev->teardown : litest_generic_device_teardown);
> tcase_add_test(t->tc, func);
> @@ -464,6 +524,40 @@ merge_events(const int *orig, const int *override)
> return events;
> }
>
> +static char *
> +litest_init_udev_rules(struct litest_test_device *dev)
> +{
> + int rc;
> + FILE *f;
> + char *path;
> +
> + if (!dev->udev_rule)
> + return NULL;
> +
> + rc = mkdir(UDEV_RULES_D, 0755);
> + if (rc == -1 && errno != EEXIST)
> + ck_abort_msg("Failed to create udev rules directory (%s)\n",
> + strerror(errno));
> +
> + rc = asprintf(&path,
> + "%s/%s%s.rules",
> + UDEV_RULES_D,
> + UDEV_RULE_PREFIX,
> + dev->shortname);
> + ck_assert_int_eq(rc,
> + strlen(UDEV_RULES_D) +
> + strlen(UDEV_RULE_PREFIX) +
> + strlen(dev->shortname) + 7);
> + f = fopen(path, "w");
> + ck_assert_notnull(f);
> + ck_assert_int_ge(fputs(dev->udev_rule, f), 0);
> + fclose(f);
> +
> + litest_reload_udev_rules();
> +
> + return path;
> +}
> +
> static struct litest_device *
> litest_create(enum litest_device_type which,
> const char *name_override,
> @@ -477,6 +571,7 @@ litest_create(enum litest_device_type which,
> const struct input_id *id;
> struct input_absinfo *abs;
> int *events;
> + char *udev_file;
>
> dev = devices;
> while (*dev) {
> @@ -491,12 +586,17 @@ litest_create(enum litest_device_type which,
> d = zalloc(sizeof(*d));
> ck_assert(d != NULL);
>
> + udev_file = litest_init_udev_rules(*dev);
> +
> /* device has custom create method */
> if ((*dev)->create) {
> (*dev)->create(d);
> - if (abs_override || events_override)
> + if (abs_override || events_override) {
> + if (udev_file)
> + unlink(udev_file);
> ck_abort_msg("Custom create cannot"
> "be overridden");
> + }
>
> return d;
> }
> @@ -511,6 +611,7 @@ litest_create(enum litest_device_type which,
> abs,
> events);
> d->interface = (*dev)->interface;
> + d->udev_rule_file = udev_file;
> free(abs);
> free(events);
>
> @@ -629,6 +730,9 @@ litest_delete_device(struct litest_device *d)
> if (!d)
> return;
>
> + if (d->udev_rule_file)
> + unlink(d->udev_rule_file);
> +
> libinput_device_unref(d->libinput_device);
> libinput_path_remove_device(d->libinput_device);
> if (d->owns_context)
> diff --git a/test/litest.h b/test/litest.h
> index 0625a71..60ec458 100644
> --- a/test/litest.h
> +++ b/test/litest.h
> @@ -84,6 +84,8 @@ struct litest_device {
> bool skip_ev_syn;
>
> void *private; /* device-specific data */
> +
> + char *udev_rule_file;
> };
>
> struct libinput *litest_create_context(void);
>
More information about the wayland-devel
mailing list