> 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 > >
_______________________________________________ Amarok-devel mailing list [email protected] https://mail.kde.org/mailman/listinfo/amarok-devel
