Review Request: Unit test : ActionsCapability

[Jasneet Bhatti]

> On June 4, 2012, 7:01 a.m., Bart Cerneels wrote:
> > I have a few nitpicks that are more about our API, ActionsCapability in particular.
> > 
> > As a part of the unit test patches you should improve the documentation as well. Will help to understand (and get us to properly define) the use cases of the interface.

Yes, I agree. Will do it for sure.


> On June 4, 2012, 7:01 a.m., Bart Cerneels wrote:
> > tests/core/capabilities/TestActionsCapability.cpp, line 46
> > <http://git.reviewboard.kde.org/r/105144/diff/2/?file=66220#file66220line46>
> >
> >     Does ActionsCapability actually claim that it has to return the same objects in the same order as creation?
> >     Perhaps you should test the order and QProprties of the returned actions instead.

The actions list( m_actions ) is a protected member of ActionsCapability. So subclasses cannot modify it and there is no function in ActionsCapability either that would seem to have a need to modify it. So I guess we can safely assume that the actions list should remain the same both in order and values, in which case I believe a simple '==' comparison would suffice.


> On June 4, 2012, 7:01 a.m., Bart Cerneels wrote:
> > tests/core/capabilities/TestActionsCapability.cpp, line 64
> > <http://git.reviewboard.kde.org/r/105144/diff/2/?file=66220#file66220line64>
> >
> >     To verify this you could have created the capability with an empty list. Does not influence the test in any way.

Yes, that would make the intention of that test much clearer. Done.


- Jasneet


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/105144/#review14402
-----------------------------------------------------------


On June 3, 2012, 3:45 p.m., Jasneet Bhatti wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/105144/
> -----------------------------------------------------------
> 
> (Updated June 3, 2012, 3:45 p.m.)
> 
> 
> Review request for Amarok.
> 
> 
> Description
> -------
> 
> This is a patch implementing unit testing of core/capabilities/ActionsCapability
> 
> 
> Diffs
> -----
> 
>   tests/core/capabilities/TestActionsCapability.cpp PRE-CREATION 
>   tests/core/CMakeLists.txt c66d3be 
>   tests/core/capabilities/CMakeLists.txt PRE-CREATION 
>   tests/core/capabilities/TestActionsCapability.h PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/105144/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jasneet Bhatti
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/amarok-devel/attachments/20120604/94319eec/attachment-0001.html>