Merge lp:~macslow/unity-notifications/dont-ignore-placeholder into lp:unity-notifications

[Mirco Müller]

Updated NotificationModel and its unit-test to not ignore the permanent PlaceHolder notification in the queue.

Updated NotificationModel and its unit-test to not ignore the permanent PlaceHolder notification in the queue.

* Are there any related MPs required for this MP to build/function as expected?
This MP is related...
* https://code.launchpad.net/~macslow/unity8/fix-object-name-of-notification-ap-test/+merge/210588

* Did you perform an exploratory manual test run of your code change and any related functionality?
Yes.

* If you changed the packaging (debian), did you subscribe the ubuntu-unity team to this MP?
Not applicable.

* If you changed the UI, has there been a design review?
Not applicable.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:198
http://jenkins.qa.ubuntu.com/job/unity-notifications-ci/45/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-notifications-trusty-amd64-ci/20
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-notifications-trusty-armhf-ci/20
        deb: http://jenkins.qa.ubuntu.com/job/unity-notifications-trusty-armhf-ci/20/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-notifications-trusty-i386-ci/20

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-notifications-ci/45/rebuild

review: Approve (continuous-integration)

[Albert Astals Cid (aacid)]

Are the examples related to the other changes? I can't see how

review: Needs Information

[Albert Astals Cid (aacid)]

Is there any bug that this change is fixing? Also is it possible to have a testcase that fails without this change but works with it? (Besides the tests you changed)

review: Needs Information

[Mirco Müller (macslow)]

The icon-changes in the examples are unrelated and just cosmetics.

There's only testVisualSDQueueWithoutCritical(). Why would it need an additional test?

[Albert Astals Cid (aacid)]

This seems to make unity8.shell.tests.test_notifications.InteractiveNotificationBase.test_sd_incoming_call fail. Is that correct?

review: Needs Information

[Mirco Müller (macslow)]

That should not happen :/ This is new. I need to look into this.

[Mirco Müller (macslow)]

The notification AP-test "test_sd_incoming_call" does not use the correct objectName "notification1", but "notification0". The PlaceHolder notification in the model has "notification0". So no AP-test should look for "notification0".

I'll push a fixing branch to unity8 to go alongside with this one.

[Mirco Müller (macslow)]

I've updated the MP-summary with a link to the fixing MP for unity8. See https://code.launchpad.net/~macslow/unity8/fix-object-name-of-notification-ap-test/+merge/210588

[Albert Astals Cid (aacid)]

Looks good.

review: Approve

=== modified file 'examples/sd-example-incoming-file.py'
--- examples/sd-example-incoming-file.py	2013-10-31 12:55:18 +0000
+++ examples/sd-example-incoming-file.py	2014-02-18 14:55:55 +0000
@@ -81,7 +81,7 @@
 	loop = gobject.MainLoop ()
 	n = pushNotification ("Incoming file",
                           "Frank would like to send you the file: essay.pdf",
-                          "document")
+                          "sync-idle")
 	n.connect ("closed", example.closedHandler, loop)
 
 	loop.run ()

=== modified file 'examples/sd-example-much-body-text.py'
--- examples/sd-example-much-body-text.py	2013-10-31 12:55:18 +0000
+++ examples/sd-example-much-body-text.py	2014-02-18 14:55:55 +0000
@@ -85,7 +85,7 @@
 	loop = gobject.MainLoop ()
 	n = pushNotification ("Uses much text",
                           "Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam voluptua. At vero eos et accusam et justo duo dolores et ea rebum. Stet clita kasd gubergren, no sea takimata sanctus est Lorem ipsum dolor sit amet.",
-						  "info")
+						  "messages")
 	n.connect ("closed", example.closedHandler, loop)
 
 	loop.run ()

=== modified file 'examples/sd-example-using-button-tint-hint.py'
--- examples/sd-example-using-button-tint-hint.py	2013-10-31 12:55:18 +0000
+++ examples/sd-example-using-button-tint-hint.py	2014-02-18 14:55:55 +0000
@@ -85,7 +85,7 @@
 	loop = gobject.MainLoop ()
 	n = pushNotification ("Question",
                           "Would you say Ok or Cancel?",
-						  "info")
+						  "search")
 	n.connect ("closed", example.closedHandler, loop)
 
 	loop.run ()

=== modified file 'src/NotificationModel.cpp'
--- src/NotificationModel.cpp	2014-01-16 15:13:04 +0000
+++ src/NotificationModel.cpp	2014-02-18 14:55:55 +0000
@@ -441,7 +441,7 @@
         }
 
         if (!inserted) {
-            insertToVisible(n, 0);
+            insertToVisible(n, 1);
         }
     }
 }

=== modified file 'test/notificationtest.cpp'
--- test/notificationtest.cpp	2014-01-27 16:00:55 +0000
+++ test/notificationtest.cpp	2014-02-18 14:55:55 +0000
@@ -156,7 +156,7 @@
     m.insertNotification(n3);
     m.insertNotification(n4);
 
-    QCOMPARE(m.getDisplayedNotification(0)->getBody(), QString("snap-decision-critical"));
+    QCOMPARE(m.getDisplayedNotification(1)->getBody(), QString("snap-decision-critical"));
 }
 
 void TestNotifications::testVisualSDQueueWithoutCritical() {
@@ -173,10 +173,10 @@
     m.insertNotification(n3);
     m.insertNotification(n4);
 
-    QCOMPARE(m.getDisplayedNotification(3)->getBody(), QString("snap-decision-1"));
-    QCOMPARE(m.getDisplayedNotification(2)->getBody(), QString("snap-decision-2"));
-    QCOMPARE(m.getDisplayedNotification(1)->getBody(), QString("snap-decision-3"));
-    QCOMPARE(m.getDisplayedNotification(0)->getBody(), QString("snap-decision-4"));
+    QCOMPARE(m.getDisplayedNotification(4)->getBody(), QString("snap-decision-1"));
+    QCOMPARE(m.getDisplayedNotification(3)->getBody(), QString("snap-decision-2"));
+    QCOMPARE(m.getDisplayedNotification(2)->getBody(), QString("snap-decision-3"));
+    QCOMPARE(m.getDisplayedNotification(1)->getBody(), QString("snap-decision-4"));
 }
 
 QTEST_MAIN(TestNotifications)