Review Request: GSoC: Unit test for DeleteSlidesCommand

[Thorsten Zachmann]

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

Ship it!


Hello Paul,

nice work. I think you can already commit the work and then work on the enhancements I have pointed out. The comment pointed out for the first function are also true for the other functions so all can be updated to follow the comments.

Thorsten



stage/part/tests/TestDeleteSlidesCommand.cpp
<http://git.reviewboard.kde.org/r/102349/#comment5130>

    To make the test more powerful it would be good to have differnet orders in the different custom slide shows. With that change different things are tested. It might be good to also test a custom slide show that has a slide in it multiple times maybe best not all in one place so like e.g.
    
    1 2 1 3 1 4 5 1
    
    That will make the test cover more of the code which is there.
    
    With that you can merge this test with the delSlideWithCopies which is just duplicated code and can be done in one test



stage/part/tests/TestDeleteSlidesCommand.cpp
<http://git.reviewboard.kde.org/r/102349/#comment5131>

    I think it would be better to test these stuff in a test file which tests the class KPrCustomSlideShows as that is not related to the delete slides command. Maybe you can move it to the file TestCustomSlideShows.h with add some more stuff therer.



stage/part/tests/TestDeleteSlidesCommand.cpp
<http://git.reviewboard.kde.org/r/102349/#comment5133>

    It would be better to check the resulting list to see it is correcty. That way it is made sure that nothing unexpected happens when removing the slide slide.



stage/part/tests/TestDeleteSlidesCommand.cpp
<http://git.reviewboard.kde.org/r/102349/#comment5132>

    You should compare the actual position of the slide in the slide show to make sure that the undo worked correctly. Maybe it is even easier to comapre the lists which you inserted to see if they are still the same.


- Thorsten


On Aug. 16, 2011, 11:06 p.m., Paul Mendez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/102349/
> -----------------------------------------------------------
> 
> (Updated Aug. 16, 2011, 11:06 p.m.)
> 
> 
> Review request for Calligra.
> 
> 
> Summary
> -------
> 
> This Test Unit tests three cases
> - Delete a single slide
> - Delete a slide with multiple copies on a custom slide show
> - Delete multiple slides
> 
> Each case test deletion (Undo/Redo) from document pages list and custom slides shows list
> 
> 
> Diffs
> -----
> 
>   stage/part/commands/KPrDeleteSlidesCommand.h 44cceaaa07efd0bd5f5efe6606a1a05f42956731 
>   stage/part/tests/CMakeLists.txt 7a2e1b534093b6331b0a0270793a4b45ef84022c 
>   stage/part/tests/TestDeleteSlidesCommand.h PRE-CREATION 
>   stage/part/tests/TestDeleteSlidesCommand.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/102349/diff
> 
> 
> Testing
> -------
> 
> Build an run
> 
> 
> Thanks,
> 
> Paul
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/calligra-devel/attachments/20110819/60ecde53/attachment.htm>