fvogt requested changes to this revision. fvogt added a comment. This revision now requires changes to proceed.
In D10716#390699 <https://phabricator.kde.org/D10716#390699>, @jriddell wrote: > - set XDG_CONFIG_HOME to put kdesutestrc not in running users config dir What you want is `QStandardPaths::setTestModeEnabled(true);`. Some more general comments: - Autotests need to be conditional based on BUILD_TESTING - Is it necessary to write the mock su/sudo in python? That introduces a big and mostly unnecessary dependency on python. - By adding a new constructor to SuCommand which allows to specify the full path to su/sudo, testing would be much easier. It might be usable outside of the tests as well. - The testcases would be simpler if it used QTESTDATA and rows for the su/sudo and correct/incorrect password cases INLINE COMMENTS > kdesutest.cpp:3 > +#include <QTest> > +#include <../src/suprocess.h> > +#include <QString> Try to make `#include "suprocess.h"` work instead. > kdesutest.cpp:13 > +#define ROOTPASSWORD "ilovekde" > +#include "config-kdesutest.h" > + Should be above the defines. > kdesutest.cpp:45 > + void sudoBadPassword() { > + KSharedConfig::Ptr config = KSharedConfig::openConfig(); > + KConfigGroup group(config, "super-user-command"); The config modification should be split into a separate method. > suprocess.cpp:34 > > +#include "../autotests/config-kdesutest.h" > + If autotests aren't built, this isn't available. AFAICT it's not necessary anyway. > suprocess.cpp:44 > QString superUserCommand; > + bool testMode; > }; Not used anymore. REPOSITORY R299 KDESu REVISION DETAIL https://phabricator.kde.org/D10716 To: jriddell, sitter, fvogt Cc: fvogt, kde-frameworks-devel, michaelh, ngraham, bruns
