Test: Use a xvfb wrapper for x11 test

.github/workflows/linux.yml

@@ -31,6 +31,19 @@ jobs:
           sudo apt install -y \
             doxygen libxcb-xkb-dev valgrind ninja-build \
             libwayland-dev wayland-protocols bison graphviz
+      - name: Install xkeyboard-config
+        run: |
+          # Install master version of xkeyboard-config, in order to ensure
+          # its latest version works well with xkbcommon.
+          # HACK: We use meson to install, while it would be cleaner
+          #       to create a proper package to install or use some PPA.
+          pushd ~
+          git clone --depth=1 https://gitlab.freedesktop.org/xkeyboard-config/xkeyboard-config.git
+          cd "xkeyboard-config"
+          BUILDDIR=build
+          meson setup $BUILDDIR -Dprefix=/usr
+          meson install -C $BUILDDIR
+          popd
       - name: Setup
         run: |
           # -gdwarf-4 - see https://github.com/llvm/llvm-project/issues/56550.

meson.build

@@ -573,9 +573,11 @@ test_dep = declare_dependency(
     link_with: libxkbcommon_test_internal,
 )
 if get_option('enable-x11')
-    libxkbcommon_x11_internal = static_library(
+    libxkbcommon_x11_test_internal = static_library(
         'xkbcommon-x11-internal',
         libxkbcommon_x11_sources,
+        'test/xvfb-wrapper.c',
+        'test/xvfb-wrapper.h',
         include_directories: include_directories('src', 'include'),
         link_with: libxkbcommon_test_internal,
         dependencies: [
@@ -584,7 +586,7 @@ if get_option('enable-x11')
         ],
     )
     x11_test_dep = declare_dependency(
-        link_with: libxkbcommon_x11_internal,
+        link_with: libxkbcommon_x11_test_internal,
         dependencies: [
             test_dep,
             xcb_dep,
@@ -689,9 +691,11 @@ if get_option('enable-x11')
         executable('test-x11', 'test/x11.c', dependencies: x11_test_dep),
         env: test_env,
     )
-    # test/x11comp is meant to be run, but it is (temporarily?) disabled.
-    # See: https://github.com/xkbcommon/libxkbcommon/issues/30
-    executable('test-x11comp', 'test/x11comp.c', dependencies: x11_test_dep)
+    test(
+        'x11comp',
+        executable('test-x11comp', 'test/x11comp.c', dependencies: x11_test_dep),
+        env: test_env,
+    )
 endif
 if get_option('enable-xkbregistry')
     test(

test/x11.c

@@ -24,10 +24,10 @@
 #include "config.h"
 
 #include "test.h"
+#include "xvfb-wrapper.h"
 #include "xkbcommon/xkbcommon-x11.h"
 
-int
-main(void)
+X11_TEST(test_basic)
 {
     struct xkb_context *ctx = test_get_context(0);
     xcb_connection_t *conn;
@@ -43,7 +43,7 @@ main(void)
     * If it fails, it's not necessarily an actual problem with the code.
     * So we don't want a FAIL here.
     */
-    conn = xcb_connect(NULL, NULL);
+    conn = xcb_connect(display, NULL);
     if (!conn || xcb_connection_has_error(conn)) {
         exit_code = SKIP_TEST;
         goto err_conn;
@@ -84,3 +84,7 @@ main(void)
 
     return exit_code;
 }
+
+int main(void) {
+    return x11_tests_run();
+}

test/x11comp.c

@@ -32,78 +32,27 @@
 #include <sys/wait.h>
 
 #include "test.h"
+#include "xvfb-wrapper.h"
 #include "xkbcommon/xkbcommon-x11.h"
 
-int
-main(void)
+X11_TEST(test_basic)
 {
-    struct xkb_context *ctx = test_get_context(0);
     struct xkb_keymap *keymap;
     xcb_connection_t *conn;
     int32_t device_id;
     int ret, status;
-    char display[512];
     char *xkb_path;
     char *original, *dump;
     char *envp[] = { NULL };
-    char *xvfb_argv[] = {
-        (char *) "Xvfb", display, NULL
-    };
-    pid_t xvfb_pid = 0;
     char *xkbcomp_argv[] = {
         (char *) "xkbcomp", (char *) "-I", NULL /* xkb_path */, display, NULL
     };
     pid_t xkbcomp_pid;
 
-    char *xhost = NULL;
-    int xdpy_current;
-    int xdpy_candidate;
-
-    /*
-     * What all of this mess does is:
-     * 1. Launch Xvfb on available DISPLAY.
-     * 2. Make an xcb connection to this display.
-     * 3. Launch xkbcomp to change the keymap of the new display (doing
-     *    this programmatically is major work [which we may yet do some
-     *    day for xkbcommon-x11] so we use xkbcomp for now).
-     * 4. Download the keymap back from the display using xkbcommon-x11.
-     * 5. Compare received keymap to the uploaded keymap.
-     * 6. Kill the server & clean up.
-     */
-
-    ret = xcb_parse_display(NULL, &xhost, &xdpy_current, NULL);
-    assert(ret != 0);
-    /*
-     * IANA assigns TCP port numbers from 6000 through 6063 to X11
-     * clients.  In addition, the current XCB implementaion shows
-     * that, when an X11 client tries to establish a TCP connetion,
-     * the port number needed is specified by adding 6000 to a given
-     * display number.  So, one of reasonable ranges of xdpy_candidate
-     * is [0, 63].
-     */
-    for (xdpy_candidate = 63; xdpy_candidate >= 0; xdpy_candidate--) {
-        if (xdpy_candidate == xdpy_current) {
-            continue;
-        }
-        snprintf(display, sizeof(display), "%s:%d", xhost, xdpy_candidate);
-        ret = posix_spawnp(&xvfb_pid, "Xvfb", NULL, NULL, xvfb_argv, envp);
-        if (ret == 0) {
-            break;
-        }
-    }
-    free(xhost);
-    if (ret != 0) {
-        ret = SKIP_TEST;
-        goto err_ctx;
-    }
-
-    /* Wait for Xvfb fully waking up to accept a connection from a client. */
-    sleep(1);
-
     conn = xcb_connect(display, NULL);
     if (xcb_connection_has_error(conn)) {
         ret = SKIP_TEST;
-        goto err_xvfd;
+        goto err_conn;
     }
     ret = xkb_x11_setup_xkb_extension(conn,
                                       XKB_X11_MIN_MAJOR_XKB_VERSION,
@@ -132,6 +81,7 @@ main(void)
         goto err_xcb;
     }
 
+    struct xkb_context *ctx = test_get_context(0);
     keymap = xkb_x11_keymap_new_from_device(ctx, conn, device_id,
                                             XKB_KEYMAP_COMPILE_NO_FLAGS);
     assert(keymap);
@@ -159,12 +109,13 @@ main(void)
     free(original);
     free(dump);
     xkb_keymap_unref(keymap);
+    xkb_context_unref(ctx);
 err_xcb:
     xcb_disconnect(conn);
-err_xvfd:
-    if (xvfb_pid > 0)
-        kill(xvfb_pid, SIGTERM);
-err_ctx:
-    xkb_context_unref(ctx);
+err_conn:
     return ret;
 }
+
+int main(void) {
+    return x11_tests_run();
+}

test/xvfb-wrapper.c

@@ -0,0 +1,164 @@
+/*
+ * Copyright © 2014 Ran Benita <[email protected]>
+ * Copyright © 2023 Pierre Le Marre <[email protected]>
+ *
+ * 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 <stdio.h>
+#include <spawn.h>
+#include <assert.h>
+#include <signal.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+
+#include "test.h"
+#include "xvfb-wrapper.h"
+#include "xkbcommon/xkbcommon-x11.h"
+
+static bool xvfb_is_ready;
+
+static void
+sigusr1_handler(int signal)
+{
+    xvfb_is_ready = true;
+}
+
+int
+xvfb_wrapper(int (*test_func)(char* display))
+{
+    int ret = 0;
+    FILE * display_fd;
+    char display_fd_string[32];
+    sigset_t mask;
+    struct sigaction sa;
+    char *xvfb_argv[] = {
+        (char *) "Xvfb", (char *) "-displayfd", display_fd_string, NULL
+    };
+    char *envp[] = { NULL };
+    pid_t xvfb_pid = 0;
+    size_t counter = 0;
+    char display[32] = ":";
+    size_t length;
+
+    /* File descriptor to retrieve the display number */
+    display_fd = tmpfile();
+    if (display_fd == NULL){
+        fprintf(stderr, "Unable to create temporary file.\n");
+        goto err_display_fd;
+    }
+    snprintf(display_fd_string, sizeof(display_fd_string), "%d", fileno(display_fd));
+
+    /* Set SIGUSR1 to SIG_IGN so Xvfb will send us that signal
+     * when it's ready to accept connections */
+    sigemptyset(&mask);
+    sigaddset(&mask, SIGUSR1);
+    sigprocmask(SIG_BLOCK, &mask, NULL);
+    sa.sa_handler = SIG_IGN;
+    sa.sa_flags = 0;
+    sigemptyset(&sa.sa_mask);
+    sigaction(SIGUSR1, &sa, NULL);
+
+    xvfb_is_ready = false;
+
+    /*
+     * Xvfb command: let the server find an available display.
+     *
+     * Note that it may generate multiple times the following output in stderr:
+     *    _XSERVTransSocketUNIXCreateListener: ...SocketCreateListener() failed
+     * It is expected: this is the server trying the ports until it finds one
+     * that works.
+     */
+    ret = posix_spawnp(&xvfb_pid, "Xvfb", NULL, NULL, xvfb_argv, envp);
+    if (ret != 0) {
+        ret = SKIP_TEST;
+        goto err_xvfd;
+    }
+
+    sa.sa_handler = SIG_DFL;
+    sa.sa_flags = 0;
+    sigemptyset(&sa.sa_mask);
+    sigaction(SIGUSR1, &sa, NULL);
+    signal(SIGUSR1, sigusr1_handler);
+    sigprocmask (SIG_UNBLOCK, &mask, NULL);
+
+    /* Now wait for the SIGUSR1 signal that Xvfb is ready */
+    while (!xvfb_is_ready) {
+        usleep(1000);
+        if (++counter >= 3000) /* 3 seconds max wait */
+            break;
+    }
+
+    signal(SIGUSR1, SIG_DFL);
+
+    /* Retrieve the display number: Xvfd writes the display number as a newline-
+     * terminated string; copy this number to form a proper display string. */
+    rewind(display_fd);
+    length = fread(&display[1], 1, sizeof(display) - 1, display_fd);
+    if (length <= 0) {
+        ret = SKIP_TEST;
+        goto err_xvfd;
+    } else {
+        /* Drop the newline character */
+        display[length] = '\0';
+    }
+
+    /* Run the function requiring a running X server */
+    ret = test_func(display);
+
+err_xvfd:
+    if (xvfb_pid > 0)
+        kill(xvfb_pid, SIGTERM);
+    fclose(display_fd);
+err_display_fd:
+    return ret;
+}
+
+/* All X11_TEST functions are in the test_functions_section ELF section.
+ * __start and __stop point to the start and end of that section. See the
+ * __attribute__(section) documentation.
+ */
+extern const struct test_function __start_test_functions_section, __stop_test_functions_section;
+
+int
+x11_tests_run()
+{
+    size_t count = 1; /* For NULL-terminated entry */
+
+    for (const struct test_function *t = &__start_test_functions_section;
+         t < &__stop_test_functions_section;
+         t++)
+        count++;
+
+    int rc;
+    for (const struct test_function *t = &__start_test_functions_section;
+         t < &__stop_test_functions_section;
+         t++) {
+        fprintf(stderr, "Running test: %s from %s\n", t->name, t->file);
+        rc = xvfb_wrapper(t->func);
+        if (rc != 0) {
+            break;
+        }
+    }
+
+    return rc;
+}