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

[Ran Benita]

On Thu, Jan 16, 2014 at 06:13:48PM +0100, Uli Schlachter wrote:
> 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.

Oh.. I meant 'interactive-evdev.c', which is similar and uses evdev
devices directly. There we handle mutiple devices.

> [...]
> > +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;
>   }

This interface is not very intuitive, but thinking about it I can't
think of any other way XCB could have handled that. Fixed.

> > +        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).

OK, removed. Btw, I got this one from here:
http://xcb.freedesktop.org/windowcontextandmanipulation/
But there the result isn't checked, so I guess the flush there *is*
necessary.

> > +    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).

Fixed both of these. Wondered for a few minutes how that can possibly
work :)

Thanks again!
Ran

> [...]
> > 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.