bruns requested changes to this revision. bruns added inline comments. This revision now requires changes to proceed.
INLINE COMMENTS > jobtest.cpp:568 > + > +void JobTest::compareXattr(const QString &src, const QString &dest, QString > &command) > +{ remove the command parameter from the method, use e.g. a `m_xattrGetCommand` JobTest member > jobtest.cpp:593 > + > + QStringList arguments = {"-n", "name", src}; > + QHashIterator<QString, QString> i(attrs); By just iterating over the expected xattrs, extraneous attrs will be silently skipped. Use '-d' > jobtest.cpp:613 > + > + arguments.replace(2, dest); > + xattrreader.start(command, arguments); Redundant code. Call this function once after writing the source, compare with what you tried to write. Call another time for each copy operation. > jobtest.cpp:626 > + > +QString getXattrCommand() > +{ Do the lookup once in `JobTest::initTestCase` > jobtest.cpp:626 > + > +QString getXattrCommand() > +{ Check for both set and get commands. > jobtest.cpp:641 > + } > + return command.split("/").last(); > +} Keep the full path. > jobtest.cpp:687 > + QVERIFY2(job->exec(), qPrintable(job->errorString())); > + compareXattr(src, dest, command); // Our test > + If this copy never happened, the check would still succeed. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D17816 To: cochise, dfaure, chinmoyr, bruns, #frameworks, tmarshall Cc: usta, scheirle, anthonyfieroni, tmarshall, arrowd, cfeck, bruns, phidrho, dhaumann, funkybomber, abika, pino, davidedmundson, ngraham, atha.kane, spoorun, nicolasfella, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh