> On June 21, 2012, 2:08 p.m., Matěj Laitl wrote: > > > > Matěj Laitl wrote: > Collection has really just 3 implemented methods. CollectionBase > shouldn't exist, because it really is Meta::MetaCapability. This should be > changed by some Amarok dev as soon as 2.6 is released. Then the change is > done, CollectionBase methods will be already tested in MetaCapability, so you > don't have to test them here. So I suggest you remove the tests for is<>() > and create<>(), too.
Ok. Will remove the unnecessary ones. > On June 21, 2012, 2:08 p.m., Matěj Laitl wrote: > > tests/core/collections/TestCollection.h, line 27 > > <http://git.reviewboard.kde.org/r/105166/diff/2/?file=70024#file70024line27> > > > > Strange indentation. Don't you mix tabs and spaces, jasneet? Actually, I had some code from the previous version of this test and some new, so consistency problems with the indentation. Will correct these. > On June 21, 2012, 2:08 p.m., Matěj Laitl wrote: > > tests/core/collections/TestCollection.cpp, line 34 > > <http://git.reviewboard.kde.org/r/105166/diff/2/?file=70025#file70025line34> > > > > Strange indentation again. Hmm? Don't understand this one. There are two levels of indentation and the variable definition is according to intro_and_style. Please explain what is the fault. > On June 21, 2012, 2:08 p.m., Matěj Laitl wrote: > > tests/core/collections/TestCollection.cpp, line 203 > > <http://git.reviewboard.kde.org/r/105166/diff/2/?file=70025#file70025line203> > > > > While entire code shouldn't be here, I wonder why you just dont > > QCOMPARE( collection->usedCapacity(), 0.0 )? You really want to pretend > > static_cast doesn't exist and use it only in at a least resort. 0.0 is a double and usedCapacity returns a float, so there was a warning on compilation. Used the cast to eliminated the warning. http://qt-project.org/doc/qt-4.8/qtest.html#QCOMPARE "In the case of comparing floats and doubles, qFuzzyCompare() is used for comparing. This means that comparing to 0 will likely fail. One solution to this is to compare to 1, and add 1 to the produced output." I'm not sure if the above applies for 0.0 too, so added 1 to not take a chance. > On June 21, 2012, 2:08 p.m., Matěj Laitl wrote: > > tests/core/collections/TestCollection.cpp, line 228 > > <http://git.reviewboard.kde.org/r/105166/diff/2/?file=70025#file70025line228> > > > > No no no, you should actually test that isWritable() returns what > > location()->isWritable() returns. To do this you can subclass your > > CollectionMock to return CollectionLocationMock where you'll cause > > different isWritable() values to be returned and verified. > > > > You can use http://qt-project.org/doc/qt-4.8/qtestlib-tutorial2.html + > > implement TestCollection::testIsWritable_data(). Then you can use > > QFETCH(bool, isWritable); in CollectionLocationMock. Working on it now. > On June 21, 2012, 2:08 p.m., Matěj Laitl wrote: > > tests/core/collections/TestCollection.cpp, line 237 > > <http://git.reviewboard.kde.org/r/105166/diff/2/?file=70025#file70025line237> > > > > Same as comment for testIsWritable() Working on it now. - Jasneet ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/105166/#review14948 ----------------------------------------------------------- On June 21, 2012, 1:38 p.m., Jasneet Bhatti wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/105166/ > ----------------------------------------------------------- > > (Updated June 21, 2012, 1:38 p.m.) > > > Review request for Amarok and Sven Krohlas. > > > Description > ------- > > This patch implements a unit test for core/collections/Collection > > There are abstract classes to be tested as well, which can only be done when > subclasses define the pure virtual functions. So tests for those will be done > along with the subclasses. > > > Diffs > ----- > > tests/core/collections/CMakeLists.txt 2efd1fe > tests/core/collections/TestCollection.h PRE-CREATION > tests/core/collections/TestCollection.cpp PRE-CREATION > > Diff: http://git.reviewboard.kde.org/r/105166/diff/ > > > Testing > ------- > > Test passes on my repository > > > Thanks, > > Jasneet Bhatti > >
_______________________________________________ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
