dfaure added a comment.
Looks good, just minor issues left. INLINE COMMENTS > fileundomanagertest.cpp:735 > + FileUndoManager::self()->recordCopyJob(copyJob); > + const bool ok = copyJob->exec(); > + QVERIFY(ok); This isn't Q_ASSERT, it's ok to perform the operation inside the macro. QVERIFY2(job->exec(), qPrintable(job->errorString())); > fileundomanagertest.cpp:744 > + const bool ok = deleteJob->exec(); > + QVERIFY(ok); > + QVERIFY(!QFileInfo::exists(dest.toLocalFile())); same here > fileundomanagertest.cpp:753 > + FileUndoManager::self()->undo(); > + QCOMPARE(spyUndoAvailable.count(), 1); > + QVERIFY(!FileUndoManager::self()->undoAvailable()); This is missing a check for the value of the bool emitted. QVERIFY(!spyUndoAvailable.at(0).at(0).toBool()); REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D10312 To: elvisangelaccio, dfaure Cc: ngraham, #frameworks, michaelh