[mutter/wip/smcv/disoriented: 1/4] tests: Change how we wait for an orientation change

[Simon McVittie <smcv src gnome org>]

commit 615633c187af0ed831107f4298ae3555e0d73060
Author: Simon McVittie <smcv debian org>
Date:   Sun Oct 10 12:31:47 2021 +0100

    tests: Change how we wait for an orientation change
    
    Previously, we were waiting up to 300ms for the signal, then proceeding
    anyway. However, 300ms is not necessarily long enough to wait on an
    autobuilder that might be heavily loaded, particularly if it's a non-x86
    with different performance characteristics.
    
    Conversely, if mutter responds to the D-Bus signal from the mock sensor
    before we have connected to the signal, then we cannot expect to receive
    the signal - it was already emitted, but we missed it. In this case, we
    need to avoid waiting.
    
    One remaining use of wait_for_orientation_changes() that would previously
    always have timed out was in
    meta_test_orientation_manager_has_accelerometer(), which does not
    actually expect to see an orientation-changed signal. Make this wait
    for the accelerometer to be detected instead.
    
    Resolves: https://gitlab.gnome.org/GNOME/mutter/-/issues/1967
    Bug-Debian: https://bugs.debian.org/995929
    Signed-off-by: Simon McVittie <smcv debian org>

 src/tests/monitor-unit-tests.c             | 107 +++++++++++-----------
 src/tests/orientation-manager-unit-tests.c | 142 ++++++++++++++++++++++++-----
 src/tests/orientation-manager-unit-tests.h |   8 +-
 3 files changed, 176 insertions(+), 81 deletions(-)
---
diff --git a/src/tests/monitor-unit-tests.c b/src/tests/monitor-unit-tests.c
index 6b93db0aed..1aa81c8a73 100644
--- a/src/tests/monitor-unit-tests.c
+++ b/src/tests/monitor-unit-tests.c
@@ -3558,7 +3558,7 @@ meta_sensors_proxy_reset (MetaSensorsProxyMock *proxy)
 
   meta_sensors_proxy_mock_set_orientation (proxy,
                                            META_ORIENTATION_NORMAL);
-  wait_for_orientation_changes (orientation_manager);
+  wait_for_orientation (orientation_manager, META_ORIENTATION_NORMAL, NULL);
   g_object_unref (proxy);
 }
 G_DEFINE_AUTOPTR_CLEANUP_FUNC (MetaSensorsProxyAutoResetMock,
@@ -3876,16 +3876,14 @@ meta_test_monitor_orientation_initial_rotated (void)
   g_autoptr (MetaSensorsProxyAutoResetMock) orientation_mock = NULL;
   g_autoptr (ClutterAutoRemoveInputDevice) touch_device = NULL;
   MetaOrientation orientation;
+  guint times_signalled = 0;
 
   orientation_mock = meta_sensors_proxy_mock_get ();
   touch_device = meta_test_add_touch_device (backend);
   orientation = META_ORIENTATION_LEFT_UP;
   meta_sensors_proxy_mock_set_orientation (orientation_mock, orientation);
-  wait_for_orientation_changes (orientation_manager);
-  g_assert_cmpuint (
-    meta_orientation_manager_get_orientation (orientation_manager),
-    ==,
-    orientation);
+  wait_for_orientation (orientation_manager, orientation, &times_signalled);
+  g_assert_cmpuint (times_signalled, <=, 1);
 
   test_setup = create_monitor_test_setup (&test_case.setup,
                                           MONITOR_TEST_FLAG_NO_STORED);
@@ -3984,15 +3982,13 @@ meta_test_monitor_orientation_initial_rotated_no_touch_mode (void)
     meta_backend_get_orientation_manager (backend);
   g_autoptr (MetaSensorsProxyAutoResetMock) orientation_mock = NULL;
   MetaOrientation orientation;
+  guint times_signalled = 0;
 
   orientation_mock = meta_sensors_proxy_mock_get ();
   orientation = META_ORIENTATION_LEFT_UP;
   meta_sensors_proxy_mock_set_orientation (orientation_mock, orientation);
-  wait_for_orientation_changes (orientation_manager);
-  g_assert_cmpuint (
-    meta_orientation_manager_get_orientation (orientation_manager),
-    ==,
-    orientation);
+  wait_for_orientation (orientation_manager, orientation, &times_signalled);
+  g_assert_cmpuint (times_signalled, <=, 1);
 
   test_setup = create_monitor_test_setup (&test_case.setup,
                                           MONITOR_TEST_FLAG_NO_STORED);
@@ -4094,6 +4090,7 @@ meta_test_monitor_orientation_initial_stored_rotated (void)
   g_autoptr (MetaSensorsProxyAutoResetMock) orientation_mock = NULL;
   g_autoptr (ClutterAutoRemoveInputDevice) touch_device = NULL;
   MetaOrientation orientation;
+  guint times_signalled = 0;
 
   if (!meta_is_stage_views_enabled ())
     {
@@ -4105,11 +4102,8 @@ meta_test_monitor_orientation_initial_stored_rotated (void)
   touch_device = meta_test_add_touch_device (backend);
   orientation = META_ORIENTATION_RIGHT_UP;
   meta_sensors_proxy_mock_set_orientation (orientation_mock, orientation);
-  wait_for_orientation_changes (orientation_manager);
-  g_assert_cmpuint (
-    meta_orientation_manager_get_orientation (orientation_manager),
-    ==,
-    orientation);
+  wait_for_orientation (orientation_manager, orientation, &times_signalled);
+  g_assert_cmpuint (times_signalled, <=, 1);
 
   test_setup = create_monitor_test_setup (&test_case.setup,
                                           MONITOR_TEST_FLAG_NONE);
@@ -4131,11 +4125,8 @@ meta_test_monitor_orientation_initial_stored_rotated (void)
 
   orientation = META_ORIENTATION_LEFT_UP;
   meta_sensors_proxy_mock_set_orientation (orientation_mock, orientation);
-  wait_for_orientation_changes (orientation_manager);
-  g_assert_cmpuint (
-    meta_orientation_manager_get_orientation (orientation_manager),
-    ==,
-    orientation);
+  wait_for_orientation (orientation_manager, orientation, &times_signalled);
+  g_assert_cmpuint (times_signalled, <=, 1);
 
   meta_backend_test_set_is_lid_closed (META_BACKEND_TEST (backend), FALSE);
   meta_monitor_manager_lid_is_closed_changed (monitor_manager);
@@ -4150,11 +4141,8 @@ meta_test_monitor_orientation_initial_stored_rotated (void)
 
   orientation = META_ORIENTATION_RIGHT_UP;
   meta_sensors_proxy_mock_set_orientation (orientation_mock, orientation);
-  wait_for_orientation_changes (orientation_manager);
-  g_assert_cmpuint (
-    meta_orientation_manager_get_orientation (orientation_manager),
-    ==,
-    orientation);
+  wait_for_orientation (orientation_manager, orientation, &times_signalled);
+  g_assert_cmpuint (times_signalled, <=, 1);
 
   META_TEST_LOG_CALL ("Checking configuration per orientation",
                       check_monitor_configuration_per_orientation (
@@ -4252,6 +4240,7 @@ meta_test_monitor_orientation_initial_stored_rotated_no_touch (void)
     meta_backend_get_orientation_manager (backend);
   g_autoptr (MetaSensorsProxyAutoResetMock) orientation_mock = NULL;
   MetaOrientation orientation;
+  guint times_signalled = 0;
 
   if (!meta_is_stage_views_enabled ())
     {
@@ -4262,11 +4251,8 @@ meta_test_monitor_orientation_initial_stored_rotated_no_touch (void)
   orientation_mock = meta_sensors_proxy_mock_get ();
   orientation = META_ORIENTATION_RIGHT_UP;
   meta_sensors_proxy_mock_set_orientation (orientation_mock, orientation);
-  wait_for_orientation_changes (orientation_manager);
-  g_assert_cmpuint (
-    meta_orientation_manager_get_orientation (orientation_manager),
-    ==,
-    orientation);
+  wait_for_orientation (orientation_manager, orientation, &times_signalled);
+  g_assert_cmpuint (times_signalled, <=, 1);
 
   test_setup = create_monitor_test_setup (&test_case.setup,
                                           MONITOR_TEST_FLAG_NONE);
@@ -4383,6 +4369,7 @@ meta_test_monitor_orientation_changes (void)
   g_autoptr (MetaMonitorsConfig) initial_config = NULL;
   g_autoptr (MetaMonitorsConfig) previous_config = NULL;
   MetaOrientation i;
+  guint times_signalled = 0;
 
   orientation_mock = meta_sensors_proxy_mock_get ();
   touch_device = meta_test_add_touch_device (backend);
@@ -4406,10 +4393,8 @@ meta_test_monitor_orientation_changes (void)
       MetaMonitorsConfig *previous;
 
       meta_sensors_proxy_mock_set_orientation (orientation_mock, i);
-      wait_for_orientation_changes (orientation_manager);
-      g_assert_cmpuint (
-        meta_orientation_manager_get_orientation (orientation_manager),
-        ==, i);
+      wait_for_orientation (orientation_manager, i, &times_signalled);
+      g_assert_cmpuint (times_signalled, <=, 1);
 
       META_TEST_LOG_CALL ("Checking configuration per orientation",
                           check_monitor_configuration_per_orientation (
@@ -4435,7 +4420,9 @@ meta_test_monitor_orientation_changes (void)
 
   meta_sensors_proxy_mock_set_orientation (orientation_mock,
                                            META_ORIENTATION_NORMAL);
-  wait_for_orientation_changes (orientation_manager);
+  wait_for_orientation (orientation_manager, META_ORIENTATION_NORMAL,
+                        &times_signalled);
+  g_assert_cmpuint (times_signalled, ==, 0);
   META_TEST_LOG_CALL ("Checking configuration per orientation",
                       check_monitor_configuration_per_orientation (
                         &test_case.expect, 0, META_ORIENTATION_NORMAL,
@@ -4453,10 +4440,8 @@ meta_test_monitor_orientation_changes (void)
       MetaMonitorsConfig *previous;
 
       meta_sensors_proxy_mock_set_orientation (orientation_mock, i);
-      wait_for_orientation_changes (orientation_manager);
-      g_assert_cmpuint (
-        meta_orientation_manager_get_orientation (orientation_manager),
-        ==, i);
+      wait_for_orientation (orientation_manager, i, &times_signalled);
+      g_assert_cmpuint (times_signalled, <=, 1);
 
       META_TEST_LOG_CALL ("Checking configuration per orientation",
                           check_monitor_configuration_per_orientation (
@@ -4607,6 +4592,7 @@ meta_test_monitor_orientation_changes_with_hotplugging (void)
   g_autoptr (ClutterAutoRemoveInputDevice) touch_device = NULL;
   g_autoptr (MetaSensorsProxyAutoResetMock) orientation_mock = NULL;
   MetaOrientation i;
+  guint times_signalled = 0;
 
   orientation_mock = meta_sensors_proxy_mock_get ();
   touch_device = meta_test_add_touch_device (backend);
@@ -4630,10 +4616,8 @@ meta_test_monitor_orientation_changes_with_hotplugging (void)
   for (i = META_N_ORIENTATIONS - 1; i > META_ORIENTATION_UNDEFINED; i--)
     {
       meta_sensors_proxy_mock_set_orientation (orientation_mock, i);
-      wait_for_orientation_changes (orientation_manager);
-      g_assert_cmpuint (
-        meta_orientation_manager_get_orientation (orientation_manager),
-        ==, i);
+      wait_for_orientation (orientation_manager, i, &times_signalled);
+      g_assert_cmpuint (times_signalled, <=, 1);
 
       META_TEST_LOG_CALL ("Checking configuration per orientation",
                           check_monitor_configuration_per_orientation (
@@ -4642,7 +4626,9 @@ meta_test_monitor_orientation_changes_with_hotplugging (void)
 
   meta_sensors_proxy_mock_set_orientation (orientation_mock,
                                            META_ORIENTATION_NORMAL);
-  wait_for_orientation_changes (orientation_manager);
+  wait_for_orientation (orientation_manager, META_ORIENTATION_NORMAL,
+                        &times_signalled);
+  g_assert_cmpuint (times_signalled, <=, 1);
   check_monitor_configuration (&test_case.expect);
 
   /* External monitor connected */
@@ -4663,10 +4649,8 @@ meta_test_monitor_orientation_changes_with_hotplugging (void)
   for (i = META_N_ORIENTATIONS - 1; i > META_ORIENTATION_UNDEFINED; i--)
     {
       meta_sensors_proxy_mock_set_orientation (orientation_mock, i);
-      wait_for_orientation_changes (orientation_manager);
-      g_assert_cmpuint (
-        meta_orientation_manager_get_orientation (orientation_manager),
-        ==, i);
+      wait_for_orientation (orientation_manager, i, &times_signalled);
+      g_assert_cmpuint (times_signalled, <=, 1);
 
       META_TEST_LOG_CALL ("Checking configuration per orientation",
                           check_monitor_configuration_per_orientation (
@@ -4675,7 +4659,9 @@ meta_test_monitor_orientation_changes_with_hotplugging (void)
 
   meta_sensors_proxy_mock_set_orientation (orientation_mock,
                                            META_ORIENTATION_NORMAL);
-  wait_for_orientation_changes (orientation_manager);
+  wait_for_orientation (orientation_manager, META_ORIENTATION_NORMAL,
+                        &times_signalled);
+  g_assert_cmpuint (times_signalled, <=, 1);
   check_monitor_configuration (&test_case.expect);
 
   /* Lid closed */
@@ -4695,13 +4681,16 @@ meta_test_monitor_orientation_changes_with_hotplugging (void)
   for (i = META_N_ORIENTATIONS - 1; i > META_ORIENTATION_UNDEFINED; i--)
     {
       meta_sensors_proxy_mock_set_orientation (orientation_mock, i);
-      wait_for_orientation_changes (orientation_manager);
+      wait_for_orientation (orientation_manager, i, &times_signalled);
+      g_assert_cmpuint (times_signalled, <=, 1);
       check_monitor_configuration (&test_case.expect);
     }
 
   meta_sensors_proxy_mock_set_orientation (orientation_mock,
                                            META_ORIENTATION_NORMAL);
-  wait_for_orientation_changes (orientation_manager);
+  wait_for_orientation (orientation_manager, META_ORIENTATION_NORMAL,
+                        &times_signalled);
+  g_assert_cmpuint (times_signalled, <=, 1);
 
   /*
    * The second part of this test emulate the following at each device rotation:
@@ -4744,7 +4733,8 @@ meta_test_monitor_orientation_changes_with_hotplugging (void)
 
       /* Change orientation */
       meta_sensors_proxy_mock_set_orientation (orientation_mock, i);
-      wait_for_orientation_changes (orientation_manager);
+      wait_for_orientation (orientation_manager, i, &times_signalled);
+      g_assert_cmpuint (times_signalled, <=, 1);
       check_monitor_configuration (&test_case.expect);
 
       /* Open the lid */
@@ -4760,7 +4750,12 @@ meta_test_monitor_orientation_changes_with_hotplugging (void)
       meta_backend_test_set_is_lid_closed (META_BACKEND_TEST (backend), FALSE);
       emulate_hotplug (test_setup);
 
-      wait_for_orientation_changes (orientation_manager);
+      /* We don't actually expect the orientation to change here, so we
+       * just wait for a moment (so that if the orientation *did* change,
+       * mutter has had a chance to process it), and then continue. */
+      wait_for_possible_orientation_change (orientation_manager, &times_signalled);
+      g_assert_cmpuint (times_signalled, ==, 0);
+
       META_TEST_LOG_CALL ("Checking configuration per orientation",
                           check_monitor_configuration_per_orientation (
                             &test_case.expect, 0, i, 1024, 768));
@@ -4797,7 +4792,9 @@ meta_test_monitor_orientation_changes_with_hotplugging (void)
 
   meta_sensors_proxy_mock_set_orientation (orientation_mock,
                                            META_ORIENTATION_NORMAL);
-  wait_for_orientation_changes (orientation_manager);
+  wait_for_orientation (orientation_manager, META_ORIENTATION_NORMAL,
+                        &times_signalled);
+  g_assert_cmpuint (times_signalled, <=, 1);
 }
 
 static void
diff --git a/src/tests/orientation-manager-unit-tests.c b/src/tests/orientation-manager-unit-tests.c
index a936dccb7a..999b0a320a 100644
--- a/src/tests/orientation-manager-unit-tests.c
+++ b/src/tests/orientation-manager-unit-tests.c
@@ -24,42 +24,120 @@
 
 #include "tests/meta-sensors-proxy-mock.h"
 
+const char *
+orientation_to_string (MetaOrientation orientation)
+{
+  switch (orientation)
+    {
+      case META_ORIENTATION_UNDEFINED:
+        return "(undefined)";
+      case META_ORIENTATION_NORMAL:
+        return "normal";
+      case META_ORIENTATION_BOTTOM_UP:
+        return "bottom-up";
+      case META_ORIENTATION_LEFT_UP:
+        return "left-up";
+      case META_ORIENTATION_RIGHT_UP:
+        return "right-up";
+      default:
+        return "(invalid)";
+    }
+}
+
+typedef struct
+{
+  MetaOrientation expected;
+  MetaOrientation orientation;
+  gulong connection_id;
+  guint timeout_id;
+  guint times_signalled;
+} WaitForOrientation;
+
 static void
-on_orientation_changed (gpointer data)
+on_orientation_changed (gpointer data,
+                        MetaOrientationManager *orientation_manager)
 {
-  gboolean *changed = data;
+  WaitForOrientation *wfo = data;
 
-  *changed = TRUE;
+  wfo->orientation = meta_orientation_manager_get_orientation (orientation_manager);
+  wfo->times_signalled++;
 }
 
 static gboolean
 on_max_wait_timeout (gpointer data)
 {
-  guint *timeout_id = data;
-
-  *timeout_id = 0;
+  WaitForOrientation *wfo = data;
 
+  wfo->timeout_id = 0;
   return G_SOURCE_REMOVE;
 }
 
+/*
+ * Assert that the orientation eventually changes to @orientation.
+ */
 void
-wait_for_orientation_changes (MetaOrientationManager *orientation_manager)
+wait_for_orientation (MetaOrientationManager *orientation_manager,
+                      MetaOrientation orientation,
+                      guint *times_signalled_out)
 {
-  gboolean changed = FALSE;
-  gulong connection_id;
-  guint timeout_id;
+  WaitForOrientation wfo = { orientation, META_ORIENTATION_UNDEFINED, 0, 0, 0 };
 
-  timeout_id = g_timeout_add (300, on_max_wait_timeout, &timeout_id);
-  connection_id = g_signal_connect_swapped (orientation_manager,
-                                            "orientation-changed",
-                                            G_CALLBACK (on_orientation_changed),
-                                            &changed);
+  wfo.orientation = meta_orientation_manager_get_orientation (orientation_manager);
 
-  while (!changed && timeout_id)
+  /* This timeout can be relatively generous because we don't expect to
+   * reach it: if we do, that's a test failure. */
+  wfo.timeout_id = g_timeout_add_seconds (10, on_max_wait_timeout, &wfo);
+  wfo.connection_id = g_signal_connect_swapped (orientation_manager,
+                                                 "orientation-changed",
+                                                 G_CALLBACK (on_orientation_changed),
+                                                 &wfo);
+
+  while (wfo.orientation != orientation && wfo.timeout_id != 0)
     g_main_context_iteration (NULL, TRUE);
 
-  g_clear_handle_id (&timeout_id, g_source_remove);
-  g_signal_handler_disconnect (orientation_manager, connection_id);
+  if (wfo.orientation != orientation)
+    g_error ("Timed out waiting for orientation to change from %s to %s "
+             "(received %u orientation-changed signal(s) while waiting)",
+             orientation_to_string (wfo.orientation),
+             orientation_to_string (orientation),
+             wfo.times_signalled);
+
+  g_clear_handle_id (&wfo.timeout_id, g_source_remove);
+  g_signal_handler_disconnect (orientation_manager, wfo.connection_id);
+
+  if (times_signalled_out != NULL)
+    *times_signalled_out = wfo.times_signalled;
+}
+
+/*
+ * Wait for a possible orientation change, but don't assert that one occurs.
+ */
+void
+wait_for_possible_orientation_change (MetaOrientationManager *orientation_manager,
+                                      guint *times_signalled_out)
+{
+  WaitForOrientation wfo = { META_ORIENTATION_UNDEFINED, META_ORIENTATION_UNDEFINED, 0, 0, 0 };
+
+  wfo.orientation = meta_orientation_manager_get_orientation (orientation_manager);
+
+  /* This can't be as long as the timeout for wait_for_orientation(),
+   * because in the usual case we expect to reach this timeout: we're
+   * only waiting so that if the orientation (incorrectly?) changed here,
+   * we'd have a chance to detect that. */
+  wfo.timeout_id = g_timeout_add (300, on_max_wait_timeout, &wfo);
+  wfo.connection_id = g_signal_connect_swapped (orientation_manager,
+                                                 "orientation-changed",
+                                                 G_CALLBACK (on_orientation_changed),
+                                                 &wfo);
+
+  while (wfo.times_signalled == 0 && wfo.timeout_id != 0)
+    g_main_context_iteration (NULL, TRUE);
+
+  g_clear_handle_id (&wfo.timeout_id, g_source_remove);
+  g_signal_handler_disconnect (orientation_manager, wfo.connection_id);
+
+  if (times_signalled_out != NULL)
+    *times_signalled_out = wfo.times_signalled;
 }
 
 static void
@@ -90,25 +168,40 @@ meta_test_orientation_manager_no_device (void)
   g_object_unref (orientation_mock);
 }
 
+static gboolean
+on_wait_for_accel_timeout (gpointer data)
+{
+  guint *timeout_p = data;
+
+  *timeout_p = 0;
+  return G_SOURCE_REMOVE;
+}
+
 static void
 meta_test_orientation_manager_has_accelerometer (void)
 {
   g_autoptr (MetaOrientationManager) manager = NULL;
   g_autoptr (MetaSensorsProxyMock) orientation_mock = NULL;
+  guint timeout_id;
 
   manager = g_object_new (META_TYPE_ORIENTATION_MANAGER, NULL);
   orientation_mock = meta_sensors_proxy_mock_get ();
 
+  timeout_id = g_timeout_add_seconds (10, on_wait_for_accel_timeout, &timeout_id);
   meta_sensors_proxy_mock_set_property (orientation_mock,
                                         "HasAccelerometer",
                                         g_variant_new_boolean (TRUE));
-  wait_for_orientation_changes (manager);
+
+  while (!meta_orientation_manager_has_accelerometer (manager) &&
+         timeout_id != 0)
+    g_main_context_iteration (NULL, TRUE);
 
   g_debug ("Checking whether accelerometer is present");
   g_assert_true (meta_orientation_manager_has_accelerometer (manager));
   g_assert_cmpuint (meta_orientation_manager_get_orientation (manager),
                     ==,
                     META_ORIENTATION_UNDEFINED);
+  g_clear_handle_id (&timeout_id, g_source_remove);
 }
 
 static void
@@ -141,14 +234,13 @@ meta_test_orientation_manager_accelerometer_orientations (void)
 
   for (i = initial + 1; i != initial; i = (i + 1) % META_N_ORIENTATIONS)
     {
-      changed_called = FALSE;
-      meta_sensors_proxy_mock_set_orientation (orientation_mock, i);
-      wait_for_orientation_changes (manager);
+      guint times_signalled = 0;
 
+      changed_called = FALSE;
       g_debug ("Checking orientation %d", i);
-      g_assert_cmpuint (meta_orientation_manager_get_orientation (manager),
-                        ==,
-                        i);
+      meta_sensors_proxy_mock_set_orientation (orientation_mock, i);
+      wait_for_orientation (manager, i, &times_signalled);
+      g_assert_cmpuint (times_signalled, <=, 1);
 
       if (i != META_ORIENTATION_UNDEFINED)
         g_assert_true (changed_called);
diff --git a/src/tests/orientation-manager-unit-tests.h b/src/tests/orientation-manager-unit-tests.h
index 56eff09d1d..2a94f81427 100644
--- a/src/tests/orientation-manager-unit-tests.h
+++ b/src/tests/orientation-manager-unit-tests.h
@@ -25,6 +25,12 @@
 
 void init_orientation_manager_tests (void);
 
-void wait_for_orientation_changes (MetaOrientationManager *orientation_manager);
+void wait_for_orientation (MetaOrientationManager *orientation_manager,
+                           MetaOrientation orientation,
+                           guint *times_signalled_out);
+void wait_for_possible_orientation_change (MetaOrientationManager *orientation_manager,
+                                           guint *times_signalled_out);
+
+const char *orientation_to_string (MetaOrientation orientation);
 
 #endif /* ORIENTATION_MANAGER_UNIT_TESTS_H */