From d4e82e7d4fa1937d27d1397a036af2b6b03349b3 Mon Sep 17 00:00:00 2001
From: Charles Kerr <charles.kerr@canonical.com>
Date: Thu, 21 Apr 2016 09:33:08 -0500
Subject: code cleanup to prepare for MR

---
 src/greeter.cpp             | 15 ++++++---------
 src/greeter.h               |  2 +-
 tests/unit/greeter-test.cpp | 40 +++++++++++++++-------------------------
 3 files changed, 22 insertions(+), 35 deletions(-)

diff --git a/src/greeter.cpp b/src/greeter.cpp
index 7f79bf9..d41160f 100644
--- a/src/greeter.cpp
+++ b/src/greeter.cpp
@@ -26,13 +26,13 @@ class UnityGreeter::Impl
 {
 public:
 
-    explicit Impl()
+    Impl()
     {
         m_cancellable.reset(
             g_cancellable_new(),
-            [](GCancellable* c){
-                g_cancellable_cancel(c);
-                g_clear_object(&c);
+            [](GCancellable* o){
+                g_cancellable_cancel(o);
+                g_clear_object(&o);
             }
         );
 
@@ -154,7 +154,7 @@ private:
     }
 
     static void on_properties_changed_signal(
-        GDBusConnection* /*connection*/,
+        GDBusConnection* /*bus*/,
         const gchar* sender_name,
         const gchar* object_path,
         const gchar* interface_name,
@@ -171,7 +171,6 @@ private:
         g_return_if_fail(g_variant_is_of_type(parameters, G_VARIANT_TYPE(DBusNames::Properties::PropertiesChanged::ARGS_VARIANT_TYPE)));
 
         auto v = g_variant_get_child_value(parameters, 1);
-
         gboolean is_active {};
         if (g_variant_lookup(v, "IsActive", "b", &is_active))
         {
@@ -179,13 +178,11 @@ private:
             self->m_is_active.set(is_active);
         }
         g_clear_pointer(&v, g_variant_unref);
-
     }
 
     core::Property<bool> m_is_active {false};
-
-    std::shared_ptr<GDBusConnection> m_bus;
     std::shared_ptr<GCancellable> m_cancellable;
+    std::shared_ptr<GDBusConnection> m_bus;
     std::string m_owner;
 };
 
diff --git a/src/greeter.h b/src/greeter.h
index 9508220..e084d25 100644
--- a/src/greeter.h
+++ b/src/greeter.h
@@ -36,7 +36,7 @@ public:
 class UnityGreeter: public Greeter
 {
 public:
-    explicit UnityGreeter();
+    UnityGreeter();
     virtual ~UnityGreeter();
     core::Property<bool>& is_active() override;
 
diff --git a/tests/unit/greeter-test.cpp b/tests/unit/greeter-test.cpp
index 874efed..b33c300 100644
--- a/tests/unit/greeter-test.cpp
+++ b/tests/unit/greeter-test.cpp
@@ -41,35 +41,19 @@ protected:
 
     std::shared_ptr<QtDBusTest::DBusTestRunner> m_dbus_runner;
     std::shared_ptr<QtDBusMock::DBusMock> m_dbus_mock;
-    GDBusConnection* m_session_bus {};
 
     void SetUp() override
     {
         super::SetUp();
   
-        // use a fresh bus for each test
- 
+        // use a fresh bus for each test run
         m_dbus_runner.reset(new QtDBusTest::DBusTestRunner());
         m_dbus_mock.reset(new QtDBusMock::DBusMock(*m_dbus_runner.get()));
-
-        GError* error {};
-        m_session_bus = g_bus_get_sync(G_BUS_TYPE_SESSION, nullptr, &error);
-        g_assert_no_error(error);
-        g_dbus_connection_set_exit_on_close(m_session_bus, false);
-    }
-
-    void TearDown() override
-    {
-        g_clear_object(&m_session_bus);
-        m_dbus_mock.reset();
-        m_dbus_runner.reset();
-
-        super::TearDown();
     }
 
     void start_greeter_service(bool is_active)
     {
-        // set a watcher to look for our mock to appear
+        // set a watcher to look for our mock greeter to appear
         bool owned {};
         QDBusServiceWatcher watcher(
             DBusNames::UnityGreeter::NAME,
@@ -92,19 +76,25 @@ protected:
         );
         m_dbus_runner->startServices();
 
-        // wait for the watcher to be triggered
+        // wait for the watcher
         ASSERT_TRUE(wait_for([&owned]{return owned;}));
     }
 };
 
-#define ASSERT_PROPERTY_EVENTUALLY(expected_in, property_in) \
+#define ASSERT_PROPERTY_EQ_EVENTUALLY(expected_in, property_in) \
     do { \
         const auto& e = expected_in; \
         const auto& p = property_in; \
-        ASSERT_TRUE(wait_for([e, &p](){ return e == p.get(); })) \
+        ASSERT_TRUE(wait_for([e, &p](){return e == p.get();})) \
             << "expected " << e << " but got " << p.get(); \
    } while(0)
 
+/**
+ * Test startup timing by looking at four different cases:
+ * [unity greeter shows up on bus (before, after) we start listening]
+ *   x [unity greeter is (active, inactive)]
+ */
+
 TEST_F(GreeterFixture, ActiveServiceStartsBeforeWatcher)
 {
     constexpr bool expected {true};
@@ -113,7 +103,7 @@ TEST_F(GreeterFixture, ActiveServiceStartsBeforeWatcher)
 
     UnityGreeter greeter;
 
-    ASSERT_PROPERTY_EVENTUALLY(expected, greeter.is_active());
+    ASSERT_PROPERTY_EQ_EVENTUALLY(expected, greeter.is_active());
 }
 
 TEST_F(GreeterFixture, WatcherStartsBeforeActiveService)
@@ -124,7 +114,7 @@ TEST_F(GreeterFixture, WatcherStartsBeforeActiveService)
 
     start_greeter_service(expected);
 
-    ASSERT_PROPERTY_EVENTUALLY(expected, greeter.is_active());
+    ASSERT_PROPERTY_EQ_EVENTUALLY(expected, greeter.is_active());
 }
 
 TEST_F(GreeterFixture, InactiveServiceStartsBeforeWatcher)
@@ -135,7 +125,7 @@ TEST_F(GreeterFixture, InactiveServiceStartsBeforeWatcher)
 
     UnityGreeter greeter;
 
-    ASSERT_PROPERTY_EVENTUALLY(expected, greeter.is_active());
+    ASSERT_PROPERTY_EQ_EVENTUALLY(expected, greeter.is_active());
 }
 
 TEST_F(GreeterFixture, WatcherStartsBeforeInactiveService)
@@ -146,6 +136,6 @@ TEST_F(GreeterFixture, WatcherStartsBeforeInactiveService)
 
     start_greeter_service(expected);
 
-    ASSERT_PROPERTY_EVENTUALLY(expected, greeter.is_active());
+    ASSERT_PROPERTY_EQ_EVENTUALLY(expected, greeter.is_active());
 }
 
-- 
cgit v1.2.3