----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/105166/#review14948 -----------------------------------------------------------
tests/core/collections/TestCollection.h <http://git.reviewboard.kde.org/r/105166/#comment11730> Strange indentation. Don't you mix tabs and spaces, jasneet? tests/core/collections/TestCollection.h <http://git.reviewboard.kde.org/r/105166/#comment11732> Hmm, I wonder why CollectionBase exists at all, it is just MetaCapability reimplemented. This is not a problem with the test, but with actual implementation. tests/core/collections/TestCollection.h <http://git.reviewboard.kde.org/r/105166/#comment11734> abstract (implemented for convenience) method, should not be tested. tests/core/collections/TestCollection.h <http://git.reviewboard.kde.org/r/105166/#comment11735> abstract (implemented for convenience) method, should not be tested. tests/core/collections/TestCollection.h <http://git.reviewboard.kde.org/r/105166/#comment11736> abstract (implemented for convenience) method, should not be tested. tests/core/collections/TestCollection.h <http://git.reviewboard.kde.org/r/105166/#comment11737> abstract (implemented for convenience) method, should not be tested. tests/core/collections/TestCollection.h <http://git.reviewboard.kde.org/r/105166/#comment11738> abstract (implemented for convenience) method, should not be tested. tests/core/collections/TestCollection.cpp <http://git.reviewboard.kde.org/r/105166/#comment11733> Strange indentation again. Hmm? tests/core/collections/TestCollection.cpp <http://git.reviewboard.kde.org/r/105166/#comment11739> 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. tests/core/collections/TestCollection.cpp <http://git.reviewboard.kde.org/r/105166/#comment11742> While not strictly needed in tests, it is good habit to write code without memory leaks. Here you would "delete collection;", because it doesn't delete itself unless prepareSomething() is called on it. tests/core/collections/TestCollection.cpp <http://git.reviewboard.kde.org/r/105166/#comment11740> 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. tests/core/collections/TestCollection.cpp <http://git.reviewboard.kde.org/r/105166/#comment11741> Same as comment for testIsWritable() - Matěj Laitl 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
