Merge lp:~charlesk/indicator-power/use-dbusmock-notify-template into lp:indicator-power

[Charles Kerr]

In tests, use python-dbusmock's notification_daemon mock

No changes to production -- only to test scaffolding.

Use python-dbusmock's notification_daemon mock instead of rolling our own.

[Antti Kaijanmäki (kaijanmaki)]

LGTM.

review: Approve

[lp:~charlesk/indicator-power/use-dbusmock-notify-template]

300. By Charles Kerr on 2016-05-26

sync tests/CMakeLists.txt dependencies with debian/control

Unmerged revisions

300. By Charles Kerr on 2016-05-26

sync tests/CMakeLists.txt dependencies with debian/control

299. By Charles Kerr on 2016-05-25

remove the dbusmock startup script and instead invoke dbusmock ourselves

298. By Charles Kerr on 2016-05-25

reuse test-dbus-fixture.h from indicator-datetime/16.10

297. By Charles Kerr on 2016-05-25

sync with trunk/16.10

296. By Charles Kerr on 2015-11-29

in tests/test-notify, use dbusmock's notification_daemon template instead of rolling our own

295. By Charles Kerr on 2015-11-29

in tests/CMakeLists.txt, add SCRIPTS_DIR definition to so tests will know where to find start-mock-notifications.py

294. By Charles Kerr on 2015-11-29

add tests/start-mock-notifications.py, a simple dbusmock helper script that can be told to emulate unity7 or unity8 notification server capabilities

293. By Charles Kerr on 2015-11-29

add tests/dbus-fixture.h, a fixture that manages a dbus connection and has helper utils

292. By Charles Kerr on 2015-11-29

in tests/glib-fixture.h, rename the protected loop variable as 'm_loop' for naming consistency

291. By Charles Kerr on 2015-11-29

remove libdbustest, we're calling dbusmock directly so we can use templates with parameters

=== modified file 'debian/control'
--- debian/control	2016-05-16 18:09:57 +0000
+++ debian/control	2016-05-26 20:21:58 +0000
@@ -17,8 +17,6 @@
 # for tests
                libgtest-dev,
                python3-dbusmock,
-               dbus-test-runner,
-               libdbustest1-dev,
 Standards-Version: 3.9.5
 Homepage: https://launchpad.net/indicator-power
 # If you aren't a member of ~indicator-applet-developers but need to upload

=== modified file 'tests/CMakeLists.txt'
--- tests/CMakeLists.txt	2016-05-16 17:59:03 +0000
+++ tests/CMakeLists.txt	2016-05-26 20:21:58 +0000
@@ -5,11 +5,6 @@
 set_target_properties (gtest PROPERTIES INCLUDE_DIRECTORIES ${INCLUDE_DIRECTORIES} ${GTEST_INCLUDE_DIR})
 set_target_properties (gtest PROPERTIES COMPILE_FLAGS ${COMPILE_FLAGS} -w)
 
-# dbustest
-pkg_check_modules(DBUSTEST REQUIRED
-                  dbustest-1>=14.04.0)
-include_directories (SYSTEM ${DBUSTEST_INCLUDE_DIRS})
-
 # add warnings
 set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++11 -g ${C_WARNING_ARGS}")
 set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-weak-vtables -Wno-global-constructors") # Google Test

=== modified file 'tests/glib-fixture.h'
--- tests/glib-fixture.h	2016-05-16 21:28:14 +0000
+++ tests/glib-fixture.h	2016-05-26 20:21:58 +0000
@@ -44,7 +44,7 @@
     {
       setlocale(LC_ALL, "C.UTF-8");
 
-      loop = g_main_loop_new(nullptr, false);
+      m_loop = g_main_loop_new(nullptr, false);
 
       // only use local, temporary settings
       g_assert(g_setenv("GSETTINGS_SCHEMA_DIR", SCHEMA_DIR, true));
@@ -62,7 +62,7 @@
     {
       g_test_assert_expected_messages ();
 
-      g_clear_pointer(&loop, g_main_loop_unref);
+      g_clear_pointer(&m_loop, g_main_loop_unref);
     }
 
     void expectLogMessage (const gchar *domain, GLogLevelFlags level, const gchar *pattern)
@@ -94,11 +94,11 @@
       // wait for the signal or for timeout, whichever comes first
       const auto handler_id = g_signal_connect_swapped(o, signal,
                                                        G_CALLBACK(g_main_loop_quit),
-                                                       loop);
+                                                       m_loop);
       const auto timeout_id = g_timeout_add_seconds(timeout_seconds,
                                                     wait_for_signal__timeout,
-                                                    loop);
-      g_main_loop_run(loop);
+                                                    m_loop);
+      g_main_loop_run(m_loop);
       g_source_remove(timeout_id);
       g_signal_handler_disconnect(o, handler_id);
     }
@@ -106,8 +106,8 @@
     /* convenience func to loop for N msec */
     void wait_msec(int msec=50)
     {
-      const auto id = g_timeout_add(msec, wait_msec__timeout, loop);
-      g_main_loop_run(loop);
+      const auto id = g_timeout_add(msec, wait_msec__timeout, m_loop);
+      g_main_loop_run(m_loop);
       g_source_remove(id);
     }
 
@@ -148,8 +148,8 @@
         g_main_loop_quit(tmp->loop);
       };
 
-      const auto timeout_id = g_timeout_add(timeout_msec, wait_msec__timeout, loop);
-      data.loop = loop;
+      const auto timeout_id = g_timeout_add(timeout_msec, wait_msec__timeout, m_loop);
+      data.loop = m_loop;
       const auto watch_id = g_bus_watch_name_on_connection(
           connection,
           name,
@@ -160,7 +160,7 @@
           nullptr // user_data_free_func
       );
 
-      g_main_loop_run(loop);
+      g_main_loop_run(m_loop);
 
       g_bus_unwatch_name(watch_id);
       g_source_remove(timeout_id);
@@ -223,6 +223,6 @@
       );
     }
 
-    GMainLoop* loop {};
+    GMainLoop* m_loop {};
 };
 

=== added file 'tests/test-dbus-fixture.h'
--- tests/test-dbus-fixture.h	1970-01-01 00:00:00 +0000
+++ tests/test-dbus-fixture.h	2016-05-26 20:21:58 +0000
@@ -0,0 +1,108 @@
+/*
+ * Copyright 2013-2016 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 3, as published
+ * by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranties of
+ * MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
+ * PURPOSE.  See the GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authors:
+ *   Charles Kerr <[e-mail]>
+ */
+
+#pragma once
+
+#include "glib-fixture.h"
+
+/***
+****
+***/
+
+class TestDBusFixture: public GlibFixture
+{
+  public:
+
+    TestDBusFixture() =default;
+    virtual ~TestDBusFixture() =default;
+
+    explicit TestDBusFixture(const std::vector<std::string>& service_dirs_in):
+        service_dirs(service_dirs_in)
+    {
+    }
+
+  private:
+
+    typedef GlibFixture super;
+
+    static void
+    on_bus_opened (GObject* /*object*/, GAsyncResult * res, gpointer gself)
+    {
+      auto self = static_cast<TestDBusFixture*>(gself);
+
+      GError* err {};
+      self->m_bus = g_bus_get_finish (res, &err);
+      g_assert_no_error (err);
+
+      g_main_loop_quit (self->m_loop);
+    }
+
+    static void
+    on_bus_closed (GObject* /*object*/, GAsyncResult * res, gpointer gself)
+    {
+      auto self = static_cast<TestDBusFixture*>(gself);
+
+      GError* err {};
+      g_dbus_connection_close_finish (self->m_bus, res, &err);
+      g_assert_no_error (err);
+
+      g_main_loop_quit (self->m_loop);
+    }
+
+  protected:
+
+    GTestDBus * m_test_dbus {};
+    GDBusConnection * m_bus {};
+    const std::vector<std::string> service_dirs;
+
+    virtual void SetUp() override
+    {
+      super::SetUp ();
+
+      // pull up a test dbus
+      m_test_dbus = g_test_dbus_new (G_TEST_DBUS_NONE);
+      for (const auto& dir : service_dirs)
+        g_test_dbus_add_service_dir (m_test_dbus, dir.c_str());
+      g_test_dbus_up (m_test_dbus);
+      const char * address = g_test_dbus_get_bus_address (m_test_dbus);
+      g_setenv ("DBUS_SYSTEM_BUS_ADDRESS", address, true);
+      g_setenv ("DBUS_SESSION_BUS_ADDRESS", address, true);
+      g_debug ("test_dbus's address is %s", address);
+
+      // wait for the GDBusConnection before returning
+      g_bus_get (G_BUS_TYPE_SYSTEM, nullptr, on_bus_opened, this);
+      g_main_loop_run (m_loop);
+    }
+
+    virtual void TearDown() override
+    {
+      wait_msec();
+
+      // close the bus
+      g_dbus_connection_close(m_bus, nullptr, on_bus_closed, this);
+      g_main_loop_run(m_loop);
+      g_clear_object(&m_bus);
+
+      // tear down the test dbus
+      g_test_dbus_down(m_test_dbus);
+      g_clear_object(&m_test_dbus);
+
+      super::TearDown();
+    }
+};

=== modified file 'tests/test-notify.cc'
--- tests/test-notify.cc	2016-05-16 17:59:03 +0000
+++ tests/test-notify.cc	2016-05-26 20:21:58 +0000
@@ -17,8 +17,7 @@
  *   Charles Kerr <[e-mail]>
  */
 
-
-#include "glib-fixture.h"
+#include "test-dbus-fixture.h"
 
 #include "dbus-shared.h"
 #include "device.h"
@@ -26,181 +25,107 @@
 
 #include <gtest/gtest.h>
 
-#include <libdbustest/dbus-test.h>
-
 #include <libnotify/notify.h>
 
-#include <glib.h>
 #include <gio/gio.h>
 
 /***
 ****
 ***/
 
-class NotifyFixture: public GlibFixture
+class NotifyFixture: public TestDBusFixture
 {
-private:
-
-  typedef GlibFixture super;
-
-  static constexpr char const * NOTIFY_BUSNAME   {"org.freedesktop.Notifications"};
-  static constexpr char const * NOTIFY_INTERFACE {"org.freedesktop.Notifications"};
-  static constexpr char const * NOTIFY_PATH      {"/org/freedesktop/Notifications"};
-
-protected:
-
-  DbusTestService * service = nullptr;
-  DbusTestDbusMock * mock = nullptr;
-  DbusTestDbusMockObject * obj = nullptr;
-  GDBusConnection * bus = nullptr;
-
-  static constexpr int FIRST_NOTIFY_ID {1234};
-
-  static constexpr int NOTIFICATION_CLOSED_EXPIRED   {1};
-  static constexpr int NOTIFICATION_CLOSED_DISMISSED {2};
-  static constexpr int NOTIFICATION_CLOSED_API       {3};
-  static constexpr int NOTIFICATION_CLOSED_UNDEFINED {4};
-
-  static constexpr char const * METHOD_CLOSE {"CloseNotification"};
-  static constexpr char const * METHOD_NOTIFY {"Notify"};
-  static constexpr char const * METHOD_GET_CAPS {"GetCapabilities"};
-  static constexpr char const * METHOD_GET_INFO {"GetServerInformation"};
-  static constexpr char const * SIGNAL_CLOSED {"NotificationClosed"};
-
-  static constexpr char const * HINT_TIMEOUT {"x-canonical-snap-decisions-timeout"};
-
-protected:
-
-  void SetUp()
-  {
-    super::SetUp();
-
-    g_setenv ("XDG_DATA_HOME", XDG_DATA_HOME, TRUE);
-
-    // init DBusMock / dbus-test-runner
-
-    service = dbus_test_service_new(nullptr);
-
-    GError * error = nullptr;
-    mock = dbus_test_dbus_mock_new(NOTIFY_BUSNAME);
-    obj = dbus_test_dbus_mock_get_object(mock,
-                                         NOTIFY_PATH,
-                                         NOTIFY_INTERFACE,
-                                         &error);
-    g_assert_no_error (error);
-  
-    // METHOD_GET_INFO 
-    dbus_test_dbus_mock_object_add_method(mock, obj, METHOD_GET_INFO,
-                                          nullptr,
-                                          G_VARIANT_TYPE("(ssss)"),
-                                          "ret = ('mock-notify', 'test vendor', '1.0', '1.1')",
-                                          &error);
-    g_assert_no_error (error);
-
-    // METHOD_NOTIFY
-    auto str = g_strdup_printf("try:\n"
-                               "  self.NextNotifyId\n"
-                               "except AttributeError:\n"
-                               "  self.NextNotifyId = %d\n"
-                               "ret = self.NextNotifyId\n"
-                               "self.NextNotifyId += 1\n",
-                               FIRST_NOTIFY_ID);
-    dbus_test_dbus_mock_object_add_method(mock, obj, METHOD_NOTIFY,
-                                          G_VARIANT_TYPE("(susssasa{sv}i)"),
-                                          G_VARIANT_TYPE_UINT32,
-                                          str,
-                                          &error);
-    g_assert_no_error (error);
-    g_free (str);
-
-    // METHOD_CLOSE 
-    str = g_strdup_printf("self.EmitSignal('%s', '%s', 'uu', [ args[0], %d ])",
-                          NOTIFY_INTERFACE,
-                          SIGNAL_CLOSED,
-                          NOTIFICATION_CLOSED_API);
-    dbus_test_dbus_mock_object_add_method(mock, obj, METHOD_CLOSE,
-                                          G_VARIANT_TYPE("(u)"),
-                                          nullptr,
-                                          str,
-                                          &error);
-    g_assert_no_error (error);
-    g_free (str);
-
-    dbus_test_service_add_task(service, DBUS_TEST_TASK(mock));
-    dbus_test_service_start_tasks(service);
-
-    bus = g_bus_get_sync(G_BUS_TYPE_SESSION, nullptr, nullptr);
-    g_dbus_connection_set_exit_on_close(bus, FALSE);
-    g_object_add_weak_pointer(G_OBJECT(bus), reinterpret_cast<gpointer*>(&bus));
-
-    notify_init(SERVICE_EXEC);
-  }
-
-  virtual void TearDown()
-  {
-    notify_uninit();
-
-    g_clear_object(&mock);
-    g_clear_object(&service);
-    g_object_unref(bus);
-
-    // wait a little while for the scaffolding to shut down,
-    // but don't block on it forever...
-    unsigned int cleartry = 0;
-    while ((bus != nullptr) && (cleartry < 50))
-      {
-        g_usleep(100000);
-        while (g_main_pending())
-          g_main_iteration(true);
-        cleartry++;
-      }
-
-    super::TearDown();
-  }
-
-  /***
-  ****
-  ***/
-
-  int get_notify_call_count() const
-  {
-    guint len {0u};
-    GError* error {nullptr};
-    dbus_test_dbus_mock_object_get_method_calls(mock, obj, METHOD_NOTIFY, &len, &error);
-    g_assert_no_error(error);
-    return len;
-  }
-
-  std::string get_notify_call_sound_file(int call_number)
-  {
-    std::string ret;
-
-    guint len {0u};
-    GError* error {nullptr};
-    auto calls = dbus_test_dbus_mock_object_get_method_calls(mock, obj, METHOD_NOTIFY, &len, &error);
-    g_return_val_if_fail(int(len) > call_number, ret);
-    g_assert_no_error(error);
-
-    constexpr int HINTS_PARAM_POSITION {6};
-    const auto& call = calls[call_number];
-    g_return_val_if_fail(g_variant_n_children(call.params) > HINTS_PARAM_POSITION, ret);
-    auto hints = g_variant_get_child_value(call.params, HINTS_PARAM_POSITION);
-    const gchar* sound_file = nullptr;
-    auto success = g_variant_lookup(hints, "sound-file", "&s", &sound_file);
-    g_return_val_if_fail(success, ret);
-    if (sound_file != nullptr)
-      ret = sound_file;
-    g_clear_pointer(&hints, g_variant_unref);
-
-    return ret;
-  }
-
-  void clear_method_calls()
-  {
-    GError* error{nullptr};
-    ASSERT_TRUE(dbus_test_dbus_mock_object_clear_method_calls(mock, obj, &error));
-    g_assert_no_error(error);
-  }
+    typedef TestDBusFixture super;
+
+    void start_unity7_notifications()
+    {
+        start_notifications("{ \"capabilities\": \""
+            "append "
+            "body "
+            "body-markup "
+            "icon-static "
+            "image/svg+xml "
+            "private-icon-only "
+            "private-synchronous "
+            "truncation "
+            "x-canonical-append "
+            "x-canonical-private-icon-only "
+            "x-canonical-private-synchronous "
+            "x-canonical-truncation"
+            "\" }"
+        );
+    }
+
+    void start_unity8_notifications()
+    {
+        start_notifications("{ \"capabilities\": \""
+            "body "
+            "body-markup "
+            "icon-static "
+            "image/svg+xml "
+            "sound-file "
+            "suppress-sound "
+            "urgency "
+            "value "
+            "x-canonical-non-shaped-icon "
+            "x-canonical-private-affirmative-tint "
+            "x-canonical-private-icon-only "
+            "x-canonical-private-menu-model "
+            "x-canonical-private-rejection-tint "
+            "x-canonical-private-synchronous "
+            "x-canonical-secondary-icon "
+            "x-canonical-snap-decisions "
+            "x-canonical-snap-decisions-swipe "
+            "x-canonical-snap-decisions-timeout "
+            "x-canonical-switch-to-application "
+            "x-canonical-truncation "
+            "x-canonical-value-bar-tint"
+            "\" }"
+        );
+    }
+
+    void start_notifications(const char* parameters)
+    {
+        std::array<const char*,8> child_argv = {
+            "python3", "-m", "dbusmock",
+            "--template", "notification_daemon",
+            "--parameters", parameters,
+            nullptr
+        };
+        GError* error {};
+        g_spawn_async(
+            nullptr /*working directory*/,
+            const_cast<char**>(child_argv.data()),
+            nullptr /*envp*/,
+            G_SPAWN_SEARCH_PATH,
+            nullptr /*child_setup*/,
+            nullptr /*user_data*/,
+            nullptr /*child_pid*/,
+            &error
+        );
+        g_assert_no_error(error);
+
+        wait_for_name_owned(m_bus, "org.freedesktop.Notifications");
+    }
+
+
+protected:
+
+    void SetUp() override
+    {
+        super::SetUp();
+
+        start_unity8_notifications();
+
+        notify_init(G_LOG_DOMAIN);
+    }
+
+    void TearDown() override
+    {
+        notify_uninit();
+
+        super::TearDown();
+    }
 };
 
 /***
@@ -216,7 +141,6 @@
 ****
 ***/
 
-
 namespace
 {
   static constexpr double percent_critical {2.0};
@@ -319,11 +243,11 @@
   // charge should show up on the bus.
   auto notifier = indicator_power_notifier_new ();
   indicator_power_notifier_set_battery (notifier, battery);
-  indicator_power_notifier_set_bus (notifier, bus);
+  indicator_power_notifier_set_bus (notifier, m_bus);
   wait_msec();
 
   ChangedParams changed_params;
-  auto sub_tag = g_dbus_connection_signal_subscribe (bus,
+  auto sub_tag = g_dbus_connection_signal_subscribe (m_bus,
                                                      nullptr,
                                                      "org.freedesktop.DBus.Properties",
                                                      "PropertiesChanged",
@@ -358,9 +282,9 @@
     }
 
   // cleanup
-  g_dbus_connection_signal_unsubscribe (bus, sub_tag);
+  g_dbus_connection_signal_unsubscribe (m_bus, sub_tag);
+  g_object_unref (notifier);
   g_object_unref (battery);
-  g_object_unref (notifier);
 }
 
 /***
@@ -369,18 +293,6 @@
 
 TEST_F(NotifyFixture, EventsThatChangeNotifications)
 {
-  // GetCapabilities returns an array containing 'actions', so that we'll
-  // get snap decisions and the 'IsWarning' property
-  GError * error = nullptr;
-  dbus_test_dbus_mock_object_add_method (mock,
-                                         obj,
-                                         METHOD_GET_CAPS,
-                                         nullptr,
-                                         G_VARIANT_TYPE_STRING_ARRAY,
-                                         "ret = ['actions', 'body']",
-                                         &error);
-  g_assert_no_error (error);
-
   auto battery = indicator_power_device_new ("/object/path",
                                              UP_DEVICE_KIND_BATTERY,
                                              percent_low + 1.0,
@@ -397,9 +309,9 @@
   // charge should show up on the bus.
   auto notifier = indicator_power_notifier_new ();
   indicator_power_notifier_set_battery (notifier, battery);
-  indicator_power_notifier_set_bus (notifier, bus);
+  indicator_power_notifier_set_bus (notifier, m_bus);
   ChangedParams changed_params;
-  auto sub_tag = g_dbus_connection_signal_subscribe (bus,
+  auto sub_tag = g_dbus_connection_signal_subscribe (m_bus,
                                                      nullptr,
                                                      "org.freedesktop.DBus.Properties",
                                                      "PropertiesChanged",
@@ -423,9 +335,6 @@
   EXPECT_EQ (FIELD_POWER_LEVEL|FIELD_IS_WARNING, changed_params.fields);
   EXPECT_EQ (indicator_power_notifier_get_power_level(battery), changed_params.power_level);
   EXPECT_TRUE (changed_params.is_warning);
-  EXPECT_EQ (1, get_notify_call_count());
-  EXPECT_EQ (low_power_uri, get_notify_call_sound_file(0));
-  clear_method_calls();
 
   // now test that the warning changes if the level goes down even lower...
   changed_params = ChangedParams();
@@ -433,9 +342,6 @@
   wait_msec();
   EXPECT_EQ (FIELD_POWER_LEVEL, changed_params.fields);
   EXPECT_STREQ (POWER_LEVEL_STR_VERY_LOW, changed_params.power_level.c_str());
-  EXPECT_EQ (1, get_notify_call_count());
-  EXPECT_EQ (low_power_uri, get_notify_call_sound_file(0));
-  clear_method_calls();
 
   // ...and that the warning is taken down if the battery is plugged back in...
   changed_params = ChangedParams();
@@ -443,7 +349,6 @@
   wait_msec();
   EXPECT_EQ (FIELD_IS_WARNING, changed_params.fields);
   EXPECT_FALSE (changed_params.is_warning);
-  EXPECT_EQ (0, get_notify_call_count());
 
   // ...and that it comes back if we unplug again...
   changed_params = ChangedParams();
@@ -451,9 +356,6 @@
   wait_msec();
   EXPECT_EQ (FIELD_IS_WARNING, changed_params.fields);
   EXPECT_TRUE (changed_params.is_warning);
-  EXPECT_EQ (1, get_notify_call_count());
-  EXPECT_EQ (low_power_uri, get_notify_call_sound_file(0));
-  clear_method_calls();
 
   // ...and that it's taken down if the power level is OK
   changed_params = ChangedParams();
@@ -462,10 +364,9 @@
   EXPECT_EQ (FIELD_POWER_LEVEL|FIELD_IS_WARNING, changed_params.fields);
   EXPECT_STREQ (POWER_LEVEL_STR_OK, changed_params.power_level.c_str());
   EXPECT_FALSE (changed_params.is_warning);
-  EXPECT_EQ (0, get_notify_call_count());
 
   // cleanup
-  g_dbus_connection_signal_unsubscribe (bus, sub_tag);
+  g_dbus_connection_signal_unsubscribe (m_bus, sub_tag);
   g_object_unref (notifier);
   g_object_unref (battery);
 }