[PATCH 1/3] tests: add expect_protocol_error function

[Marek Chalupa]

On 19 June 2014 04:37, Bryce W. Harrington <b.harrington at samsung.com> wrote:

> On Mon, May 26, 2014 at 04:58:05PM +0200, Marek Chalupa wrote:
> > This function checks if a particular protocol error came in wire.
> > It's usefull in the cases where we hitherto used FAIL_TEST.
> > The problem with FAIL_TEST is that *any* assert will pass the test,
> > but we want only some asserts to pass the test (i. e. we don't
> > want the test to pass when it, for example, can't connect to display).
> > FAIL_TESTs are good only for sanity testing.
> >
> > The expect_protocol_error allows us to turn all FAIL_TESTs to TESTs
> > as will be introduced in following patches.
> > ---
> >  tests/weston-test-client-helper.c | 49
> +++++++++++++++++++++++++++++++++++++++
> >  tests/weston-test-client-helper.h |  4 ++++
> >  2 files changed, 53 insertions(+)
> >
> > diff --git a/tests/weston-test-client-helper.c
> b/tests/weston-test-client-helper.c
> > index 186b395..fb2e477 100644
> > --- a/tests/weston-test-client-helper.c
> > +++ b/tests/weston-test-client-helper.c
> > @@ -26,6 +26,7 @@
> >  #include <stdio.h>
> >  #include <string.h>
> >  #include <unistd.h>
> > +#include <errno.h>
> >  #include <sys/mman.h>
> >
> >  #include "../shared/os-compatibility.h"
> > @@ -512,6 +513,54 @@ skip(const char *fmt, ...)
> >       exit(77);
> >  }
> >
> > +void
> > +expect_protocol_error(struct client *client,
> > +                   const struct wl_interface *intf,
> > +                   uint32_t code)
> > +{
> > +     int err;
> > +     uint32_t errcode, failed = 0;
> > +     const struct wl_interface *interface;
> > +     unsigned int id;
> > +
> > +     /* if the error has not came yet, make it happen */
>
> "has not come yet"
>

> > +     wl_display_roundtrip(client->wl_display);
> > +
> > +     err = wl_display_get_error(client->wl_display);
> > +
> > +     assert(err && "Expected protocol error but nothing came");
> > +     assert(err == EPROTO && "Expected protocol error but got local
> error");
> > +
> > +
> > +     errcode = wl_display_get_protocol_error(client->wl_display,
> > +                                             &interface, &id);
> > +
> > +     /* check error */
> > +     if (errcode != code) {
> > +             fprintf(stderr, "Should get error code %d but got %d\n",
> > +                     code, errcode);
> > +             failed = 1;
> > +     }
> > +
> > +     /* this should be definitely set */
> > +     assert(interface);
> > +
> > +     if (strcmp(intf->name, interface->name) != 0) {
> > +             fprintf(stderr, "Should get interface '%s' but got '%s'\n",
> > +                     intf->name, interface->name);
> > +             failed = 1;
> > +     }
> > +
> > +     if (failed) {
> > +             fprintf(stderr, "Expected other protocol error\n");
> > +             abort();
> > +     }
> > +
> > +     /* all OK */
> > +     fprintf(stderr, "Got expected protocol error on '%s' (object id:
> %d) "
> > +                     "with code %d\n", interface->name, id, errcode);
> > +}
> > +
>
> I think if you move the assert higher, you can condense the logic a
> bit.  Maybe it's clearer:


>         /* this should be definitely set */
>         assert(interface);
>
>         if (errcode != code) {
>                 fprintf(stderr, "Should get error code %d but got %d\n",
>                         code, errcode);
>         } else if (strcmp(intf->name, interface->name) != 0) {
>                 fprintf(stderr, "Should get interface '%s' but got '%s'\n",
>                         intf->name, interface->name);
>         } else {
>                 fprintf(stderr, "Got expected protocol error on '%s'
> (object id: %d) "
>                         "with code %d\n", interface->name, id, errcode);
>                 return;
>         }
>
>         fprintf(stderr, "Expected a different protocol error\n");
>         abort();
> }
>
> I wonder if the final error message is a bit redundant.
>

Maybe, but consider this output (subsurface test):

test-client: got global pointer 45 75
test-client: got keyboard keymap
test-client: got surface enter output 0x156e850
libwayland: wl_subcompositor at 11: error 0: get_subsurface: wl_subsurface at 21:
wl_surface at 15 is already a sub-surface
Got expected protocol error on 'wl_subcompositor' (object id: 11) with code
0
test "test_subsurface_change_link":     exit status 0, pass.
test-client: got global pointer 45 75
test-client: got keyboard keymap
test-client: got surface enter output 0x156e850
libwayland: wl_subcompositor at 13: error 0: get_subsurface: wl_subsurface at 20:
wl_surface at 14 is already a sub-surface
Got expected protocol error on 'wl_subcompositor' (object id: 13) with code
0

Without the message about the expected error it would look like the test is
wrong,
since there's message from libwayland about an error and the test passes.


>
> >  static void
> >  log_handler(const char *fmt, va_list args)
> >  {
> > diff --git a/tests/weston-test-client-helper.h
> b/tests/weston-test-client-helper.h
> > index 4bfc3fa..f154661 100644
> > --- a/tests/weston-test-client-helper.h
> > +++ b/tests/weston-test-client-helper.h
> > @@ -129,4 +129,8 @@ get_n_egl_buffers(struct client *client);
> >  void
> >  skip(const char *fmt, ...);
> >
> > +void
> > +expect_protocol_error(struct client *client,
> > +                   const struct wl_interface *intf, uint32_t code);
> > +
> >  #endif
>
>
> Reviewed-by: Bryce Harrington <b.harrington at samsung.com>
>


Thank you,
Marek
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20140620/9afde977/attachment.html>