D17545: Do not stat move/copy job if the destination file system does not support writing

David Faure noreply at phabricator.kde.org
Sun Dec 16 17:31:31 GMT 2018


dfaure added inline comments.

INLINE COMMENTS

> jobtest.cpp:1004
> +
> +    QSignalSpy spy(job, &KJob::error);
> +    job->setUiDelegate(nullptr);

At runtime, Qt says:

  QSignalSpy: Not a valid signal: ''

Indeed this is the getter, not a signal. `result()` is the signal, but you don't really need to connect to it anyway.

> jobtest.cpp:1007
> +    QVERIFY(!job->exec());
> +    QCOMPARE(job->error(), (int)KIO::ERR_CANNOT_WRITE);
> +    QCOMPARE(spy.count(), 1); // one error should be emitted by the move job

FAIL!  : JobTest::moveDirectoryToInaccessibleFilesystem() Compared values are not the same

  Actual   (job->error())              : 115
  Expected ((int)KIO::ERR_CANNOT_WRITE): 129

115 is ERR_ACCESS_DENIED, see `grep -w 15 src/core/global.h `

I think that's because you went too far (due to copy/pasting). The dest dir shouldn't be inaccessible, it should only be readonly.

Also you forgot the cleanup at the end of the method, which breaks running the test multiple times.

OK, even with all this the test doesn't pass.
Stepping into statNextSrc, I see that it goes into startRenameJob on line 846 so it doesn't get into your new code which is further down.
That's because the src and dest are on the same partition, so a simple rename is enough.

Changing dst_dir to use otherTmpDir() ... ok, the error code changes, now it's 137, i.e. ERR_COULD_NOT_MKDIR :-)
This requires further investigation...

REPOSITORY
  R241 KIO

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

To: shubham, #frameworks, dfaure
Cc: davidedmundson, ngraham, broulik, kde-frameworks-devel, michaelh, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20181216/c98782e0/attachment.html>


More information about the Kde-frameworks-devel mailing list