Review Request 127398: Add unit tests for KNotification and fix a whole bunch of issues uncovered thanks to them

[Martin Klapetek]

> On March 18, 2016, 6:20 a.m., Anthony Fieroni wrote:
> > src/knotification.cpp, line 63
> > <https://git.reviewboard.kde.org/r/127398/diff/1/?file=453383#file453383line63>
> >
> >     ref can't be UINT_MAX, but id can, no? Negative rage will brake all, it's this possible, maybe not. Counters it's not good idea to be signed.

Yes...if your application will emit more than 32767 notifications in its lifetime. At which point the application is probalby broken.


> On March 18, 2016, 6:20 a.m., Anthony Fieroni wrote:
> > src/knotificationmanager.cpp, line 168
> > <https://git.reviewboard.kde.org/r/127398/diff/1/?file=453384#file453384line168>
> >
> >     force = true, id not present how can take value? force is compromised

The force is actually unused and should go at some later point.


- Martin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127398/#review93676
-----------------------------------------------------------


On March 16, 2016, 9:23 p.m., Martin Klapetek wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127398/
> -----------------------------------------------------------
> 
> (Updated March 16, 2016, 9:23 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: knotifications
> 
> 
> Description
> -------
> 
> Adds basic set of unit tests including fake notifications server.
> 
> This helped uncover and fix these issues:
> 
> * Calling close() on KNotification that was not "sent" would not emit closed() and would not delete it
> * Closing a notification can delete the KNotification object prematurely
> * Invoking an action leads to unnecessary dbus roundtrip
> * Invoking an action would fail to properly close and delete the KNotification object
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt 6d09051 
>   autotests/CMakeLists.txt PRE-CREATION 
>   autotests/fake_notifications_server.h PRE-CREATION 
>   autotests/fake_notifications_server.cpp PRE-CREATION 
>   autotests/knotification_test.cpp PRE-CREATION 
>   autotests/knotifications5/qttest.notifyrc PRE-CREATION 
>   autotests/qtest_dbus.h PRE-CREATION 
>   src/knotification.cpp 17b0be2 
>   src/knotificationmanager.cpp e80d48c 
>   src/knotificationplugin.cpp acf964c 
>   src/notifybypopup.cpp b9489c1 
> 
> Diff: https://git.reviewboard.kde.org/r/127398/diff/
> 
> 
> Testing
> -------
> 
> Unit tests all pass.
> 
> 
> Thanks,
> 
> Martin Klapetek
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20160321/8bf7e1c0/attachment.html>