dfaure requested changes to this revision. dfaure added a comment. This revision now requires changes to proceed.
Looks good, just some minor improvement suggestions for the unittest. INLINE COMMENTS > dropjobtest.cpp:361 > + KIO::CopyJob *copyJob = KIO::move(QUrl::fromLocalFile(m_srcFile), > QUrl(QStringLiteral("trash:/"))); > + QSignalSpy copyingDoneSpy(copyJob, > SIGNAL(copyingDone(KIO::Job*,QUrl,QUrl,QDateTime,bool,bool))); > + QVERIFY(copyJob->exec()); Would be simpler with the PMF connect syntax ;) QSignalSpy copyingDoneSpy(copyJob, &KIO::CopyJob::copyingDone); > dropjobtest.cpp:376 > + // Then an error should be reported and no files action should occur > + QVERIFY2(job->exec() == false, qPrintable(job->errorString())); > + QVERIFY(job->error() == KIO::ERR_DROP_ON_ITSELF); If exec() returns true, there won't be an errorString. So this line can be simplified to QVERIFY(!job->exec()); > dropjobtest.cpp:377 > + QVERIFY2(job->exec() == false, qPrintable(job->errorString())); > + QVERIFY(job->error() == KIO::ERR_DROP_ON_ITSELF); > + } QVERIFY(a==b) should always be QCOMPARE(a, b) instead, to see both values in case of a failure. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D19168 To: hallas, #frameworks, elvisangelaccio, ngraham, dfaure Cc: kde-frameworks-devel, michaelh, ngraham, bruns