[PATCH 2/3] tests: remove assert from frame_callback_wait()

[Pekka Paalanen]

On Mon, 26 May 2014 16:58:06 +0200
Marek Chalupa <mchqwerty at gmail.com> wrote:

> This function did dispatching wrapped in assert so when an error came,
> the test was aborted. Now, with expect_protocol_error, we need
> this function to abort only when we are not calling expect_protocol_error
> afterwards. So instead of calling the assert inside of this function,
> wrap this function into assert in appropriate places.
> ---
>  tests/event-test.c                | 6 +++---
>  tests/weston-test-client-helper.c | 7 +++++--
>  tests/weston-test-client-helper.h | 2 +-
>  3 files changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/tests/event-test.c b/tests/event-test.c
> index 980dfaa..facce49 100644
> --- a/tests/event-test.c
> +++ b/tests/event-test.c
> @@ -395,7 +395,7 @@ TEST(buffer_release)
>  	wl_surface_attach(surface, buf2, 0, 0);
>  	frame_callback_set(surface, &frame);
>  	wl_surface_commit(surface);
> -	frame_callback_wait(client, &frame);
> +	assert(frame_callback_wait(client, &frame));
>  	assert(buf1_released == 0);
>  	/* buf2 may or may not be released */
>  	assert(buf3_released == 0);
> @@ -403,7 +403,7 @@ TEST(buffer_release)
>  	wl_surface_attach(surface, buf3, 0, 0);
>  	frame_callback_set(surface, &frame);
>  	wl_surface_commit(surface);
> -	frame_callback_wait(client, &frame);
> +	assert(frame_callback_wait(client, &frame));
>  	assert(buf1_released == 0);
>  	assert(buf2_released == 1);
>  	/* buf3 may or may not be released */
> @@ -411,7 +411,7 @@ TEST(buffer_release)
>  	wl_surface_attach(surface, client->surface->wl_buffer, 0, 0);
>  	frame_callback_set(surface, &frame);
>  	wl_surface_commit(surface);
> -	frame_callback_wait(client, &frame);
> +	assert(frame_callback_wait(client, &frame));
>  	assert(buf1_released == 0);
>  	assert(buf2_released == 1);
>  	assert(buf3_released == 1);
> diff --git a/tests/weston-test-client-helper.c b/tests/weston-test-client-helper.c
> index fb2e477..f0b1b06 100644
> --- a/tests/weston-test-client-helper.c
> +++ b/tests/weston-test-client-helper.c
> @@ -80,12 +80,15 @@ frame_callback_set(struct wl_surface *surface, int *done)
>  	return callback;
>  }
>  
> -void
> +int
>  frame_callback_wait(struct client *client, int *done)
>  {
>  	while (!*done) {
> -		assert(wl_display_dispatch(client->wl_display) >= 0);
> +		if (wl_display_dispatch(client->wl_display) < 0)
> +			return 0;
>  	}
> +
> +	return 1;
>  }
>  
>  void
> diff --git a/tests/weston-test-client-helper.h b/tests/weston-test-client-helper.h
> index f154661..18c9e39 100644
> --- a/tests/weston-test-client-helper.h
> +++ b/tests/weston-test-client-helper.h
> @@ -120,7 +120,7 @@ move_client(struct client *client, int x, int y);
>  struct wl_callback *
>  frame_callback_set(struct wl_surface *surface, int *done);
>  
> -void
> +int
>  frame_callback_wait(struct client *client, int *done);
>  

Hi,

I think you may have missed the frame_callback_wait() in
move_client().

If a test is calling frame_callback_wait() without assert() and
without expect_protocol_error(), and simply returns from the test
function, do we still get a test failure if there was any error?

Maybe it would be better to leave frame_callback_wait() as is, and
add a new function frame_callback_wait_nofail() or something that
would not cause the test to fail if it hits an error from
wl_display. The change would be smaller to the test code, and
uncaught uses of the old function would not be silent false-passes.


Thanks,
pq