Fixed issue with warning notification. Fixed race conditions in tests

3 files changed, 103 insertions, 75 deletions

diff --git a/src/service.vala b/src/service.vala
index 65d5943..9749089 100644
--- a/src/service.vala
+++ b/src/service.vala
@@ -277,6 +277,7 @@ public class IndicatorSound.Service: Object {
 	private bool notify_server_supports_actions = false;
 	private bool notify_server_supports_sync = false;
 	private bool block_info_notifications = false;
+	private bool waiting_user_approve_warn = false;
 
 	private string get_volume_icon (double volume, VolumeControl.ActiveOutput active_output)
 	{
@@ -594,67 +595,72 @@ public class IndicatorSound.Service: Object {
 					_pre_warn_volume = null;
 					volume_control.volume = tmp;
 				}
+				waiting_user_approve_warn = false;
 			});
 			warn_notification.add_action ("cancel", _("Cancel"), (n, a) => {
 				_pre_warn_volume = null;
+				waiting_user_approve_warn = false;
 			});
+			waiting_user_approve_warn = true;
 			show_notification(warn_notification);
 		} else {
-			close_notification(warn_notification);
-
-			if (notify_server_supports_sync && !block_info_notifications) {
-
-				/* Determine Label */
-			        string volume_label = loud
-					 ? _("High volume can damage your hearing.")
-					 : "";
+			if (!waiting_user_approve_warn) {
+				close_notification(warn_notification);
+	
+				if (notify_server_supports_sync && !block_info_notifications) {
+	
+					/* Determine Label */
+				        string volume_label = loud
+						 ? _("High volume can damage your hearing.")
+						 : "";
 				
-				if (volume_label == "") {
-					if (volume_control.active_output == VolumeControl.ActiveOutput.SPEAKERS) {
-						volume_label = _("Speakers");
-					}
-
-					if (volume_control.active_output == VolumeControl.ActiveOutput.HEADPHONES) {
-						volume_label = _("Headphones");
-					}
-
-					if (volume_control.active_output == VolumeControl.ActiveOutput.BLUETOOTH_HEADPHONES) {
-						volume_label = _("Bluetooth headphones");
-					}
-					
-					if (volume_control.active_output == VolumeControl.ActiveOutput.BLUETOOTH_SPEAKER) {
-						volume_label = _("Bluetooth speaker");
-					}
-
-					if (volume_control.active_output == VolumeControl.ActiveOutput.USB_SPEAKER) {
-						volume_label = _("Usb speaker");
-					}
-
-					if (volume_control.active_output == VolumeControl.ActiveOutput.USB_HEADPHONES) {
-						volume_label = _("Usb headphones");
-					}
+					if (volume_label == "") {
+						if (volume_control.active_output == VolumeControl.ActiveOutput.SPEAKERS) {
+							volume_label = _("Speakers");
+						}
+	
+						if (volume_control.active_output == VolumeControl.ActiveOutput.HEADPHONES) {
+							volume_label = _("Headphones");
+						}
+	
+						if (volume_control.active_output == VolumeControl.ActiveOutput.BLUETOOTH_HEADPHONES) {
+							volume_label = _("Bluetooth headphones");
+						}
+						
+						if (volume_control.active_output == VolumeControl.ActiveOutput.BLUETOOTH_SPEAKER) {
+							volume_label = _("Bluetooth speaker");
+						}
+	
+						if (volume_control.active_output == VolumeControl.ActiveOutput.USB_SPEAKER) {
+							volume_label = _("Usb speaker");
+						}
+	
+						if (volume_control.active_output == VolumeControl.ActiveOutput.USB_HEADPHONES) {
+							volume_label = _("Usb headphones");
+						}
+						
+						if (volume_control.active_output == VolumeControl.ActiveOutput.HDMI_SPEAKER) {
+							volume_label = _("HDMI speaker");
+						}
 					
-					if (volume_control.active_output == VolumeControl.ActiveOutput.HDMI_SPEAKER) {
-						volume_label = _("HDMI speaker");
-					}
-			
-					if (volume_control.active_output == VolumeControl.ActiveOutput.HDMI_HEADPHONES) {
-						volume_label = _("HDMI headphones");
+						if (volume_control.active_output == VolumeControl.ActiveOutput.HDMI_HEADPHONES) {
+							volume_label = _("HDMI headphones");
+						}
 					}
+	
+					/* Choose an icon */
+				 	string icon = get_volume_notification_icon (volume_control.volume.volume, loud, volume_control.active_output);
+
+					/* Reset the notification */
+					var n = this.info_notification;
+					n.update (_("Volume"), volume_label, icon);
+					n.clear_hints();
+					n.set_hint ("x-canonical-non-shaped-icon", "true");
+					n.set_hint ("x-canonical-private-synchronous", "true");
+					n.set_hint ("x-canonical-value-bar-tint", loud ? "true" : "false");
+					n.set_hint ("value", (int32)Math.round(get_volume_percent() * 100.0));
+					show_notification(n);
 				}
-
-				/* Choose an icon */
-			 	string icon = get_volume_notification_icon (volume_control.volume.volume, loud, volume_control.active_output);
-
-				/* Reset the notification */
-				var n = this.info_notification;
-				n.update (_("Volume"), volume_label, icon);
-				n.clear_hints();
-				n.set_hint ("x-canonical-non-shaped-icon", "true");
-				n.set_hint ("x-canonical-private-synchronous", "true");
-				n.set_hint ("x-canonical-value-bar-tint", loud ? "true" : "false");
-				n.set_hint ("value", (int32)Math.round(get_volume_percent() * 100.0));
-				show_notification(n);
 			}
 		}
 	}
diff --git a/tests/integration/indicator-sound-test-base.cpp b/tests/integration/indicator-sound-test-base.cpp
index 3c6e52a..2bd71b6 100644
--- a/tests/integration/indicator-sound-test-base.cpp
+++ b/tests/integration/indicator-sound-test-base.cpp
@@ -669,7 +669,7 @@ QString IndicatorSoundTestBase::getDevicePortString(DevicePortType port)
 
 void IndicatorSoundTestBase::checkPortDevicesLabels(DevicePortType speakerPort, DevicePortType headphonesPort)
 {
-    double INITIAL_VOLUME = 1.0;
+    double INITIAL_VOLUME = 0.0;
 
     QString speakerString;
     QString speakerStringMenu;
@@ -739,7 +739,7 @@ void IndicatorSoundTestBase::checkPortDevicesLabels(DevicePortType speakerPort,
         // GetServerInformation and GetCapabilities
         checkNotificationWithNoArgs("GetServerInformation", notificationsSpy.at(0));
         checkNotificationWithNoArgs("GetCapabilities", notificationsSpy.at(1));
-        checkVolumeNotification(1.0, speakerString, false, notificationsSpy.at(2));
+        checkVolumeNotification(INITIAL_VOLUME, speakerString, false, notificationsSpy.at(2));
         notificationsSpy.clear();
     }
 
@@ -754,13 +754,13 @@ void IndicatorSoundTestBase::checkPortDevicesLabels(DevicePortType speakerPort,
         // GetServerInformation and GetCapabilities
         checkNotificationWithNoArgs("GetServerInformation", notificationsSpy.at(0));
         checkNotificationWithNoArgs("GetCapabilities", notificationsSpy.at(1));
-        checkVolumeNotification(1.0, headphonesString, false, notificationsSpy.at(2));
+        checkVolumeNotification(INITIAL_VOLUME, headphonesString, false, notificationsSpy.at(2));
         notificationsSpy.clear();
     }
     else
     {
         WAIT_FOR_SIGNALS(notificationsSpy, 1);
-        checkVolumeNotification(1.0, headphonesString, false, notificationsSpy.at(0));
+        checkVolumeNotification(INITIAL_VOLUME, headphonesString, false, notificationsSpy.at(0));
         notificationsSpy.clear();
     }
 
@@ -777,7 +777,7 @@ void IndicatorSoundTestBase::checkPortDevicesLabels(DevicePortType speakerPort,
             .item(mh::MenuItemMatcher()
                 .section()
                 .item(silentModeSwitch(false))
-                .item(volumeSlider(1.0, headphonesStringMenu))
+                .item(volumeSlider(INITIAL_VOLUME, headphonesStringMenu))
             )
         ).match());
 
@@ -785,7 +785,7 @@ void IndicatorSoundTestBase::checkPortDevicesLabels(DevicePortType speakerPort,
     EXPECT_TRUE(activateHeadphones(false));
 
     WAIT_FOR_SIGNALS(notificationsSpy, 1);
-    checkVolumeNotification(1.0, speakerString, false, notificationsSpy.at(0));
+    checkVolumeNotification(INITIAL_VOLUME, speakerString, false, notificationsSpy.at(0));
     notificationsSpy.clear();
 
     // check the label in the menu
@@ -801,7 +801,7 @@ void IndicatorSoundTestBase::checkPortDevicesLabels(DevicePortType speakerPort,
             .item(mh::MenuItemMatcher()
                 .section()
                 .item(silentModeSwitch(false))
-                .item(volumeSlider(1.0, speakerStringMenu))
+                .item(volumeSlider(INITIAL_VOLUME, speakerStringMenu))
             )
         ).match());
 }
diff --git a/tests/integration/test-indicator.cpp b/tests/integration/test-indicator.cpp
index fccaf84..b301586 100644
--- a/tests/integration/test-indicator.cpp
+++ b/tests/integration/test-indicator.cpp
@@ -52,19 +52,27 @@ TEST_F(TestIndicator, PhoneChangeRoleVolume)
     int randInt = qrand() % 100;
     double randomVolume = randInt / 100.0;
 
+    QSignalSpy &userAccountsSpy = *signal_spy_volume_changed_;
     // set an initial volume to the alert role
+    userAccountsSpy.clear();
     setStreamRestoreVolume("alert", 1.0);
-    EXPECT_TRUE(waitVolumeChangedInIndicator());
+    WAIT_FOR_SIGNALS(userAccountsSpy, 2);
 
+    userAccountsSpy.clear();
     // play a test sound, it should change the role in the indicator
     EXPECT_TRUE(startTestSound("multimedia"));
-    EXPECT_TRUE(waitVolumeChangedInIndicator());
 
+    // this time we only expect 1 signal as it's only the indicator
+    // updating the value
+    WAIT_FOR_SIGNALS(userAccountsSpy, 1);
+    //EXPECT_TRUE(waitVolumeChangedInIndicator());
+
+    userAccountsSpy.clear();
     // set the random volume to the multimedia role
     EXPECT_TRUE(setStreamRestoreVolume("multimedia", randomVolume));
     if (randomVolume != INITIAL_VOLUME)
     {
-        EXPECT_TRUE(waitVolumeChangedInIndicator());
+        WAIT_FOR_SIGNALS(userAccountsSpy, 1);
     }
 
     // check the indicator
@@ -98,13 +106,14 @@ TEST_F(TestIndicator, PhoneChangeRoleVolume)
             )
         ).match());
 
+    userAccountsSpy.clear();
     // stop the test sound, the role should change again to alert
     stopTestSound();
     if (randomVolume != 1.0)
     {
         // we only wait if the volume in the alert and the
         // one set in the multimedia roles differ
-        EXPECT_TRUE(waitVolumeChangedInIndicator());
+        WAIT_FOR_SIGNALS(userAccountsSpy, 1);
     }
 
     // check the initial volume for the alert role
@@ -727,21 +736,26 @@ TEST_F(TestIndicator, PhoneNotificationWarningVolume)
     EXPECT_TRUE(startTestSound("multimedia"));
     EXPECT_TRUE(waitVolumeChangedInIndicator());
 
-    // change volume to 0.0... no warning should be emitted
-    setActionValue("volume", QVariant::fromValue(0.0));
-
-    WAIT_FOR_SIGNALS(notificationsSpy, 4);
-
+    WAIT_FOR_SIGNALS(notificationsSpy, 3);
     // the first time we also have the calls to
     // GetServerInformation and GetCapabilities
     checkNotificationWithNoArgs("GetServerInformation", notificationsSpy.at(0));
     checkNotificationWithNoArgs("GetCapabilities", notificationsSpy.at(1));
     checkVolumeNotification(0.0, "Headphones", false, notificationsSpy.at(2));
-    checkVolumeNotification(0.0, "Headphones", false, notificationsSpy.at(3));
+    notificationsSpy.clear();
+
+    // change volume to 0.3... no warning should be emitted
+    setActionValue("volume", QVariant::fromValue(0.3));
+    EXPECT_TRUE(waitVolumeChangedInIndicator());
+
+    WAIT_FOR_SIGNALS(notificationsSpy, 1);
+
+    checkVolumeNotification(0.3, "Headphones", false, notificationsSpy.at(0));
     notificationsSpy.clear();
 
     // change volume to 0.5... no warning should be emitted
     setActionValue("volume", QVariant::fromValue(0.5));
+    EXPECT_TRUE(waitVolumeChangedInIndicator());
 
     WAIT_FOR_SIGNALS(notificationsSpy, 1);
 
@@ -750,6 +764,7 @@ TEST_F(TestIndicator, PhoneNotificationWarningVolume)
 
     // change volume to 1.0... warning should be emitted
     setActionValue("volume", QVariant::fromValue(1.0));
+    EXPECT_TRUE(waitVolumeChangedInIndicator());
 
     WAIT_FOR_SIGNALS(notificationsSpy, 4);
 
@@ -787,18 +802,21 @@ TEST_F(TestIndicator, PhoneNotificationWarningVolume)
 
     // try again...
     notificationsSpy.clear();
+
+    qWarning() << "-----------------------------------------------------------";
     // change volume to 1.0... warning should be emitted
     setActionValue("volume", QVariant::fromValue(1.0));
+    EXPECT_TRUE(waitVolumeChangedInIndicator());
 
-    WAIT_FOR_SIGNALS(notificationsSpy, 4);
+    WAIT_FOR_SIGNALS(notificationsSpy, 2);
 
-    checkCloseNotification(1, notificationsSpy.at(0));
+//    checkCloseNotification(1, notificationsSpy.at(0));
+    checkHighVolumeNotification(notificationsSpy.at(0));
+//    checkCloseNotification(1, notificationsSpy.at(2));
     checkHighVolumeNotification(notificationsSpy.at(1));
-    checkCloseNotification(1, notificationsSpy.at(2));
-    checkHighVolumeNotification(notificationsSpy.at(3));
 
     // get the last notification ID
-    idNotification = getNotificationID(notificationsSpy.at(3));
+    idNotification = getNotificationID(notificationsSpy.at(1));
     ASSERT_NE(-1, idNotification);
 
     qWarning() << "XGM: id Notification: " << idNotification;
@@ -833,12 +851,15 @@ TEST_F(TestIndicator, PhoneNotificationWarningVolume)
 
     // change volume to 0.5... no warning should be emitted
     setActionValue("volume", QVariant::fromValue(0.5));
+    EXPECT_TRUE(waitVolumeChangedInIndicator());
 
-    WAIT_FOR_SIGNALS(notificationsSpy, 2);
+    WAIT_FOR_SIGNALS(notificationsSpy, 4);
 
     // check the notification TODO check why the sound indicator sends it twice
-    checkVolumeNotification(0.5, "Headphones", false, notificationsSpy.at(0));
+    checkCloseNotification(idNotification, notificationsSpy.at(0));
     checkVolumeNotification(0.5, "Headphones", false, notificationsSpy.at(1));
+    checkCloseNotification(idNotification, notificationsSpy.at(2));
+    checkVolumeNotification(0.5, "Headphones", false, notificationsSpy.at(3));
 
     // check that the volume was applied
     // and that we don't have the warning item
@@ -863,6 +884,7 @@ TEST_F(TestIndicator, PhoneNotificationWarningVolume)
     notificationsSpy.clear();
 
     setActionValue("volume", QVariant::fromValue(1.0));
+    EXPECT_TRUE(waitVolumeChangedInIndicator());
 
     WAIT_FOR_SIGNALS(notificationsSpy, 2);