[PATCH 1/3] tests: add expect_protocol_error function

[Marek Chalupa]

Hi,

I fixed the comment as Bryce wrote (s/came/come) and fixed a white-space
error.
The logic of this version of the patch is equivalent to the old one.

Thanks,
Marek


On 7 July 2014 20:41, Bryce W. Harrington <b.harrington at samsung.com> wrote:

> On Mon, Jul 07, 2014 at 05:47:41PM +0300, Pekka Paalanen wrote:
> > On Thu, 19 Jun 2014 02:37:40 +0000
> > "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.
> > >
> > > >  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>
> >
> > Hi Bryce,
> >
> > is this a reviewed-by also for the original patch without
> > suggested changes?
>
> Yes
>
> > And only for patch 1?
>
> I didn't see any issues with patches 2 and 3.
>
> Bryce
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20140716/3ee4d669/attachment.html>