Using eventually to avoid arbitrary timeouts in tests

Approved by: Charles Kerr, PS Jenkins bot

2 files changed, 140 insertions, 18 deletions

diff --git a/debian/changelog b/debian/changelog
index 4d0314b..c360efd 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,9 @@
+indicator-sound (12.10.2+15.10.20150507+eventually4-0ubuntu1) wily; urgency=medium
+
+  * Using eventually to avoid arbitrary timeouts in tests
+
+ -- Ted Gould <ted@ubuntu.com>  Wed, 27 May 2015 11:11:19 -0500
+
 indicator-sound (12.10.2+15.10.20150507-0ubuntu1) wily; urgency=medium
 
   [ Charles Kerr ]
diff --git a/tests/media-player-user.cc b/tests/media-player-user.cc
index 8d2fcb1..ca20b5f 100644
--- a/tests/media-player-user.cc
+++ b/tests/media-player-user.cc
@@ -17,6 +17,9 @@
  *      Ted Gould <ted@canonical.com>
  */
 
+#include <chrono>
+#include <future>
+
 #include <gtest/gtest.h>
 #include <gio/gio.h>
 #include <libdbustest/dbus-test.h>
@@ -31,24 +34,55 @@ class MediaPlayerUserTest : public ::testing::Test
 {
 
 	protected:
-		DbusTestService * service = NULL;
+		DbusTestService * testsystem = NULL;
 		AccountsServiceMock service_mock;
 
+		DbusTestService * testsession = NULL;
+
+		DbusTestProcess * systemmonitor = nullptr;
+		DbusTestProcess * sessionmonitor = nullptr;
+
 		GDBusConnection * system = NULL;
+		GDBusConnection * session = NULL;
 		GDBusProxy * proxy = NULL;
 
+		std::chrono::milliseconds _eventuallyTime = std::chrono::seconds{5};
+
 		virtual void SetUp() {
-			service = dbus_test_service_new(NULL);
-			dbus_test_service_set_bus(service, DBUS_TEST_SERVICE_BUS_SYSTEM);
+			/* System Bus */
+			testsystem = dbus_test_service_new(NULL);
+			dbus_test_service_set_bus(testsystem, DBUS_TEST_SERVICE_BUS_SYSTEM);
+
+			systemmonitor = dbus_test_process_new("dbus-monitor");
+			dbus_test_process_append_param(systemmonitor, "--system");
+			dbus_test_task_set_name(DBUS_TEST_TASK(systemmonitor), "System");
+			dbus_test_service_add_task(testsystem, DBUS_TEST_TASK(systemmonitor));
 
-			dbus_test_service_add_task(service, (DbusTestTask*)service_mock);
-			dbus_test_service_start_tasks(service);
+			dbus_test_service_add_task(testsystem, (DbusTestTask*)service_mock);
+			dbus_test_service_start_tasks(testsystem);
 
 			system = g_bus_get_sync(G_BUS_TYPE_SYSTEM, NULL, NULL);
 			ASSERT_NE(nullptr, system);
 			g_dbus_connection_set_exit_on_close(system, FALSE);
 			g_object_add_weak_pointer(G_OBJECT(system), (gpointer *)&system);
 
+			/* Session Bus */
+			testsession = dbus_test_service_new(NULL);
+			dbus_test_service_set_bus(testsession, DBUS_TEST_SERVICE_BUS_SESSION);
+
+			sessionmonitor = dbus_test_process_new("dbus-monitor");
+			dbus_test_process_append_param(sessionmonitor, "--session");
+			dbus_test_task_set_name(DBUS_TEST_TASK(sessionmonitor), "Session");
+			dbus_test_service_add_task(testsession, DBUS_TEST_TASK(sessionmonitor));
+
+			dbus_test_service_start_tasks(testsession);
+
+			session = g_bus_get_sync(G_BUS_TYPE_SYSTEM, NULL, NULL);
+			ASSERT_NE(nullptr, session);
+			g_dbus_connection_set_exit_on_close(session, FALSE);
+			g_object_add_weak_pointer(G_OBJECT(session), (gpointer *)&session);
+
+			/* Setup proxy */
 			proxy = g_dbus_proxy_new_sync(system,
 				G_DBUS_PROXY_FLAGS_NONE,
 				NULL,
@@ -60,10 +94,15 @@ class MediaPlayerUserTest : public ::testing::Test
 		}
 
 		virtual void TearDown() {
+			g_clear_object(&sessionmonitor);
+			g_clear_object(&systemmonitor);
+
 			g_clear_object(&proxy);
-			g_clear_object(&service);
+			g_clear_object(&testsystem);
+			g_clear_object(&testsession);
 
 			g_object_unref(system);
+			g_object_unref(session);
 
 			#if 0
 			/* Accounts Service keeps a bunch of references around so we
@@ -95,8 +134,78 @@ class MediaPlayerUserTest : public ::testing::Test
 		void set_property (const gchar * name, GVariant * value) {
 			dbus_test_dbus_mock_object_update_property((DbusTestDbusMock *)service_mock, service_mock.get_sound(), name, value, NULL);
 		}
+
+		testing::AssertionResult expectEventually (std::function<testing::AssertionResult(void)> &testfunc) {
+			auto loop = std::shared_ptr<GMainLoop>(g_main_loop_new(nullptr, FALSE), [](GMainLoop * loop) { if (loop != nullptr) g_main_loop_unref(loop); });
+
+			std::promise<testing::AssertionResult> retpromise;
+			auto retfuture = retpromise.get_future();
+			auto start = std::chrono::steady_clock::now();
+
+			/* The core of the idle function as an object so we can use the C++-isms
+			   of attaching the variables and make this code reasonably readable */
+			std::function<void(void)> idlefunc = [&loop, &retpromise, &testfunc, &start, this]() -> void {
+				auto result = testfunc();
+
+				if (result == false && _eventuallyTime > (std::chrono::steady_clock::now() - start)) {
+					return;
+				}
+
+				retpromise.set_value(result);
+				g_main_loop_quit(loop.get());
+			};
+
+			auto idlesrc = g_idle_add([](gpointer data) -> gboolean {
+				auto func = reinterpret_cast<std::function<void(void)> *>(data);
+				(*func)();
+				return G_SOURCE_CONTINUE;
+			}, &idlefunc);
+
+			g_main_loop_run(loop.get());
+			g_source_remove(idlesrc);
+
+			return retfuture.get();
+		}
+
+		/* Eventually Helpers */
+		#define _EVENTUALLY_HELPER(oper) \
+		template <typename... Args> testing::AssertionResult expectEventually##oper (Args&& ... args) { \
+			std::function<testing::AssertionResult(void)> func = [&]() { \
+				return testing::internal::CmpHelper##oper(std::forward<Args>(args)...); \
+			}; \
+			return expectEventually(func); \
+		}
+
+		_EVENTUALLY_HELPER(EQ);
+		_EVENTUALLY_HELPER(NE);
+		_EVENTUALLY_HELPER(LT);
+		_EVENTUALLY_HELPER(GT);
+		_EVENTUALLY_HELPER(STREQ);
+		_EVENTUALLY_HELPER(STRNE);
+
+		#undef _EVENTUALLY_HELPER
 };
 
+/* Helpers */
+#define EXPECT_EVENTUALLY_EQ(expected, actual) \
+	EXPECT_PRED_FORMAT2(MediaPlayerUserTest::expectEventuallyEQ, expected, actual)
+
+#define EXPECT_EVENTUALLY_NE(expected, actual) \
+	EXPECT_PRED_FORMAT2(MediaPlayerUserTest::expectEventuallyNE, expected, actual)
+
+#define EXPECT_EVENTUALLY_LT(expected, actual) \
+	EXPECT_PRED_FORMAT2(MediaPlayerUserTest::expectEventuallyLT, expected, actual)
+
+#define EXPECT_EVENTUALLY_GT(expected, actual) \
+	EXPECT_PRED_FORMAT2(MediaPlayerUserTest::expectEventuallyGT, expected, actual)
+
+#define EXPECT_EVENTUALLY_STREQ(expected, actual) \
+	EXPECT_PRED_FORMAT2(MediaPlayerUserTest::expectEventuallySTREQ, expected, actual)
+
+#define EXPECT_EVENTUALLY_STRNE(expected, actual) \
+	EXPECT_PRED_FORMAT2(MediaPlayerUserTest::expectEventuallySTRNE, expected, actual)
+
+
 TEST_F(MediaPlayerUserTest, BasicObject) {
 	MediaPlayerUser * player = media_player_user_new("user");
 	ASSERT_NE(nullptr, player);
@@ -125,6 +234,11 @@ TEST_F(MediaPlayerUserTest, BasicObject) {
 	g_clear_object(&player);
 }
 
+void
+running_update (GObject * obj, GParamSpec * pspec, bool * running) {
+	*running = media_player_get_is_running(MEDIA_PLAYER(obj)) == TRUE;
+};
+
 TEST_F(MediaPlayerUserTest, DataSet) {
 	/* Put data into Acts */
 	set_property("Timestamp", g_variant_new_uint64(g_get_monotonic_time()));
@@ -141,11 +255,11 @@ TEST_F(MediaPlayerUserTest, DataSet) {
 	MediaPlayerUser * player = media_player_user_new("user");
 	ASSERT_NE(nullptr, player);
 
-	/* Get the proxy -- and it's precious precious data -- oh, my, precious! */
-	loop(100);
-
 	/* Ensure even with the proxy we don't have anything */
-	EXPECT_TRUE(media_player_get_is_running(MEDIA_PLAYER(player)));
+	bool running = false;
+	g_signal_connect(G_OBJECT(player), "notify::is-running", G_CALLBACK(running_update), &running);
+	running_update(G_OBJECT(player), nullptr, &running);
+	EXPECT_EVENTUALLY_EQ(true, running);
 	EXPECT_TRUE(media_player_get_can_raise(MEDIA_PLAYER(player)));
 	EXPECT_STREQ("user", media_player_get_id(MEDIA_PLAYER(player)));
 	EXPECT_STREQ("The Player Formerly Known as Prince", media_player_get_name(MEDIA_PLAYER(player)));
@@ -180,24 +294,26 @@ TEST_F(MediaPlayerUserTest, TimeoutTest) {
 	set_property("Album", g_variant_new_string("Vinyl is dead"));
 	set_property("ArtUrl", g_variant_new_string("http://art.url"));
 
-	/* Ensure the properties get set before we pull them */
-	loop(100);
-
 	/* Build our media player */
 	MediaPlayerUser * player = media_player_user_new("user");
 	ASSERT_NE(nullptr, player);
 
-	/* Get the proxy -- and the old data, so old, like forever */
-	loop(100);
+	bool running = false;
+	g_signal_connect(G_OBJECT(player), "notify::is-running", G_CALLBACK(running_update), &running);
+	running_update(G_OBJECT(player), nullptr, &running);
 
 	/* Ensure that we show up as not running */
-	EXPECT_FALSE(media_player_get_is_running(MEDIA_PLAYER(player)));
+	EXPECT_EVENTUALLY_EQ(false, running);
 
 	/* Update to make running */
 	set_property("Timestamp", g_variant_new_uint64(g_get_monotonic_time()));
-	loop(100);
 
-	EXPECT_TRUE(media_player_get_is_running(MEDIA_PLAYER(player)));
+	EXPECT_EVENTUALLY_EQ(true, running);
+
+	/* Clear to not run */
+	set_property("Timestamp", g_variant_new_uint64(1));
+
+	EXPECT_EVENTUALLY_EQ(false, running);
 
 	g_clear_object(&in_icon);
 	g_clear_object(&player);