[PATCH wayland v3] tests/queue-test: Add tests for proxy wrappers

[Pekka Paalanen]

On Thu, 28 Apr 2016 15:31:32 +0800
Jonas Ådahl <jadahl at gmail.com> wrote:

> Test that doing wl_display.sync on a wrapped proxy with a special queue
> works as expected.
> 
> Test that creating a wrapper on a destroyed but not freed proxy fails.
> 
> Signed-off-by: Jonas Ådahl <jadahl at gmail.com>
> ---
> 
> Changes since v1:
> 
>  - Made the test single threaded, instead pretending to be multi threaded.
>  - Added a negative-test for illustrating how NOT to do thread safe queue
>    assignment.
>  - Changed create-wrapper-failed test to be less confusing
> 
> Jonas

Hi Jonas,

much appreciated. :-)

>  tests/queue-test.c | 157 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 157 insertions(+)
> 
> diff --git a/tests/queue-test.c b/tests/queue-test.c
> index 02865ae..a6f82ac 100644
> --- a/tests/queue-test.c
> +++ b/tests/queue-test.c
> @@ -205,6 +205,127 @@ client_test_queue_roundtrip(void)
>  }
>  
>  static void
> +client_test_queue_proxy_wrapper(void)
> +{
> +	struct wl_event_queue *queue;
> +	struct wl_display *display;
> +	struct wl_display *display_wrapper;
> +	struct wl_callback *callback;
> +	bool done = false;
> +
> +	/*
> +	 * For an illustration of what usage would normally fail without using
> +	 * proxy wrappers, see the `client_test_queue_set_queue_race' test case.
> +	 */
> +
> +	display = wl_display_connect(NULL);
> +	assert(display);
> +
> +	/* Pretend we are in a separate thread where a thread-local queue is
> +	 * used. */
> +	queue = wl_display_create_queue(display);
> +	assert(queue);
> +
> +	display_wrapper = wl_proxy_create_wrapper(display);
> +	assert(display_wrapper);
> +	wl_proxy_set_queue((struct wl_proxy *) display_wrapper, queue);
> +	callback = wl_display_sync(display_wrapper);
> +	wl_proxy_wrapper_destroy(display_wrapper);
> +	assert(callback != NULL);
> +
> +	/* Pretend we are now another thread and dispatch the dispatch the main

-"dispatch the"

> +	 * queue while also knowing our callback is read and queued. */
> +	wl_display_roundtrip(display);
> +
> +	/* Make sure that the pretend-to-be main thread didn't dispatch our
> +	 * callback, behind our back. */
> +	wl_callback_add_listener(callback, &sync_listener_roundtrip, &done);
> +	wl_display_flush(display);

Is this flush needed for something?

> +
> +	assert(!done);
> +
> +	/* Make sure that we eventually end up dispatching our callback. */
> +	while (!done)
> +		assert(wl_display_dispatch_queue(display, queue) != -1);
> +
> +	wl_callback_destroy(callback);
> +	wl_event_queue_destroy(queue);
> +
> +	wl_display_disconnect(display);
> +}
> +
> +static void
> +client_test_queue_set_queue_race(void)
> +{
> +	struct wl_event_queue *queue;
> +	struct wl_display *display;
> +	struct wl_callback *callback;
> +	bool done = false;
> +
> +	/*
> +	 * This test illustrates the multi threading scenario which would fail
> +	 * without doing what is done in the `client_test_queue_proxy_wrapper'
> +	 * test.
> +	 */

I think this is crucial for understanding what the positive test is
testing for. Thank you for adding this. :-)

> +
> +	display = wl_display_connect(NULL);
> +	assert(display);
> +
> +	/* Pretend we are in a separate thread where a thread-local queue is
> +	 * used. */
> +	queue = wl_display_create_queue(display);
> +	assert(queue);
> +
> +	callback = wl_display_sync(display);
> +	assert(callback != NULL);
> +
> +	/* Pretend we are now another thread and dispatch the dispatch the main

-"dispatch the"

> +	 * queue while also knowing our callback is read, queued on the wrong
> +	 * queue, and dispatched. */
> +	wl_display_roundtrip(display);
> +
> +	/* Pretend we are back in the separate thread, and continue with setting
> +	 * up our callback. */
> +	wl_callback_add_listener(callback, &sync_listener_roundtrip, &done);
> +	wl_proxy_set_queue((struct wl_proxy *) callback, queue);
> +
> +	/* Roundtrip our separate thread queue to make sure any events are
> +	 * dispatched. */
> +	wl_display_roundtrip_queue(display, queue);
> +
> +	/* Verify that the callback has indeed been dropped. */
> +	assert(!done);
> +
> +	wl_callback_destroy(callback);
> +	wl_event_queue_destroy(queue);
> +
> +	wl_display_disconnect(display);
> +}
> +
> +static void
> +client_test_queue_wrap_destroyed_proxy(void)
> +{
> +	struct wl_display *display;
> +	struct wl_callback *callback;
> +	struct wl_callback *callback_wrapper;
> +
> +	display = wl_display_connect(NULL);
> +	assert(display);
> +
> +	callback = wl_display_sync(display);
> +	assert(callback);
> +
> +	wl_display_roundtrip(display);
> +
> +	/* The callback will now been removed. */
> +	callback_wrapper = wl_proxy_create_wrapper(callback);
> +	assert(!callback_wrapper);
> +
> +	wl_callback_destroy(callback);
> +	wl_display_disconnect(display);
> +}

Yes, this is exactly what I meant!

> +
> +static void
>  dummy_bind(struct wl_client *client,
>  	   void *data, uint32_t version, uint32_t id)
>  {
> @@ -259,3 +380,39 @@ TEST(queue_roundtrip)
>  
>  	display_destroy(d);
>  }
> +
> +TEST(queue_set_queue_proxy_wrapper)
> +{
> +	struct display *d = display_create();
> +
> +	test_set_timeout(2);
> +
> +	client_create_noarg(d, client_test_queue_proxy_wrapper);
> +	display_run(d);
> +
> +	display_destroy(d);
> +}
> +
> +TEST(queue_set_queue_race)
> +{
> +	struct display *d = display_create();
> +
> +	test_set_timeout(2);
> +
> +	client_create_noarg(d, client_test_queue_set_queue_race);
> +	display_run(d);
> +
> +	display_destroy(d);
> +}
> +
> +TEST(queue_wrap_destroyed_proxy)
> +{
> +	struct display *d = display_create();
> +
> +	test_set_timeout(2);
> +
> +	client_create_noarg(d, client_test_queue_wrap_destroyed_proxy);
> +	display_run(d);
> +
> +	display_destroy(d);
> +}

Apart from few cosmetic issues, excellent.

Reviewed-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 811 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20160428/145f48ef/attachment.sig>