fvogt added a comment.
In D10716#391331 <https://phabricator.kde.org/D10716#391331>, @jriddell wrote: > > Is it necessary to write the mock su/sudo in python? That introduces a big and mostly unnecessary dependency on python. > > Nope but that's what was easiest for me and I'm out of energy for this > > > The testcases would be simpler if it used QTESTDATA and rows for the su/sudo and correct/incorrect password cases > > Something else I need to learn about. Ok, those aren't major issues anyway. >> 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. > > I looked at that but I'd be scared of introducing binary incompatiblity. This approach covers the current needs. I don't really like that there's now a config entry only used for and during testing, maybe an environment variable would work? Other than that, this is still missing: - Autotests need to be conditional based on BUILD_TESTING It would be nice to have a check for python3 as well, but IMO the message on failure should be obvious enough. INLINE COMMENTS > jriddell wrote in su:28 > Not sure what you mean by won't work anywhere else, it'll work using the > kdesu_stub which is built by the local compile. It doens't need kdesu_stub > to be installed. Now it doesn't - the comment was referring to the hardcoded path to kdesu_stub, which is now gone. To avoid such misleading comments in the future, you can mark a comment as done to prevent it from reappearing out of context. > suprocess.cpp:122 > + KConfigGroup group(config, "super-user-command"); > + QByteArray defaultPath = QByteArray(CMAKE_INSTALL_FULL_LIBEXECDIR_KF5) + > "/kdesu_stub"; > + QString kdesuStubPath = group.readEntry("kdesu_stub_path", > QString::fromLatin1(defaultPath)); Use QStringLiteral instead. > suprocess.h:75 > private: > + friend class KdeSuTest; > enum SuErrors { Still necessary? REPOSITORY R299 KDESu REVISION DETAIL https://phabricator.kde.org/D10716 To: jriddell, sitter, fvogt Cc: fvogt, kde-frameworks-devel, michaelh, ngraham, bruns
