Review Request 112561: Ensure that the items are sorted correctly when the refreshItems() signal is received

[Emmanuel Pescosta]

> On Sept. 6, 2013, 5:21 p.m., Emmanuel Pescosta wrote:
> > One unit test failed.
> > 
> > ********* Start testing of KFileItemModelTest *********
> > Config: Using QTest library 4.8.5, Qt 4.8.5
> > PASS   : KFileItemModelTest::initTestCase()
> > PASS   : KFileItemModelTest::testDefaultRoles()
> > PASS   : KFileItemModelTest::testDefaultSortRole()
> > PASS   : KFileItemModelTest::testDefaultGroupedSorting()
> > PASS   : KFileItemModelTest::testNewItems()
> > Critical: qttest(7642)/kdecore (K*TimeZone*): No time zone information obtained from ktimezoned 
> > PASS   : KFileItemModelTest::testRemoveItems()
> > PASS   : KFileItemModelTest::testDirLoadingCompleted()
> > PASS   : KFileItemModelTest::testSetData()
> > PASS   : KFileItemModelTest::testSetDataWithModifiedSortRole()
> > PASS   : KFileItemModelTest::testChangeSortRole()
> > PASS   : KFileItemModelTest::testResortAfterChangingName()
> > PASS   : KFileItemModelTest::testModelConsistencyWhenInsertingItems()
> > PASS   : KFileItemModelTest::testItemRangeConsistencyWhenInsertingItems()
> > PASS   : KFileItemModelTest::testExpandItems()
> > PASS   : KFileItemModelTest::testExpandParentItems()
> > PASS   : KFileItemModelTest::testMakeExpandedItemHidden()
> > PASS   : KFileItemModelTest::testSorting()
> > PASS   : KFileItemModelTest::testIndexForKeyboardSearch()
> > PASS   : KFileItemModelTest::testNameFilter()
> > PASS   : KFileItemModelTest::testEmptyPath()
> > PASS   : KFileItemModelTest::testRefreshExpandedItem()
> > PASS   : KFileItemModelTest::testRemoveHiddenItems()
> > PASS   : KFileItemModelTest::collapseParentOfHiddenItems()
> > PASS   : KFileItemModelTest::removeParentOfHiddenItems()
> > PASS   : KFileItemModelTest::testGeneralParentChildRelationships()
> > FAIL!  : KFileItemModelTest::testNameRoleGroups() 'QTest::kWaitForSignal(m_model, SIGNAL(itemsMoved(KItemRange,QList<int>)), DefaultTimeout)' returned FALSE. ()
> >    Loc: [/home/emmanuel/kde-devel/src/kde-baseapps/dolphin/src/tests/kfileitemmodeltest.cpp(1275)]
> > PASS   : KFileItemModelTest::cleanupTestCase()
> > Totals: 26 passed, 1 failed, 0 skipped
> > ********* Finished testing of KFileItemModelTest *********
> 
> Frank Reininghaus wrote:
>     I forgot to mention that this test is expected to fail if you apply the patch to master. This is because re-sorting does not cause the itemsMoved signal to be emitted any more in master if the items are not moved at all. When this test is merged to master, it needs to be modified to listen to the groupsChanged signal instead.

Ah ok, thx


- Emmanuel


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


On Sept. 6, 2013, 3:29 p.m., Frank Reininghaus wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/112561/
> -----------------------------------------------------------
> 
> (Updated Sept. 6, 2013, 3:29 p.m.)
> 
> 
> Review request for Dolphin.
> 
> 
> Description
> -------
> 
> A side-effect of https://git.reviewboard.kde.org/r/111146/ is that the model is sometimes not sorted correctly after a refreshItems() signal has been received:
> 
> 1. mkdir test && cd test && touch a b c && dolphin .
> 2. Sort by size -> the order of the items is "a b c" because the name is used as a fallback (all files have zero size).
> 3. Rename "a" -> "d", either using non-inline renaming or in another view (such that the model receives the refreshItems() signal).
> 4. Note that the items are not resorted.
> 
> This patch fixes the problem. I've factored out some code that was added in https://git.reviewboard.kde.org/r/111721/ to a new function, such that both setData() and slotRefreshItems() can make use of it.
> 
> I added a unit test for the bug and another one for the grouping problem that Emmanuel found in the first version of https://git.reviewboard.kde.org/r/111721/
> 
> 
> Diffs
> -----
> 
>   dolphin/src/kitemviews/kfileitemmodel.h 5917e68 
>   dolphin/src/kitemviews/kfileitemmodel.cpp 7bbc1d1 
>   dolphin/src/tests/kfileitemmodeltest.cpp f843978 
> 
> Diff: http://git.reviewboard.kde.org/r/112561/diff/
> 
> 
> Testing
> -------
> 
> Works. Old and new unit tests pass.
> 
> 
> Thanks,
> 
> Frank Reininghaus
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20130907/c6106196/attachment.htm>