[Xcb] [PATCH libxkbcommon 2/2 v2] x11: add a couple of tests

[Uli Schlachter]

On 15.01.2014 23:35, Ran Benita wrote:
[...]
> diff --git a/test/interactive-x11.c b/test/interactive-x11.c
> new file mode 100644
> index 0000000..09fec32
> --- /dev/null
> +++ b/test/interactive-x11.c
> @@ -0,0 +1,347 @@
[...]
> +/*
> + * Note: This program only handles the core keyboard device for now.
> + * It should be straigtforward to change struct keyboard to a list of
> + * keyboards with device IDs, as in test/interactive-x11.c. This would
> + * require:

This comment is a bit self referential. I hope you do not really mean
interactive-x11.c, but I do not know what is really meant.

[...]
> +static int
> +loop(xcb_connection_t *conn, struct keyboard *kbd)
> +{
> +    while (!terminate) {

Please add:

  if (xcb_connection_has_error(conn)) {
     fprintf(stderr, "Some good error message here (error: %d)\n",
        xcb_connection_has_error(conn));
     break;
  }

> +        xcb_generic_event_t *event = xcb_wait_for_event(conn);
> +        process_event(event, kbd);
> +        free(event);
> +    }
> +
> +    return 0;
> +}
> +
> +static int
> +create_capture_window(xcb_connection_t *conn)
> +{
> +    xcb_generic_error_t *error;
> +    xcb_void_cookie_t cookie;
> +    xcb_screen_t *screen =
> +        xcb_setup_roots_iterator(xcb_get_setup(conn)).data;
> +    xcb_window_t window = xcb_generate_id(conn);
> +    uint32_t values[2] = {
> +        screen->white_pixel,
> +        XCB_EVENT_MASK_KEY_PRESS,
> +    };
> +
> +    cookie = xcb_create_window_checked(conn, XCB_COPY_FROM_PARENT,
> +                                       window, screen->root,
> +                                       10, 10, 100, 100, 1,
> +                                       XCB_WINDOW_CLASS_INPUT_OUTPUT,
> +                                       screen->root_visual,
> +                                       XCB_CW_BACK_PIXEL | XCB_CW_EVENT_MASK,
> +                                       values);
> +    if ((error = xcb_request_check(conn, cookie)) != NULL) {
> +        free(error);
> +        return -1;
> +    }
> +
> +    cookie = xcb_map_window_checked(conn, window);
> +    if ((error = xcb_request_check(conn, cookie)) != NULL) {
> +        free(error);
> +        return -1;
> +    }
> +
> +    xcb_flush(conn);

This flush doesn't do anything (xcb_request_check() will already have flushed
both requests).

> +    return 0;
> +}
> +
> +int
> +main(int argc, char *argv[])
> +{
> +    int ret;
> +    xcb_connection_t *conn;
> +    uint8_t first_xkb_event;
> +    int32_t core_kbd_device_id;
> +    struct xkb_context *ctx;
> +    struct keyboard core_kbd;
> +
> +    setlocale(LC_ALL, "");
> +
> +    conn = xcb_connect(NULL, NULL);
> +    if (!conn) {
> +        fprintf(stderr, "Couldn't connect to X server\n");
> +        ret = -1;
> +        goto err_out;
> +    }

xcb_connect() never returns NULL (but leave this in just to make sure). You are
looking for xcb_connection_has_error() (and it would be nice to also print the
error number that it returns).

[...]
> diff --git a/test/x11.c b/test/x11.c
> new file mode 100644
> index 0000000..6ab3865
> --- /dev/null
> +++ b/test/x11.c
> @@ -0,0 +1,78 @@
[...]
> +#include "test.h"
> +#include "xkbcommon/xkbcommon-x11.h"
> +
> +int
> +main(void)
> +{
> +    struct xkb_context *ctx = test_get_context(0);
> +    xcb_connection_t *conn;
> +    int ret;
> +    int32_t device_id;
> +    struct xkb_keymap *keymap;
> +    struct xkb_state *state;
> +    char *dump;
> +
> +    /*
> +    * The next two steps depend on a running X server with XKB support.
> +    * 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);
> +    if (!conn)
> +        return SKIP_TEST;

Again a missing xcb_connection_has_error().

[...]

Cheers,
Uli
-- 
"Are you preparing for another war, Plutarch?" I ask.
"Oh, not now. Now we're in that sweet period where
everyone agrees that our recent horrors should never
be repeated," he says. "But collective thinking is
usually short-lived. We're fickle, stupid beings with
poor memories and a great gift for self-destruction.