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

[Matěj Laitl]

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


Good work, I see you tests have oncovered a couple of our errors, which is great.

Can you please split the actual fixes & cleanups (to be merged first) from the test? I'll try to look on the mpris conver failures, you may have uncovered another real ploblem as it was already reported on our bugzilla that MPRIS covers are sometimes wrong. (but perhaps it was fixed since then?)


src/core/meta/support/MetaUtility.h
<http://git.reviewboard.kde.org/r/105629/#comment14826>

    This IMO results in multiple copies of the strings in the final binary. (each time it is included)
    
    The strings should be declared in .h, but defined (value assigned) in .cpp, you'll much likely need to remove the static modifier.



tests/core/meta/support/TestMetaUtility.cpp
<http://git.reviewboard.kde.org/r/105629/#comment14824>

    nitpick: trailing whitespace


- Matěj Laitl


On July 20, 2012, 3:54 p.m., Jasneet Bhatti wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/105629/
> -----------------------------------------------------------
> 
> (Updated July 20, 2012, 3:54 p.m.)
> 
> 
> Review request for Amarok, Matěj Laitl and Sven Krohlas.
> 
> 
> Description
> -------
> 
> Test for core/meta/support/MetaUtility
> 
> THIS IS NOT YET COMPLETE.
> 
> The testMprisMapFromTrack() and testMpris20MapFromTrack() test functions are not setting the cover image properly and I'm unable to understand why. I've added a couple of qDebug() statements to show the same. I'd be grateful if someone could tell me what the problem actually is.
> 
> Everything else is working fine.
> 
> 
> Diffs
> -----
> 
>   src/core-impl/collections/db/sql/SqlRegistry.h 8d80117 
>   src/core/meta/support/MetaUtility.h c1ba442 
>   src/core/meta/support/MetaUtility.cpp 133076b 
>   tests/core/meta/CMakeLists.txt 3ae78c9 
>   tests/core/meta/support/CMakeLists.txt PRE-CREATION 
>   tests/core/meta/support/TestMetaUtility.h PRE-CREATION 
>   tests/core/meta/support/TestMetaUtility.cpp PRE-CREATION 
>   tests/data/cover/amarok.png PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/105629/diff/
> 
> 
> Testing
> -------
> 
> Builds and runs fine. Just the image setting problem exists.
> 
> 
> Thanks,
> 
> Jasneet Bhatti
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/amarok-devel/attachments/20120909/8214ae1e/attachment.html>