for low power notifications, use libnotify's 'sound-file' property instead of indicator-power calling the sound player directly

4 files changed, 63 insertions, 205 deletions

diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
index 41e48cd..566132e 100644
--- a/tests/CMakeLists.txt
+++ b/tests/CMakeLists.txt
@@ -9,13 +9,6 @@ include_directories (SYSTEM ${DBUSTEST_INCLUDE_DIRS})
 # add warnings
 set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++11 ${C_WARNING_ARGS}")
 
-# build the mocks
-set(MOCK_LIB "indicatorpowerservicemocks")
-set(MOCK_SOURCES sound-player-mock.c)
-set_source_files_properties(${MOCK_SOURCES}
-                            PROPERTIES COMPILE_FLAGS "${C_WARNING_ARGS} -g -std=c99")
-add_library(${MOCK_LIB} STATIC ${MOCK_SOURCES})
-
 # build the necessary schemas
 set_directory_properties (PROPERTIES
                           ADDITIONAL_MAKE_CLEAN_FILES gschemas.compiled)
@@ -57,8 +50,8 @@ function(add_test_by_name name)
   add_executable (${TEST_NAME} ${TEST_NAME}.cc)
   target_link_options(${TEST_NAME} PRIVATE -no-pie)
   add_test (${TEST_NAME} ${TEST_NAME})
-  add_dependencies (${TEST_NAME} ${MOCK_LIB} ayatanaindicatorpowerservice gschemas-compiled)
-  target_link_libraries (${TEST_NAME} ${MOCK_LIB} ayatanaindicatorpowerservice gtest ${DBUSTEST_LIBRARIES} ${SERVICE_DEPS_LIBRARIES} ${GTEST_LIBS} ${URLDISPATCHER_LIBRARIES} ${GMOCK_LIBRARIES})
+  add_dependencies (${TEST_NAME} ayatanaindicatorpowerservice gschemas-compiled)
+  target_link_libraries (${TEST_NAME} ayatanaindicatorpowerservice gtest ${DBUSTEST_LIBRARIES} ${SERVICE_DEPS_LIBRARIES} ${GTEST_LIBS} ${URLDISPATCHER_LIBRARIES} ${GMOCK_LIBRARIES})
 endfunction()
 add_test_by_name(test-notify)
 add_test(NAME dear-reader-the-next-test-takes-80-seconds COMMAND true)
diff --git a/tests/sound-player-mock.c b/tests/sound-player-mock.c
deleted file mode 100644
index 35c3b57..0000000
--- a/tests/sound-player-mock.c
+++ /dev/null
@@ -1,94 +0,0 @@
-/*
- * Copyright 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 <charles.kerr@canonical.com>
- */
-
-#include "sound-player.h"
-#include "sound-player-mock.h"
-
-enum
-{
-  SIGNAL_URI_PLAYED,
-  LAST_SIGNAL
-};
-
-static guint signals[LAST_SIGNAL] = { 0 };
-
-/***
-****  GObject boilerplate
-***/
-
-static void indicator_power_sound_player_interface_init (
-                                IndicatorPowerSoundPlayerInterface * iface);
-
-G_DEFINE_TYPE_WITH_CODE (
-  IndicatorPowerSoundPlayerMock,
-  indicator_power_sound_player_mock,
-  G_TYPE_OBJECT,
-  G_IMPLEMENT_INTERFACE (INDICATOR_TYPE_POWER_SOUND_PLAYER,
-                         indicator_power_sound_player_interface_init))
-
-/***
-****  IndicatorPowerSoundPlayer virtual functions
-***/
-
-static void
-my_play_uri (IndicatorPowerSoundPlayer * self, const gchar * uri)
-{
-  g_signal_emit (self, signals[SIGNAL_URI_PLAYED], 0, uri, NULL);
-}
-
-/***
-****  Instantiation
-***/
-
-static void
-indicator_power_sound_player_mock_class_init (IndicatorPowerSoundPlayerMockClass * klass G_GNUC_UNUSED)
-{
-  signals[SIGNAL_URI_PLAYED] = g_signal_new (
-    "uri-played",
-    G_TYPE_FROM_CLASS(klass),
-    G_SIGNAL_RUN_LAST,
-    G_STRUCT_OFFSET (IndicatorPowerSoundPlayerMockClass, uri_played),
-    NULL, NULL,
-    g_cclosure_marshal_VOID__STRING,
-    G_TYPE_NONE, 1, G_TYPE_STRING);
-}
-
-static void
-indicator_power_sound_player_interface_init (IndicatorPowerSoundPlayerInterface * iface)
-{
-  iface->play_uri = my_play_uri;
-}
-
-static void
-indicator_power_sound_player_mock_init (IndicatorPowerSoundPlayerMock * self G_GNUC_UNUSED)
-{
-}
-
-/***
-****  Public API
-***/
-
-IndicatorPowerSoundPlayer *
-indicator_power_sound_player_mock_new (void)
-{
-  gpointer o = g_object_new (INDICATOR_TYPE_POWER_SOUND_PLAYER_MOCK, NULL);
-
-  return INDICATOR_POWER_SOUND_PLAYER (o);
-}
-
diff --git a/tests/sound-player-mock.h b/tests/sound-player-mock.h
deleted file mode 100644
index f844924..0000000
--- a/tests/sound-player-mock.h
+++ /dev/null
@@ -1,75 +0,0 @@
-/*
- * Copyright 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 <charles.kerr@canonical.com>
- */
-
-#ifndef __INDICATOR_POWER_SOUND_PLAYER_MOCK__H__
-#define __INDICATOR_POWER_SOUND_PLAYER_MOCK__H__
-
-#include <glib-object.h> /* parent class */
-
-#include "sound-player.h"
-
-G_BEGIN_DECLS
-
-#define INDICATOR_TYPE_POWER_SOUND_PLAYER_MOCK \
-  (indicator_power_sound_player_mock_get_type())
-
-#define INDICATOR_POWER_SOUND_PLAYER_MOCK(o) \
-  (G_TYPE_CHECK_INSTANCE_CAST ((o), \
-                               INDICATOR_TYPE_POWER_SOUND_PLAYER_MOCK, \
-                               IndicatorPowerSoundPlayerMock))
-
-#define INDICATOR_POWER_SOUND_PLAYER_MOCK_GET_CLASS(o) \
- (G_TYPE_INSTANCE_GET_CLASS ((o), \
-                             INDICATOR_TYPE_POWER_SOUND_PLAYER_MOCK, \
-                             IndicatorPowerSoundPlayerMockClass))
-
-#define INDICATOR_IS_POWER_SOUND_PLAYER_MOCK(o) \
-  (G_TYPE_CHECK_INSTANCE_TYPE ((o), \
-                               INDICATOR_TYPE_POWER_SOUND_PLAYER_MOCK))
-
-typedef struct _IndicatorPowerSoundPlayerMock
-                IndicatorPowerSoundPlayerMock;
-typedef struct _IndicatorPowerSoundPlayerMockPriv
-                IndicatorPowerSoundPlayerMockPriv;
-typedef struct _IndicatorPowerSoundPlayerMockClass
-                IndicatorPowerSoundPlayerMockClass;
-
-/**
- * An IndicatorPowerSoundPlayer which gets its devices from Mock.
- */
-struct _IndicatorPowerSoundPlayerMock
-{
-  GObject parent_instance;
-};
-
-struct _IndicatorPowerSoundPlayerMockClass
-{
-  GObjectClass parent_class;
-
-  /* signals */
-  void (*uri_played) (IndicatorPowerSoundPlayer * self, const gchar* uri);
-};
-
-GType indicator_power_sound_player_mock_get_type (void);
-
-IndicatorPowerSoundPlayer * indicator_power_sound_player_mock_new (void);
-
-G_END_DECLS
-
-#endif /* __INDICATOR_POWER_SOUND_PLAYER_MOCK__H__ */
diff --git a/tests/test-notify.cc b/tests/test-notify.cc
index 0d92d7a..d01ef5f 100644
--- a/tests/test-notify.cc
+++ b/tests/test-notify.cc
@@ -23,7 +23,6 @@
 #include "dbus-shared.h"
 #include "device.h"
 #include "notifier.h"
-#include "sound-player-mock.h"
 
 #include <gtest/gtest.h>
 
@@ -160,6 +159,51 @@ protected:
 
     super::TearDown();
   }
+
+  /***
+  ****
+  ***/
+
+  int get_notify_call_count() const
+  {
+    // confirm that we got exactly one call
+    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);
+  }
 };
 
 /***
@@ -278,8 +322,7 @@ TEST_F(NotifyFixture, LevelsDuringBatteryDrain)
 
   // set up a notifier and give it the battery so changing the battery's
   // charge should show up on the bus.
-  auto sound_player = indicator_power_sound_player_mock_new ();
-  auto notifier = indicator_power_notifier_new (sound_player);
+  auto notifier = indicator_power_notifier_new ();
   indicator_power_notifier_set_battery (notifier, battery);
   indicator_power_notifier_set_bus (notifier, bus);
   wait_msec();
@@ -323,21 +366,12 @@ TEST_F(NotifyFixture, LevelsDuringBatteryDrain)
   g_dbus_connection_signal_unsubscribe (bus, sub_tag);
   g_object_unref (battery);
   g_object_unref (notifier);
-  g_object_unref (sound_player);
 }
 
 /***
 ****
 ***/
 
-namespace
-{
-  static void on_uri_played(IndicatorPowerSoundPlayer*, const char* uri, gpointer glast_uri)
-  {
-    *static_cast<std::string*>(glast_uri) = uri;
-  }
-}
-
 TEST_F(NotifyFixture, EventsThatChangeNotifications)
 {
   // GetCapabilities returns an array containing 'actions', so that we'll
@@ -367,10 +401,7 @@ TEST_F(NotifyFixture, EventsThatChangeNotifications)
 
   // set up a notifier and give it the battery so changing the battery's
   // charge should show up on the bus.
-  std::string last_uri;
-  auto sound_player = indicator_power_sound_player_mock_new ();
-  g_signal_connect(sound_player, "uri-played", G_CALLBACK(on_uri_played), &last_uri);
-  auto notifier = indicator_power_notifier_new (sound_player);
+  auto notifier = indicator_power_notifier_new ();
   indicator_power_notifier_set_battery (notifier, battery);
   indicator_power_notifier_set_bus (notifier, bus);
   ChangedParams changed_params;
@@ -388,7 +419,6 @@ TEST_F(NotifyFixture, EventsThatChangeNotifications)
   // test setup case
   wait_msec();
   EXPECT_STREQ (POWER_LEVEL_STR_OK, changed_params.power_level.c_str());
-  EXPECT_TRUE(last_uri.empty());
 
   // change the percent past the 'low' threshold and confirm that
   // a) the power level changes
@@ -399,48 +429,52 @@ TEST_F(NotifyFixture, EventsThatChangeNotifications)
   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 (low_power_uri, last_uri);
+  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...
-  last_uri.clear();
   changed_params = ChangedParams();
   set_battery_percentage (battery, percent_very_low);
   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 (low_power_uri, last_uri);
+  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...
-  last_uri.clear();
+  //last_uri.clear();
   changed_params = ChangedParams();
   g_object_set (battery, INDICATOR_POWER_DEVICE_STATE, UP_DEVICE_STATE_CHARGING, nullptr);
   wait_msec();
   EXPECT_EQ (FIELD_IS_WARNING, changed_params.fields);
   EXPECT_FALSE (changed_params.is_warning);
-  EXPECT_TRUE(last_uri.empty());
+  EXPECT_EQ (0, get_notify_call_count());
 
   // ...and that it comes back if we unplug again...
-  last_uri.clear();
+  //last_uri.clear();
   changed_params = ChangedParams();
   g_object_set (battery, INDICATOR_POWER_DEVICE_STATE, UP_DEVICE_STATE_DISCHARGING, nullptr);
   wait_msec();
   EXPECT_EQ (FIELD_IS_WARNING, changed_params.fields);
   EXPECT_TRUE (changed_params.is_warning);
-  EXPECT_EQ (low_power_uri, last_uri);
+  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
-  last_uri.clear();
+  //last_uri.clear();
   changed_params = ChangedParams();
   set_battery_percentage (battery, percent_low+1);
   wait_msec();
   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_TRUE(last_uri.empty());
+  EXPECT_EQ (0, get_notify_call_count());
 
   // cleanup
   g_dbus_connection_signal_unsubscribe (bus, sub_tag);
   g_object_unref (notifier);
   g_object_unref (battery);
-  g_object_unref (sound_player);
 }