On Tue, Jul 05, 2016 at 12:43:43PM +1000, Peter Hutterer wrote:
> litest_add_device and litest_delete_device trigger a udev rule reload. This
> messes with some test devices and when we run multiple tests in parallel we
> get weird errors like "keyboard $BLAH failed the touchpad sanity test".
> 
> Still not 100% reliable to run tests in parallel, but it's vastly improved
> now.
> 
> Signed-off-by: Peter Hutterer <[email protected]>
> ---
>  configure.ac  |  3 +++
>  test/litest.c | 63 
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 66 insertions(+)
> 
> diff --git a/configure.ac b/configure.ac
> index 8c14efe..3e973d4 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -194,6 +194,9 @@ if test "x$build_tests" = "xyes"; then
>               AC_DEFINE_UNQUOTED(HAVE_ADDR2LINE, 1, [addr2line found])
>               AC_DEFINE_UNQUOTED(ADDR2LINE, ["$ADDR2LINE"], [Path to 
> addr2line])
>       fi
> +
> +     AC_DEFINE(LITEST_UDEV_LOCKFILE, ["/tmp/litest-udev.lock"],
> +               [Lock file used to restrict udev reloads during tests])
>  fi
>  AM_CONDITIONAL(HAVE_LIBUNWIND, [test "x$HAVE_LIBUNWIND" = xyes])
>  
> diff --git a/test/litest.c b/test/litest.c
> index 9b4feed..1250b3f 100644
> --- a/test/litest.c
> +++ b/test/litest.c
> @@ -1164,6 +1164,58 @@ litest_restore_log_handler(struct libinput *libinput)
>       libinput_log_set_handler(libinput, litest_log_handler);
>  }
>  
> +static inline int
> +create_udev_lock_file(void)
> +{
> +     int lfd;
> +
> +     /* Running the multiple tests in parallel usually trips over udev
> +      * not being  up-to-date. We change the udev rules for every device
> +      * created, sometimes this means we end up getting the wrong udev
> +      * device, or having wrong properties applied.
> +      *
> +      * litests use the path interface and there is a window between
> +      * creating the device (which triggers udev reloads) and adding the
> +      * device to the libinput context where another udev reload may
> +      * upset things.
> +      *
> +      * To avoid this, create a lockfile on device add and device delete
> +      * to make sure that we have exclusive access to udev while
> +      * the udev rules are reloaded.
> +      */
> +     do {
> +             lfd = open(LITEST_UDEV_LOCKFILE, O_CREAT|O_EXCL, O_RDWR);
> +
> +             if (lfd == -1) {
> +                     struct stat st;
> +                     time_t now = time(NULL);
> +
> +                     litest_assert_int_eq(errno, EEXIST);
> +                     msleep(10);
> +
> +                     /* If the lock file is older than 10s, it's a
> +                        leftover from some aborted test */
> +                     if (stat(LITEST_UDEV_LOCKFILE, &st) != -1) {
> +                             if (st.st_mtime < now - 10) {

I suppose making sure the test case runs fine when changing between
summer time/not summer time is unimportant enough to care about, but I
suspect it could make this check fail.

This and the next one is Reviewed-by: Jonas Ådahl <[email protected]>


Jonas

> +                                     fprintf(stderr,
> +                                             "Removing stale lock file 
> %s.\n",
> +                                             LITEST_UDEV_LOCKFILE);
> +                                     unlink(LITEST_UDEV_LOCKFILE);
> +                             }
> +                     }
> +             }
> +     } while (lfd < 0);
> +
> +     return lfd;
> +}
> +
> +static inline void
> +delete_udev_lock_file(int lfd)
> +{
> +     close(lfd);
> +     unlink(LITEST_UDEV_LOCKFILE);
> +}
> +
>  struct litest_device *
>  litest_add_device_with_overrides(struct libinput *libinput,
>                                enum litest_device_type which,
> @@ -1177,6 +1229,8 @@ litest_add_device_with_overrides(struct libinput 
> *libinput,
>       int rc;
>       const char *path;
>  
> +     int lfd = create_udev_lock_file();
> +
>       d = litest_create(which,
>                         name_override,
>                         id_override,
> @@ -1202,6 +1256,9 @@ litest_add_device_with_overrides(struct libinput 
> *libinput,
>               d->interface->min[ABS_Y] = libevdev_get_abs_minimum(d->evdev, 
> ABS_Y);
>               d->interface->max[ABS_Y] = libevdev_get_abs_maximum(d->evdev, 
> ABS_Y);
>       }
> +
> +     delete_udev_lock_file(lfd);
> +
>       return d;
>  }
>  
> @@ -1258,9 +1315,13 @@ litest_handle_events(struct litest_device *d)
>  void
>  litest_delete_device(struct litest_device *d)
>  {
> +     int lfd;
> +
>       if (!d)
>               return;
>  
> +     lfd = create_udev_lock_file();
> +
>       if (d->udev_rule_file) {
>               unlink(d->udev_rule_file);
>               free(d->udev_rule_file);
> @@ -1278,6 +1339,8 @@ litest_delete_device(struct litest_device *d)
>       free(d->private);
>       memset(d,0, sizeof(*d));
>       free(d);
> +
> +     delete_udev_lock_file(lfd);
>  }
>  
>  void
> -- 
> 2.7.4
> 
> _______________________________________________
> wayland-devel mailing list
> [email protected]
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel
_______________________________________________
wayland-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Reply via email to