Refactor DBusProxy and the Search part of lenses

(bzr r2866.7.1)

3 files changed, 97 insertions, 128 deletions

diff --git a/tests/test_gdbus_proxy.cpp b/tests/test_gdbus_proxy.cpp
index de9675bf1..50347b55f 100644
--- a/tests/test_gdbus_proxy.cpp
+++ b/tests/test_gdbus_proxy.cpp
@@ -3,71 +3,36 @@
 #include <UnityCore/GLibWrapper.h>
 #include <UnityCore/GLibDBusProxy.h>
 
+#include "test_utils.h"
+
 using namespace std;
 using namespace unity;
 
 namespace
 {
 
-GMainLoop* loop_ = NULL;
-glib::DBusProxy* proxy = NULL;
-
 class TestGDBusProxy: public ::testing::Test
 {
 public:
 TestGDBusProxy()
- : connected_result(false)
- , got_signal_return(false)
+ : got_signal_return(false)
 , got_result_return(false)
+ , proxy("com.canonical.Unity.Test",
+ "/com/canonical/gdbus_wrapper",
+ "com.canonical.gdbus_wrapper")
 {
 }
- bool connected_result;
+
 bool got_signal_return;
 bool got_result_return;
+ glib::DBusProxy proxy;
 };
 
 TEST_F(TestGDBusProxy, TestConstruction)
 {
- loop_ = g_main_loop_new(NULL, FALSE);
- proxy = new glib::DBusProxy("com.canonical.Unity.Test", 
- "/com/canonical/gdbus_wrapper", 
- "com.canonical.gdbus_wrapper");
- // performs a check on the proxy, if the proxy is connected, report a sucess
- auto timeout_check = [] (gpointer data) -> gboolean
- {
- TestGDBusProxy* self = static_cast<TestGDBusProxy*>(data);
- if (proxy->IsConnected())
- {
- self->connected_result = true;
- g_main_loop_quit(loop_);
- return FALSE;
- }
- else
- {
- self->connected_result = false;
- return TRUE;
- }
- };
- 
-
- // if the proxy is not connected when this lambda runs, fail.
- auto timeout_bailout = [] (gpointer data) -> gboolean 
- {
- TestGDBusProxy* self = static_cast<TestGDBusProxy*>(data);
- // reached timeout, failed testing
- self->connected_result = false;
- g_main_loop_quit(loop_);
- return FALSE;
- };
- 
- guint timeout_source = g_timeout_add(1000, timeout_check, this); // check once a second
- guint bailout_source = g_timeout_add(10000, timeout_bailout, this); // bail out after ten
-
- g_main_loop_run(loop_);
- g_source_remove(timeout_source);
- g_source_remove(bailout_source);
- 
- EXPECT_EQ(connected_result, true);
+ EXPECT_FALSE(proxy.IsConnected());
+ Utils::WaitUntil(sigc::mem_fun(proxy, &glib::DBusProxy::IsConnected));
+ EXPECT_TRUE(proxy.IsConnected());
 }
 
 TEST_F(TestGDBusProxy, TestMethodReturn)
@@ -75,23 +40,19 @@ TEST_F(TestGDBusProxy, TestMethodReturn)
 // Our service is setup so that if you call the TestMethod method, it will emit the TestSignal method
 // with whatever string you pass in
 gchar* expected_return = (gchar *)"TestStringTestString☻☻☻"; // cast to get gcc to shut up
- gchar* returned_result = g_strdup("Not equal"); 
- gchar* returned_signal = g_strdup("Not equal"); 
+ std::string returned_signal("Not equal");
+ std::string returned_result("Not equal");
 
- GVariant* param_value = g_variant_new_string(expected_return);
- GVariant* parameters = g_variant_new_tuple(&param_value, 1);
+ GVariant* parameters = g_variant_new("(s)", expected_return);
 // signal callback
 auto signal_connection = [&](GVariant *variant)
 {
 if (variant != nullptr)
 {
- g_free(returned_signal);
- returned_signal = g_strdup(g_variant_get_string(g_variant_get_child_value(variant, 0), NULL));
+ returned_signal = g_variant_get_string(g_variant_get_child_value(variant, 0), NULL);
 }
 
 got_signal_return = true;
- if (got_signal_return && got_result_return)
- g_main_loop_quit(loop_);
 };
 
 // method callback
@@ -99,37 +60,72 @@ TEST_F(TestGDBusProxy, TestMethodReturn)
 {
 if (variant != nullptr)
 {
- g_free(returned_result);
- returned_result = g_strdup(g_variant_get_string(g_variant_get_child_value(variant, 0), NULL));
+ returned_result = g_variant_get_string(g_variant_get_child_value(variant, 0), NULL);
 }
 
 got_result_return = true;
- if (got_signal_return && got_result_return)
- g_main_loop_quit(loop_);
 };
- 
- auto timeout_bailout = [] (gpointer data) -> gboolean // bail out after 10 seconds
+
+ Utils::WaitUntil(sigc::mem_fun(proxy, &glib::DBusProxy::IsConnected));
+ EXPECT_TRUE(proxy.IsConnected()); // fail if we are not connected
+ proxy.Connect("TestSignal", signal_connection);
+ proxy.Call("TestMethod", parameters, method_connection); 
+
+ Utils::WaitUntil(got_result_return);
+ Utils::WaitUntil(got_signal_return);
+ 
+ EXPECT_EQ(returned_result, expected_return);
+ EXPECT_EQ(returned_signal, expected_return);
+}
+
+TEST_F(TestGDBusProxy, TestCancelling)
+{
+ std::string call_return;
+ // method callback
+ auto method_connection = [&](GVariant *variant)
 {
- g_main_loop_quit(loop_);
- return FALSE;
+ if (variant != nullptr)
+ {
+ call_return = g_variant_get_string(g_variant_get_child_value(variant, 0), NULL);
+ }
+
+ got_result_return = true;
 };
- 
- guint bailout_source = g_timeout_add(10000, timeout_bailout, this);
 
- EXPECT_EQ(proxy->IsConnected(), true); // fail if we are not connected
- proxy->Connect("TestSignal", signal_connection);
- proxy->Call("TestMethod", parameters, method_connection); 
+ EXPECT_FALSE(proxy.IsConnected()); // we shouldn't be connected yet
+ glib::Object<GCancellable> cancellable(g_cancellable_new());
+ // but this has to work eitherway
+ proxy.Call("TestMethod", g_variant_new("(s)", "TestStringTestString"),
+ method_connection, cancellable);
 
- 
- // next check we get 30 entries from this specific known callback
- g_main_loop_run(loop_);
+ // this could mostly cause the next test to fail
+ g_cancellable_cancel(cancellable);
+ EXPECT_FALSE(got_result_return);
+}
 
- EXPECT_EQ(g_strcmp0(expected_return, returned_result), 0);
- EXPECT_EQ(g_strcmp0(expected_return, returned_signal), 0);
+TEST_F(TestGDBusProxy, TestMethodCall)
+{
+ std::string call_return;
+ // method callback
+ auto method_connection = [&](GVariant *variant)
+ {
+ if (variant != nullptr)
+ {
+ call_return = g_variant_get_string(g_variant_get_child_value(variant, 0), NULL);
+ }
 
- g_free(returned_result);
- g_free(returned_signal);
- g_source_remove(bailout_source);
+ got_result_return = true;
+ };
+
+ EXPECT_FALSE(proxy.IsConnected()); // we shouldn't be connected yet
+ // but this has to work eitherway
+ proxy.Call("TestMethod", g_variant_new("(s)", "TestStringTestString"),
+ method_connection);
+
+ Utils::WaitUntil(got_result_return);
+ 
+ EXPECT_TRUE(proxy.IsConnected());
+ EXPECT_EQ("TestStringTestString", call_return);
 }
 
 
diff --git a/tests/test_home_lens.cpp b/tests/test_home_lens.cpp
index 2cbd9d53a..f409d5325 100644
--- a/tests/test_home_lens.cpp
+++ b/tests/test_home_lens.cpp
@@ -10,6 +10,7 @@
 #include <sigc++/trackable.h>
 
 #include <UnityCore/GLibWrapper.h>
+#include <UnityCore/GLibSource.h>
 #include <UnityCore/Variant.h>
 #include <UnityCore/HomeLens.h>
 #include <UnityCore/Lens.h>
@@ -30,14 +31,6 @@ namespace
 
 class StaticTestLens;
 
-typedef struct {
- StaticTestLens* lens;
- gchar* search_string;
-} LensSearchClosure;
-
-static gboolean dispatch_global_search(gpointer userdata);
-
-
 /*
 * Mock Lens instance that does not use DBus. The default search does like this:
 * For input "bar" output:
@@ -134,22 +127,27 @@ public:
 }
 
 g_free(row_buf);
-
- global_search_finished.emit(Hints());
 }
 
- void GlobalSearch(string const& search_string)
+ void GlobalSearch(string const& search_string, SearchFinishedCallback cb)
 {
 /* Dispatch search async, because that's what it'd normally do */
- LensSearchClosure* closure = g_new0(LensSearchClosure, 1);
- closure->lens = this;
- closure->search_string = g_strdup(search_string.c_str());
- g_idle_add(dispatch_global_search, closure);
+ source_manager_.Add(new glib::Idle([this, search_string, cb] ()
+ {
+ DoGlobalSearch(search_string);
+ cb(Lens::Hints(), glib::Error());
+ return false;
+ }));
 }
 
- void Search(string const& search_string)
+ void Search(string const& search_string, SearchFinishedCallback cb)
 {
-
+ /* Dispatch search async, because that's what it'd normally do */
+ source_manager_.Add(new glib::Idle([search_string, cb] ()
+ {
+ cb(Lens::Hints(), glib::Error());
+ return false;
+ }));
 }
 
 void Activate(string const& uri)
@@ -181,20 +179,9 @@ private:
 string results_base_name_;
 int num_results_;
 bool provides_personal_results_;
+ glib::SourceManager source_manager_;
 };
 
-static gboolean dispatch_global_search(gpointer userdata)
-{
- LensSearchClosure* closure = (LensSearchClosure*) userdata;
-
- closure->lens->DoGlobalSearch(closure->search_string);
-
- g_free(closure->search_string);
- g_free(closure);
-
- return FALSE;
-}
-
 /*
 * Mock Lenses class
 */
@@ -408,10 +395,8 @@ TEST(TestHomeLens, TestOneSearch)
 
 home_lens_.AddLenses(lenses_);
 
- home_lens_.Search("ape");
-
 bool finished = false;
- home_lens_.search_finished.connect([&finished] (Lens::Hints const& hints)
+ home_lens_.Search("ape", [&finished] (Lens::Hints const&, glib::Error const&)
 {
 finished = true;
 });
@@ -473,10 +458,8 @@ TEST(TestHomeLens, TestOrderingAfterSearch)
 order_changed = true;
 });
 
- home_lens_.Search("ape");
-
 bool finished = false;
- home_lens_.search_finished.connect([&finished] (Lens::Hints const& hints)
+ home_lens_.Search("ape", [&finished] (Lens::Hints const&, glib::Error const&)
 {
 finished = true;
 });
@@ -523,10 +506,8 @@ TEST(TestHomeLens, TestOrderingWithExactAppsMatch)
 order_changed = true;
 });
 
- home_lens_.Search("ape");
-
 bool finished = false;
- home_lens_.search_finished.connect([&finished] (Lens::Hints const& hints)
+ home_lens_.Search("ape", [&finished] (Lens::Hints const&, glib::Error const&)
 {
 finished = true;
 });
@@ -570,10 +551,8 @@ TEST(TestHomeLens, TestOrderingWithoutExactAppsMatch)
 order_changed = true;
 });
 
- home_lens_.Search("ape");
-
 bool finished = false;
- home_lens_.search_finished.connect([&finished] (Lens::Hints const& hints)
+ home_lens_.Search("ape", [&finished] (Lens::Hints const&, glib::Error const&)
 {
 finished = true;
 });
@@ -623,10 +602,8 @@ TEST(TestHomeLens, TestOrderingByNumResults)
 order_changed = true;
 });
 
- home_lens_.Search("ape");
-
 bool finished = false;
- home_lens_.search_finished.connect([&finished] (Lens::Hints const& hints)
+ home_lens_.Search("ape", [&finished] (Lens::Hints const&, glib::Error const&)
 {
 finished = true;
 });
@@ -691,10 +668,8 @@ TEST(TestHomeLens, TestPersonalResultsFirst)
 order_changed = true;
 });
 
- home_lens_.Search("ape");
-
 bool finished = false;
- home_lens_.search_finished.connect([&finished] (Lens::Hints const& hints)
+ home_lens_.Search("ape", [&finished] (Lens::Hints const&, glib::Error const&)
 {
 finished = true;
 });
@@ -752,10 +727,8 @@ TEST(TestHomeLens, TestNonPersonalCategoriesBeforeEmpty)
 order_changed = true;
 });
 
- home_lens_.Search("ape");
-
 bool finished = false;
- home_lens_.search_finished.connect([&finished] (Lens::Hints const& hints)
+ home_lens_.Search("ape", [&finished] (Lens::Hints const&, glib::Error const&)
 {
 finished = true;
 });
diff --git a/tests/test_lens.cpp b/tests/test_lens.cpp
index 79decb031..629ab52f1 100644
--- a/tests/test_lens.cpp
+++ b/tests/test_lens.cpp
@@ -86,7 +86,7 @@ public:
 {
 Results::Ptr results = lens_->results;
 
- lens_->Search("Test Search String");
+ lens_->Search("Test Search String", nullptr);
 WaitForModel<Result>(results.get(), 5);
 
 Result result = results->RowAtIndex(0);
@@ -145,7 +145,7 @@ TEST_F(TestLens, TestSearch)
 {
 Results::Ptr results = lens_->results;
 
- lens_->Search("Test Search String");
+ lens_->Search("Test Search String", nullptr);
 WaitForModel<Result>(results.get(), 5);
 
 for (unsigned int i = 0; i < 5; i++)
@@ -169,7 +169,7 @@ TEST_F(TestLens, TestGlobalSearch)
 {
 Results::Ptr results = lens_->global_results;
 
- lens_->GlobalSearch("Test Global Search String");
+ lens_->GlobalSearch("Test Global Search String", nullptr);
 WaitForModel<Result>(results.get(), 10);
 
 for (unsigned int i = 0; i < 10; i++)