[Kde-pim] Review Request 111755: Fix, update and re-enable IMAP resource unit-tests

Kevin Ottens ervin at kde.org
Mon Jul 29 07:32:55 BST 2013


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



resources/imap/resourcestateinterface.h
<http://git.reviewboard.kde.org/r/111755/#comment27112>

    I would say the abstraction level for that method is too low as indicate the name. It probably should be less generic and closer to its use which is uniquely about the missing body check.
    
    So I'd say it needs the following adjustments:
     * Renaming;
     * Removing the scope parameter;
     * Removing the session parameter.
    
    The last point is especially important, as it'd mean moving the creation of that session in the ResourceState object. In turns that would allow removing the tie RetrieveItemsTask has to Akonadi::Session which is a layer violation. Since this patch seek to remove this layer violation, this too should be fixed.


- Kevin Ottens


On July 28, 2013, 7:03 p.m., Dan Vrátil wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/111755/
> -----------------------------------------------------------
> 
> (Updated July 28, 2013, 7:03 p.m.)
> 
> 
> Review request for KDEPIM and Kevin Ottens.
> 
> 
> Description
> -------
> 
> 1) makes the tests compile
> 2) adds a new abstraction that allows us to move Akonadi::ItemFetchJob from RetrieveItemsTask to ImapResource and replace it by a dummy Job in unittests
> 3) fixes MoveItemsTask to pass tests
> 4) updates all unittests to pass (changed behavior in tasks)
> 
> For TestMoveItemsTask to pass, a patch in KIMAP is needed (will post in a new review)
> 
> 
> Diffs
> -----
> 
>   resources/imap/CMakeLists.txt 8abb522 
>   resources/imap/imapresource.h a137497 
>   resources/imap/imapresource.cpp a2f16fd 
>   resources/imap/moveitemstask.cpp 5dc7021 
>   resources/imap/resourcestate.h 87e9811 
>   resources/imap/resourcestate.cpp 5d521a5 
>   resources/imap/resourcestateinterface.h 7bfe5a3 
>   resources/imap/resourcetask.h 4a121fb 
>   resources/imap/resourcetask.cpp ce780cb 
>   resources/imap/retrieveitemstask.h b90cac1 
>   resources/imap/retrieveitemstask.cpp 1df3f6e 
>   resources/imap/tests/CMakeLists.txt 295a666 
>   resources/imap/tests/dummyresourcestate.h eb4b2ef 
>   resources/imap/tests/dummyresourcestate.cpp d1e4e29 
>   resources/imap/tests/testaddcollectiontask.cpp c8816d3 
>   resources/imap/tests/testmoveitemstask.cpp PRE-CREATION 
>   resources/imap/tests/testmoveitemtask.cpp 3bf67b2 
>   resources/imap/tests/testremoveitemstask.cpp PRE-CREATION 
>   resources/imap/tests/testremoveitemtask.cpp 0e61135 
>   resources/imap/tests/testretrieveitemstask.cpp b542423 
> 
> Diff: http://git.reviewboard.kde.org/r/111755/diff/
> 
> 
> Testing
> -------
> 
> All tests pass
> 
> 
> Thanks,
> 
> Dan Vrátil
> 
>

_______________________________________________
KDE PIM mailing list kde-pim at kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/


More information about the kde-pim mailing list