<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On 7 July 2014 17:22, Pekka Paalanen <span dir="ltr"><<a href="mailto:ppaalanen@gmail.com" target="_blank">ppaalanen@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div class=""><div class="h5">On Mon, 26 May 2014 16:58:06 +0200<br>
Marek Chalupa <<a href="mailto:mchqwerty@gmail.com">mchqwerty@gmail.com</a>> wrote:<br>
<br>
> This function did dispatching wrapped in assert so when an error came,<br>
> the test was aborted. Now, with expect_protocol_error, we need<br>
> this function to abort only when we are not calling expect_protocol_error<br>
> afterwards. So instead of calling the assert inside of this function,<br>
> wrap this function into assert in appropriate places.<br>
> ---<br>
> tests/event-test.c | 6 +++---<br>
> tests/weston-test-client-helper.c | 7 +++++--<br>
> tests/weston-test-client-helper.h | 2 +-<br>
> 3 files changed, 9 insertions(+), 6 deletions(-)<br>
><br>
> diff --git a/tests/event-test.c b/tests/event-test.c<br>
> index 980dfaa..facce49 100644<br>
> --- a/tests/event-test.c<br>
> +++ b/tests/event-test.c<br>
> @@ -395,7 +395,7 @@ TEST(buffer_release)<br>
> wl_surface_attach(surface, buf2, 0, 0);<br>
> frame_callback_set(surface, &frame);<br>
> wl_surface_commit(surface);<br>
> - frame_callback_wait(client, &frame);<br>
> + assert(frame_callback_wait(client, &frame));<br>
> assert(buf1_released == 0);<br>
> /* buf2 may or may not be released */<br>
> assert(buf3_released == 0);<br>
> @@ -403,7 +403,7 @@ TEST(buffer_release)<br>
> wl_surface_attach(surface, buf3, 0, 0);<br>
> frame_callback_set(surface, &frame);<br>
> wl_surface_commit(surface);<br>
> - frame_callback_wait(client, &frame);<br>
> + assert(frame_callback_wait(client, &frame));<br>
> assert(buf1_released == 0);<br>
> assert(buf2_released == 1);<br>
> /* buf3 may or may not be released */<br>
> @@ -411,7 +411,7 @@ TEST(buffer_release)<br>
> wl_surface_attach(surface, client->surface->wl_buffer, 0, 0);<br>
> frame_callback_set(surface, &frame);<br>
> wl_surface_commit(surface);<br>
> - frame_callback_wait(client, &frame);<br>
> + assert(frame_callback_wait(client, &frame));<br>
> assert(buf1_released == 0);<br>
> assert(buf2_released == 1);<br>
> assert(buf3_released == 1);<br>
> diff --git a/tests/weston-test-client-helper.c b/tests/weston-test-client-helper.c<br>
> index fb2e477..f0b1b06 100644<br>
> --- a/tests/weston-test-client-helper.c<br>
> +++ b/tests/weston-test-client-helper.c<br>
> @@ -80,12 +80,15 @@ frame_callback_set(struct wl_surface *surface, int *done)<br>
> return callback;<br>
> }<br>
><br>
> -void<br>
> +int<br>
> frame_callback_wait(struct client *client, int *done)<br>
> {<br>
> while (!*done) {<br>
> - assert(wl_display_dispatch(client->wl_display) >= 0);<br>
> + if (wl_display_dispatch(client->wl_display) < 0)<br>
> + return 0;<br>
> }<br>
> +<br>
> + return 1;<br>
> }<br>
><br>
> void<br>
> diff --git a/tests/weston-test-client-helper.h b/tests/weston-test-client-helper.h<br>
> index f154661..18c9e39 100644<br>
> --- a/tests/weston-test-client-helper.h<br>
> +++ b/tests/weston-test-client-helper.h<br>
> @@ -120,7 +120,7 @@ move_client(struct client *client, int x, int y);<br>
> struct wl_callback *<br>
> frame_callback_set(struct wl_surface *surface, int *done);<br>
><br>
> -void<br>
> +int<br>
> frame_callback_wait(struct client *client, int *done);<br>
><br>
<br>
</div></div>Hi,<br>
<br>
I think you may have missed the frame_callback_wait() in<br>
move_client().<br></blockquote><div><br></div><div>Yes<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
If a test is calling frame_callback_wait() without assert() and<br>
without expect_protocol_error(), and simply returns from the test<br>
function, do we still get a test failure if there was any error?<br>
<br></blockquote><div><br></div><div>No. There's no way how to fail the test without manually checking for<br>errors, because the wl_display object is not accessible to the test-runner and if<br>the test is not aborted from inside, the test_run function returns EXIT_SUCCESS.<br>
So if programmer doesn't check for errors at the end of the test, the test<br></div><div>won't fail if there's an error.<br>Here in libwayland: <a href="http://lists.freedesktop.org/archives/wayland-devel/2014-May/015096.html">http://lists.freedesktop.org/archives/wayland-devel/2014-May/015096.html</a><br>
</div><div>I solved it by checking for errors in destroying client (in client_disconnect function).<br><br></div><div>Another solution is to return EXIT_FAILURE (or another non-zero value) from the test_run (<a href="http://cgit.freedesktop.org/wayland/weston/tree/tests/weston-test-runner.c#n55">http://cgit.freedesktop.org/wayland/weston/tree/tests/weston-test-runner.c#n55</a>)<br>
which would force the programmers to call exit(0) upon successful end of test. This would require to rewrite (add exit(0) at the end of) all the tests, though.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
Maybe it would be better to leave frame_callback_wait() as is, and<br>
add a new function frame_callback_wait_nofail() or something that<br>
would not cause the test to fail if it hits an error from<br>
wl_display. The change would be smaller to the test code, and<br>
uncaught uses of the old function would not be silent false-passes.<br>
<br></blockquote><div><br></div><div>Yea, maybe it's better.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Thanks,<br>
pq<br>
</blockquote></div><br></div><div class="gmail_extra">Thanks,<br>Marek<br></div></div>