Review Request: Modify TestDeleteSlidesCommand and TestCustomSlideShows to include more tests

Thorsten Zachmann t.zachmann at zagge.de
Sun Aug 21 05:25:26 BST 2011


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

Ship it!


Nice work. Please commit after addressing the comments


stage/part/tests/TestCustomSlideShows.cpp
<http://git.reviewboard.kde.org/r/102393/#comment5186>

    This code is used in various test. It would be good to put it into a function to use it everywhere where it is needed. This will avoid code duplication.



stage/part/tests/TestCustomSlideShows.cpp
<http://git.reviewboard.kde.org/r/102393/#comment5187>

    It would be good to get a list of all remaining slide shows and check that it contains only the slide shows that should be in. That makes sure there are no side effects.



stage/part/tests/TestCustomSlideShows.cpp
<http://git.reviewboard.kde.org/r/102393/#comment5188>

    Best also insert customeShowName2 and test it does not get modified by update.



stage/part/tests/TestCustomSlideShows.cpp
<http://git.reviewboard.kde.org/r/102393/#comment5189>

    This can be made more strict by creating a QList<QString> where you put the slide show names in and compare it to the result of doc.customSlideShows()->names()



stage/part/tests/TestCustomSlideShows.cpp
<http://git.reviewboard.kde.org/r/102393/#comment5190>

    It would be better to check that all slides are as they should be.



stage/part/tests/TestCustomSlideShows.cpp
<http://git.reviewboard.kde.org/r/102393/#comment5191>

    As above it would be better to check that all slides are correct. 



stage/part/tests/TestCustomSlideShows.cpp
<http://git.reviewboard.kde.org/r/102393/#comment5192>

    Please verify the content of all slides after the removal



stage/part/tests/TestCustomSlideShows.cpp
<http://git.reviewboard.kde.org/r/102393/#comment5193>

    Please check the content of all slides after removal


- Thorsten


On Aug. 20, 2011, 7:50 p.m., Paul Mendez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/102393/
> -----------------------------------------------------------
> 
> (Updated Aug. 20, 2011, 7:50 p.m.)
> 
> 
> Review request for Calligra.
> 
> 
> Summary
> -------
> 
> Modify TestDeleteSlidesCommand following recommendations on previous review request
> Move some tests to TestCustomSlideShows file and implement some missing tests.
> 
> 
> Diffs
> -----
> 
>   stage/part/tests/TestCustomSlideShows.h 4586c340da08cfde756b52b5a05fe8a78efbe7ef 
>   stage/part/tests/TestCustomSlideShows.cpp 5376e2bf5574813781e85a53627319b94d63a418 
>   stage/part/tests/TestDeleteSlidesCommand.h 63caace87b2ce386ca464c8158f383af84e55565 
>   stage/part/tests/TestDeleteSlidesCommand.cpp 4043215ab9e3a2696b46be54e06c0ab306ef8e66 
> 
> Diff: http://git.reviewboard.kde.org/r/102393/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Paul
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/calligra-devel/attachments/20110821/8888e2b6/attachment.htm>


More information about the calligra-devel mailing list