D10716: handle wrong password when using sudo which asks for another password

[Fabian Vogt]

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20190110/29d78e9e/attachment-0001.html>