Re: [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  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 
> ---
>  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 @@
> allow-null="true"/>
>  
>  
> +  
> +  
> +  
>
>
>  
> 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 
>  
> +#include "shared/timespec-util.h"
>  #include "weston-test-client-helper.h"
>  
> +static const struct timespec t1 = { .tv_sec = 1, .tv_nsec = 101 };
> +static const struct timespec t2 = { .tv_sec = 2, .tv_nsec = 201 };
> +
> +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, _sec_hi, _sec_lo, _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, , 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, , 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, , 0, WL_KEYBOARD_KEY_STATE_PRESSED);
> + assert(keyboard->key_time == timespec_to_msec());
> +
> + send_key(client, , 0, WL_KEYBOARD_KEY_STATE_RELEASED);
> + assert(keyboard->key_time == timespec_to_msec());

0 is defined a KEY_RESERVED, I think 

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

[Alexandros Frantzis]

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 
---
 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 @@
   
 
 
+  
+  
+  
   
   
 
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 
 
+#include "shared/timespec-util.h"
 #include "weston-test-client-helper.h"
 
+static const struct timespec t1 = { .tv_sec = 1, .tv_nsec = 101 };
+static const struct timespec t2 = { .tv_sec = 2, .tv_nsec = 201 };
+
+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, _sec_hi, _sec_lo, _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, , 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, , expect_key, expect_state);
} else {
break;
}
@@ -71,3 +93,15 @@ TEST(simple_keyboard_test)
client_roundtrip(client);
}
 }
+
+TEST(keyboard_key_event_time)
+{
+   struct client *client = create_client_with_keyboard_focus();
+   struct keyboard *keyboard = client->input->keyboard;
+
+   send_key(client, , 0, WL_KEYBOARD_KEY_STATE_PRESSED);
+   assert(keyboard->key_time == timespec_to_msec());
+
+   send_key(client, , 0, WL_KEYBOARD_KEY_STATE_RELEASED);
+   assert(keyboard->key_time == timespec_to_msec());
+}
diff --git a/tests/weston-test-client-helper.c 
b/tests/weston-test-client-helper.c
index 92def14d..ef58d77a 100644
--- a/tests/weston-test-client-helper.c
+++ b/tests/weston-test-client-helper.c
@@ -280,6 +280,7 @@ keyboard_handle_key(void *data, struct wl_keyboard 
*wl_keyboard,
 
keyboard->key = key;
keyboard->state = state;
+   keyboard->key_time = time;
 
fprintf(stderr, "test-client: got keyboard key %u %u\n", key, state);
 }
diff --git a/tests/weston-test-client-helper.h 
b/tests/weston-test-client-helper.h
index 1b4d83c7..1be727c1 100644
--- a/tests/weston-test-client-helper.h
+++ b/tests/weston-test-client-helper.h
@@ -111,6 +111,7 @@ struct keyboard {
int rate;
int delay;
} repeat_info;
+   uint32_t key_time;
 };
 
 struct touch {