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

[Fabian Vogt]

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