Review Request: SoK - Unit Test : core/meta/support/MetaConstants

Jasneet Bhatti jazneetbhatti at gmail.com
Tue Jul 3 08:25:26 UTC 2012


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

Review request for Amarok, Ralf Engels, Matěj Laitl, and Sven Krohlas.


Description
-------

Added a unit test for core/meta/support/MetaConstants
Corrected a couple of minor code style issues I came across in another test.

This test has some lines longer than 90 characters because they look more elegant that way in the context, although I could break them should that be an issue.

testValueForField() uses some existing Mock classes with the addition of MockLabel. The rest of the test methods use data driven testing.

I also have a doubt regarding the intention behind a statement in 'case 0' of valueForField() in MetaConstants.cpp :
        allInfos += track->playableUrl().path()
            += track->name()
            += track->comment();
This statement concatenates the three strings and adds them as one entity in the set allInfos, although it appears to me that the intention was to add them separately.
Ralf, since you authored it, I think you'd be the best person to clarify my doubt :)


Diffs
-----

  tests/TestTrackOrganizer.h 47fc22b 
  tests/TestTrackOrganizer.cpp 72e7b9b 
  tests/core/meta/CMakeLists.txt 3ae78c9 
  tests/core/meta/support/CMakeLists.txt PRE-CREATION 
  tests/core/meta/support/TestMetaConstants.h PRE-CREATION 
  tests/core/meta/support/TestMetaConstants.cpp PRE-CREATION 
  tests/mocks/MetaMock.h b478c87 

Diff: http://git.reviewboard.kde.org/r/105424/diff/


Testing
-------

Builds and runs fine.


Thanks,

Jasneet Bhatti

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/amarok-devel/attachments/20120703/6f3c96c6/attachment.html>


More information about the Amarok-devel mailing list