[PATCH weston 7/8] tests: Add test for keyboard key event timestamps

[Pekka Paalanen]

On Mon,  4 Dec 2017 15:34:07 +0200
Alexandros Frantzis <alexandros.frantzis at collabora.com> wrote:

> Add test to verify that the server correctly sets the timestamps of
> keyboard key events. This requires updating the weston-test protocol to
> support passing key event timestamps.
> 
> Signed-off-by: Alexandros Frantzis <alexandros.frantzis at collabora.com>
> ---
>  protocol/weston-test.xml          |  3 ++
>  tests/keyboard-test.c             | 58 +++++++++++++++++++++++++++++++--------
>  tests/weston-test-client-helper.c |  1 +
>  tests/weston-test-client-helper.h |  1 +
>  tests/weston-test.c               |  3 +-
>  5 files changed, 53 insertions(+), 13 deletions(-)
> 
> diff --git a/protocol/weston-test.xml b/protocol/weston-test.xml
> index a4a7ad4e..37fa221f 100644
> --- a/protocol/weston-test.xml
> +++ b/protocol/weston-test.xml
> @@ -64,6 +64,9 @@
>        <arg name="surface" type="object" interface="wl_surface" allow-null="true"/>
>      </request>
>      <request name="send_key">
> +      <arg name="tv_sec_hi" type="uint"/>
> +      <arg name="tv_sec_lo" type="uint"/>
> +      <arg name="tv_nsec" type="uint"/>
>        <arg name="key" type="uint"/>
>        <arg name="state" type="uint"/>
>      </request>
> diff --git a/tests/keyboard-test.c b/tests/keyboard-test.c
> index 6b4ba19d..df1940f8 100644
> --- a/tests/keyboard-test.c
> +++ b/tests/keyboard-test.c
> @@ -27,21 +27,45 @@
>  
>  #include <stdint.h>
>  
> +#include "shared/timespec-util.h"
>  #include "weston-test-client-helper.h"
>  
> +static const struct timespec t1 = { .tv_sec = 1, .tv_nsec = 1000001 };
> +static const struct timespec t2 = { .tv_sec = 2, .tv_nsec = 2000001 };
> +
> +static struct client *
> +create_client_with_keyboard_focus(void)
> +{
> +	struct client *cl = create_client_and_test_surface(10, 10, 1, 1);
> +	assert(cl);
> +
> +	weston_test_activate_surface(cl->test->weston_test,
> +				     cl->surface->wl_surface);
> +	client_roundtrip(cl);
> +
> +	return cl;
> +}
> +
> +static void
> +send_key(struct client *client, const struct timespec *time,
> +	 uint32_t key, uint32_t state)
> +{
> +	uint32_t tv_sec_hi, tv_sec_lo, tv_nsec;
> +
> +	timespec_to_proto(time, &tv_sec_hi, &tv_sec_lo, &tv_nsec);
> +	weston_test_send_key(client->test->weston_test, tv_sec_hi, tv_sec_lo,
> +			     tv_nsec, key, state);
> +	client_roundtrip(client);
> +}
> +
>  TEST(simple_keyboard_test)
>  {
> -	struct client *client;
> -	struct surface *expect_focus = NULL;
> -	struct keyboard *keyboard;
> +	struct client *client = create_client_with_keyboard_focus();
> +	struct keyboard *keyboard = client->input->keyboard;
> +	struct surface *expect_focus = client->surface;
>  	uint32_t expect_key = 0;
>  	uint32_t expect_state = 0;
>  
> -	client = create_client_and_test_surface(10, 10, 1, 1);
> -	assert(client);
> -
> -	keyboard = client->input->keyboard;
> -
>  	while (1) {
>  		assert(keyboard->key == expect_key);
>  		assert(keyboard->state == expect_state);
> @@ -49,8 +73,7 @@ TEST(simple_keyboard_test)
>  
>  		if (keyboard->state == WL_KEYBOARD_KEY_STATE_PRESSED) {
>  			expect_state = WL_KEYBOARD_KEY_STATE_RELEASED;
> -			weston_test_send_key(client->test->weston_test,
> -					     expect_key, expect_state);
> +			send_key(client, &t1, expect_key, expect_state);
>  		} else if (keyboard->focus) {
>  			expect_focus = NULL;
>  			weston_test_activate_surface(
> @@ -62,8 +85,7 @@ TEST(simple_keyboard_test)
>  			weston_test_activate_surface(
>  				client->test->weston_test,
>  				expect_focus->wl_surface);
> -			weston_test_send_key(client->test->weston_test,
> -					     expect_key, expect_state);
> +			send_key(client, &t1, expect_key, expect_state);
>  		} else {
>  			break;
>  		}
> @@ -71,3 +93,15 @@ TEST(simple_keyboard_test)
>  		client_roundtrip(client);
>  	}

The testing sequence in the while loop is changed. In two of the three
cases there is now a double-roundtrip. Before the test started with no
focus, now it starts with focus which means it first loses focus and
then continues on the old sequence.

Those are all subtle and apparently harmless changes, but also never
mentioned in the commit message.

I'd be more comfortable if you moved the existing roundtrip to the case
still needing it, avoiding the double-roundtrips, and justified the new
sequence in the commit message.

>  }
> +
> +TEST(keyboard_key_event_time)
> +{
> +	struct client *client = create_client_with_keyboard_focus();
> +	struct keyboard *keyboard = client->input->keyboard;
> +
> +	send_key(client, &t1, 0, WL_KEYBOARD_KEY_STATE_PRESSED);
> +	assert(keyboard->key_time == timespec_to_msec(&t1));
> +
> +	send_key(client, &t2, 0, WL_KEYBOARD_KEY_STATE_RELEASED);
> +	assert(keyboard->key_time == timespec_to_msec(&t2));

0 is defined a KEY_RESERVED, I think it would be better to use another
value just in case.


The rest looks fine.


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