D23875: KCoreDirLister: fix crash when creating new folders from kfilewidget

[Ahmad Samir]

ahmadsamir added a comment.


  Thanks for the explanation; it filled all the, various, gaps in my reasoning.
  
  Looks good to me. (I don't have a dev account, so probably phab.kde.org doesn't show the "accept revision" or similar in its web UI for me).

INLINE COMMENTS

> dfaure wrote in kfilewidgettest.cpp:516
> This test fails when being run twice in a row (without the fix), or if /tmp/folder1 already exists for some unrelated reason.
> 
> I suggest using QTemporaryDir instead of QDir::tempPath, so that the deletion of the QTemporaryDir cleans up in all cases (success or failure).
> 
> I did it locally to debug this, here it is:
> 
>   QTemporaryDir tempDir;
>   QVERIFY(tempDir.isValid());
>   const QString dir = tempDir.path();
>   const QUrl url = QUrl::fromLocalFile(dir); 
> 
> (and then you can remove the cleanup code at the end of the method)

Yes, it does fail if the dir got created and the test didn't finish running for whatever reason; I was thinking of CI (which will nuke the whole build and start over), rather than of other people running the test locally.

> dfaure wrote in kfilewidgettest.cpp:522
> show() and activateWindow() are not needed for this unittest to demonstrate the failure.
> 
> I'm trying to reduce the number of stuff popping up when running ctest, since that's rather annoying if I'm doing something else at the same time :-)

Yes, they're quiet sudden, and disappear too quickly too. But it won't matter for KIO tests, it's just one of many many dialogs :)

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D23875

To: dfaure, #frameworks, ahmadsamir
Cc: dhaumann, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20190915/62d3fbf1/attachment.html>