D28169: [krununittest] Skip the test if ktelnetservice5{, .desktop} isn't found

[David Faure]

dfaure added a comment.


  In D28169#631893 , @ahmadsamir 
wrote:
  
  > Yesterday, I did try tweaking XDG_DATA_DIRS to make it find the .desktop 
file without it being in /usr/share/applications
  
  
  It's more portable to use QStandardPaths test mode and copy the file where 
QStandardPaths will expect it.
  XDG_DATA_DIRS wouldn't work on macOS or Windows.
  
  > but I didn't think that it'd take time for ksyscoca to pick it up (or even 
that ksycoca was involved, though in retrospect that makes sense).
  
  KMimeTypeTrader is all about querying ksycoca.
  
  > - I still think the workaround for vlc should be tightened, with e.g. 
checking for .desktop file name
  
  I don't think this is VLC specific at all.
  xine or mplayer or other non-KIO-based apps would have the exact same 
problem. They can't get hold of the password.
  Anyhow, to be discussed with the FUSE people if I'm missing something. As you 
probably know, I'm not involved in the FUSE stuff.
  
  > - Currently the code calls hasSchemeHandler() twice, once from KRun::init() 
and then in resultingArguments(); also for 
KMimeTypeTrader::self()->preferredService(), it's called from KRun 
schemeService() and from hasSchemeHandler() (the latter is used twice so, 
preferredService() is called three times :D)
  
  Interesting point.
  Checking hasSchemeHandler in both KRun and inside the implementation 
ProcessLauncherJob makes sense to me. It's two different layers of public API; 
passing information between the two would make for horrible API.
  In KRun it allows us to say ok, we don't need to find the mimetype, go 
straight to running the associated service.
  In KProcessRunner (used by ProcessLauncherJob), it's indeed a bit stupid to 
look up the associated service when we *are* running this service. Oh, 
especially since KIO::DesktopExecParser::supportedProtocols already grabs those 
protocols from the MimeType... So `supported` is true. Then what? why? 
Debugging further OK I see. The earlier comment I deleted is true again. 
ssh://root@ has username, so this makes the fuse request, fails, and then 
blindly sets useKioexec = true for no good reason. I'll fix that logic. And 
then we can remove the unneeded hasSchemeHandler() hack.
  
  Thanks for making me dig further ;)

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D28169

To: ahmadsamir, #frameworks, dfaure, sitter, feverfew, ngraham
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D28169: [krununittest] Skip the test if ktelnetservice5{, .desktop} isn't found

[Ahmad Samir]

ahmadsamir abandoned this revision.
ahmadsamir added a comment.


  In D28169#631885 , @dfaure wrote:
  
  > OK, it's fixed now, after 3 commits :-)
  >  You can drop this change, it's better that CI actually tests the code.
  
  
  [..]
  Yesterday, I did try tweaking XDG_DATA_DIRS to make it find the .desktop file 
without it being in /usr/share/applications, but I didn't think that it'd take 
time for ksyscoca to pick it up (or even that ksycoca was involved, though in 
retrospect that makes sense).
  
  However, a couple of points that are still bothering me (consider them food 
for thought):
  
  - I still think the workaround for vlc should be tightened, with e.g. 
checking for .desktop file name
  - Currently the code calls hasSchemeHandler() twice, once from KRun::init() 
and then in resultingArguments(); also for 
KMimeTypeTrader::self()->preferredService(), it's called from KRun 
schemeService() and from hasSchemeHandler() (the latter is used twice so, 
preferredService() is called three times :D)

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D28169

To: ahmadsamir, #frameworks, dfaure, sitter, feverfew, ngraham
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D28169: [krununittest] Skip the test if ktelnetservice5{, .desktop} isn't found

[David Faure]

dfaure added a comment.


  OK, it's fixed now, after 3 commits :-)
  You can drop this change, it's better that CI actually tests the code.
  Thanks for your help though (with this, and with everything else - very much 
appreciated)

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D28169

To: ahmadsamir, #frameworks, dfaure, sitter, feverfew, ngraham
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D28169: [krununittest] Skip the test if ktelnetservice5{, .desktop} isn't found

[David Faure]

dfaure added a comment.


  The bug is that, after the fuse dbus call fails, we can't just blindly go to 
kioexec. It depends on `supported`. ktelnetservice supports ssh, so we don't 
need kioexec. Will rework...

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D28169

To: ahmadsamir, #frameworks, dfaure, sitter, feverfew, ngraham
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D28169: [krununittest] Skip the test if ktelnetservice5{, .desktop} isn't found

[David Faure]

dfaure added a comment.


  In D28169#631841 , @ahmadsamir 
wrote:
  
  > In D28169#631809 , @dfaure wrote:
  >
  > > Ah, we had tried already to make it work uninstalled.
  > >
  > > The actual way to do that would be to copy that desktop file into a local 
ApplicationsLocation. I'll do that, I just did something similar in KParts.
  >
  >
  > That ApplicationsLocation will be on the CI system?
  
  
  Yes, in ~/.qttest/share/applications
  See https://commits.kde.org/kio/b60aae15eaf97bc921fb7ad3a2068448f303055f
  But it still fails, I'm trying to debug why.
  
  Can't reproduce it locally even after uninstaling kio and all 
ktelnetservice*.desktop from /usr, so the problem is something else...

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D28169

To: ahmadsamir, #frameworks, dfaure, sitter, feverfew, ngraham
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D28169: [krununittest] Skip the test if ktelnetservice5{, .desktop} isn't found

[Ahmad Samir]

ahmadsamir added a comment.


  In D28169#631809 , @dfaure wrote:
  
  > Ah, we had tried already to make it work uninstalled.
  >
  > The actual way to do that would be to copy that desktop file into a local 
ApplicationsLocation. I'll do that, I just did something similar in KParts.
  
  
  That ApplicationsLocation will be on the CI system?

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D28169

To: ahmadsamir, #frameworks, dfaure, sitter, feverfew, ngraham
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D28169: [krununittest] Skip the test if ktelnetservice5{, .desktop} isn't found

[Ahmad Samir]

ahmadsamir updated this revision to Diff 78156.
ahmadsamir added a comment.


  Verbatim

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28169?vs=78152&id=78156

BRANCH
  l-krun-ssh-2 (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D28169

AFFECTED FILES
  autotests/krununittest.cpp

To: ahmadsamir, #frameworks, dfaure, sitter, feverfew, ngraham
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D28169: [krununittest] Skip the test if ktelnetservice5{, .desktop} isn't found

[David Faure]

dfaure added a comment.


  Ah, we had tried already to make it work uninstalled.
  
  The actual way to do that would be to copy that desktop file into a local 
ApplicationsLocation. I'll do that, I just did something similar in KParts.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D28169

To: ahmadsamir, #frameworks, dfaure, sitter, feverfew, ngraham
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D28169: [krununittest] Skip the test if ktelnetservice5{, .desktop} isn't found

[Ahmad Samir]

ahmadsamir updated this revision to Diff 78152.
ahmadsamir retitled this revision from "[DesktopExecParser] Let the service 
open protocols is claims to support" to "[krununittest] Skip the test if 
ktelnetservice5{,.desktop} isn't found".
ahmadsamir edited the test plan for this revision.
ahmadsamir added a comment.


  Just fix unittest

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28169?vs=78120&id=78152

BRANCH
  l-krun-ssh-2 (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D28169

AFFECTED FILES
  autotests/krununittest.cpp

To: ahmadsamir, #frameworks, dfaure, sitter, feverfew, ngraham
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns