Review Request: SoK - Unit Test : core/collections/support/TrackForUrlWorker

Jasneet Bhatti jazneetbhatti at gmail.com
Thu Jul 5 00:15:50 UTC 2012



> On July 3, 2012, 10:17 a.m., Matěj Laitl wrote:
> > tests/core/collections/support/TestTrackForUrlWorker.cpp, line 44
> > <http://git.reviewboard.kde.org/r/105389/diff/1/?file=70647#file70647line44>
> >
> >     Hmm, I don't think this is the correct test. Instead, you should implement run() in MockTrackForUrlWorker and set m_track inside to to different values (perhaps using data-driven testing, MockTrackForUrlWorker can be documented what variable name and type it fetches) and then ensue that finishedLookup signal emits the correct TrackPtr. QTest::kWaitForSignal may be handy for it.

Implemented data driven testing, but with mock tracks as test data.
Didn't find any reason to include or test using track related info( name, artist, etc. ), since the track pointer comparison is enough to verify that the correct track is emitted.


> On July 3, 2012, 10:17 a.m., Matěj Laitl wrote:
> > tests/mocks/MockTrackForUrlWorker.h, line 53
> > <http://git.reviewboard.kde.org/r/105389/diff/1/?file=70648#file70648line53>
> >
> >     I don't think this helper method should exist.

completeJob() is private. So we do require a public method to call it directly or indirectly. I went for this as it also tests if the connection was properly made in the constructor ( although Qt should guarantee that, still ! ).


- Jasneet


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


On July 5, 2012, 12:07 a.m., Jasneet Bhatti wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/105389/
> -----------------------------------------------------------
> 
> (Updated July 5, 2012, 12:07 a.m.)
> 
> 
> Review request for Amarok, Matěj Laitl and Sven Krohlas.
> 
> 
> Description
> -------
> 
> Added unit test for core/collections/support/TrackForUrlWorker
> 
> Just the one slot completeJob() to test.
> Tested for both KUrl and QString types of urls.
> 
> 
> Diffs
> -----
> 
>   tests/core/collections/CMakeLists.txt b01b655 
>   tests/core/collections/support/CMakeLists.txt PRE-CREATION 
>   tests/core/collections/support/TestTrackForUrlWorker.h PRE-CREATION 
>   tests/core/collections/support/TestTrackForUrlWorker.cpp PRE-CREATION 
>   tests/mocks/MockTrackForUrlWorker.h PRE-CREATION 
>   tests/mocks/MockTrackForUrlWorker.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/105389/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/20120705/4bb7217b/attachment.html>


More information about the Amarok-devel mailing list