Merge lp:~ted/indicator-sound/eventually-testing into lp:indicator-sound/15.10

[Ted Gould]

Using eventually to avoid arbitrary timeouts in tests

[lp:~ted/indicator-sound/eventually-testing]

504. By Ted Gould on 2015-05-28

Dumping the bus, old school style

505. By Ted Gould on 2015-05-28

For Launchpad

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:503
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~ted/indicator-sound/eventually-testing/+merge/260373/+edit-commit-message

http://jenkins.qa.ubuntu.com/job/indicator-sound-ci/225/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/indicator-sound-wily-amd64-ci/1/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-sound-wily-armhf-ci/1
        deb: http://jenkins.qa.ubuntu.com/job/indicator-sound-wily-armhf-ci/1/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/indicator-sound-ci/225/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:503
http://jenkins.qa.ubuntu.com/job/indicator-sound-ci/226/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-sound-wily-amd64-ci/2
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-sound-wily-armhf-ci/2
        deb: http://jenkins.qa.ubuntu.com/job/indicator-sound-wily-armhf-ci/2/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/indicator-sound-ci/226/rebuild

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:505
http://jenkins.qa.ubuntu.com/job/indicator-sound-ci/227/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-sound-wily-amd64-ci/3
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-sound-wily-armhf-ci/3
        deb: http://jenkins.qa.ubuntu.com/job/indicator-sound-wily-armhf-ci/3/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/indicator-sound-ci/227/rebuild

review: Approve (continuous-integration)

[Charles Kerr (charlesk)]

Nice improvements all around.

review: Approve

=== modified file 'debian/changelog'
--- debian/changelog	2015-05-07 15:27:00 +0000
+++ debian/changelog	2015-05-28 15:03:10 +0000
@@ -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 <[e-mail]>  Wed, 27 May 2015 11:11:19 -0500
+
 indicator-sound (12.10.2+15.10.20150507-0ubuntu1) wily; urgency=medium
 
   [ Charles Kerr ]

=== modified file 'tests/media-player-user.cc'
--- tests/media-player-user.cc	2015-01-30 16:05:10 +0000
+++ tests/media-player-user.cc	2015-05-28 15:03:10 +0000
@@ -17,6 +17,9 @@
  *      Ted Gould <[e-mail]>
  */
 
+#include <chrono>
+#include <future>
+
 #include <gtest/gtest.h>
 #include <gio/gio.h>
 #include <libdbustest/dbus-test.h>
@@ -31,24 +34,55 @@
 {
 
 	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);
-
-			dbus_test_service_add_task(service, (DbusTestTask*)service_mock);
-			dbus_test_service_start_tasks(service);
+			/* 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(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 @@
 		}
 
 		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 @@
 		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 @@
 	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 @@
 	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 @@
 	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);