Review Request 111081: KIO::Job: more core/gui splitup.

[David Faure]

> On June 18, 2013, 2:06 p.m., Kevin Ottens wrote:
> > kio/kio/job.cpp, line 209
> > <http://git.reviewboard.kde.org/r/111081/diff/1/?file=164098#file164098line209>
> >
> >     Interesting... So not having the uiDelegate() but the interactionInterface() qualify as interactive... Not sure if that could be a problem. Maybe better to let it go and let client code test what they need (delegate or extra interface)
> >

Well spotted. I removed the method now, added a note in the porting file, and used interactionInterface() in CopyJob.


> On June 18, 2013, 2:06 p.m., Kevin Ottens wrote:
> > kio/tests/jobtest.cpp, line 264
> > <http://git.reviewboard.kde.org/r/111081/diff/1/?file=164106#file164106line264>
> >
> >     Do we agree that the goal is that both uiDelegate() and interactionInterface() will return 0 if you link only kiocore?
> >     
> >     In such case those lines will become "useless" in automated tests?
> >     
> >     At that point it's probably even better if in the tests initTestCase() methods you register mock interfaces which allow to check what the job calls in the interfaces.

Yes, however if you suddenly link another lib and it breaks your tests, that would count as rather unexpected, wouldn't it?
In any case, at this point, the splitup is not done, so I'm exactly in that situation: the widget-based interaction interface is getting registered, so I need the explicit setting-to-0.

About your last question, there are three cases, really:
1) in some cases we just expect the job to work, and if there's a bug, we want an error code rather than a widget popping up. That's the case at the line where you added the comment, see the QVERIFY(ok) two lines down.
2) I use mock interfaces in some places, see PredefinedAnswerJobUiDelegate used in kdirmodeltest.cpp:    job->setInteractionInterface(delegate);  [ok, the naming is a bit historical]
3) indeed there are also cases where we expect the job to fail, like the two lines with "no skip dialog, thanks" as comment. The point of the test wasn't to test the user interaction though. Could be added, of course, there's always room for more testing...


- David


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/111081/#review34581
-----------------------------------------------------------


On June 18, 2013, 7:39 a.m., David Faure wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/111081/
> -----------------------------------------------------------
> 
> (Updated June 18, 2013, 7:39 a.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Description
> -------
> 
> KIO::Job: more core/gui splitup.
> 
> job->ui() no longer returns a KIO-specific class which was holding
> QWidget* window and usertimestamp. It now returns the core-only base
> class, like in KJob. The widget-specific information is stored as
> a dynamic property into the job, using KJobWidgets API.
> 
> Split up delegate (for error handling) and "interaction interface"
> InteractionInterface is used for the rename and skip dialog, shown by
> copyjob and movejob.
> 
> 
> Diffs
> -----
> 
>   KDE5PORTING.html 9cac0c69861b999d968c7af5bdeb113b17086b86 
>   kio/kio/copyjob.cpp e403735f05d644463122728af3a0694454d31367 
>   kio/kio/job.cpp 365752866ef44d79b42c41af38b446c9fd407c7b 
>   kio/kio/job_p.h e1dd97995825a372d81a8098e0da38bb821b7a20 
>   kio/kio/jobclasses.h 27a0f12788d35b95934ac714b58df99478e36c31 
>   kio/kio/jobuidelegate.h 87d9d2777f8bd28e417f54900bf85826fef7fa9b 
>   kio/kio/jobuidelegate.cpp 9f7e1b2093bb22fff5105bdc958ef519bbee8fcb 
>   kio/kio/renamedialog.h 2b9efe21bb5266ced199d2be9b2f8e45df2be4d3 
>   kio/kio/skipdialog.h 93dd30958377fb997a9f10fa2c3cba7702b14350 
>   kio/kio/skipdialog.cpp 98251e058704ef7b8f7fc9b349d8da95c121d2ae 
>   kio/tests/jobtest.cpp 7a50857eee19faee995ebbe30e5ba0f954cc858d 
>   kio/tests/kdirmodeltest.cpp 6d9f89070d8579bccb494cbeca080498627e2b1e 
>   kio/tests/kiotesthelper.h 6ecf13409d0117148cc52c774aa5fced37a28e1d 
>   staging/kjobwidgets/src/kdialogjobuidelegate.h f33d34911db53f6e1a90589e70bc9396f049ffc4 
>   staging/kjobwidgets/src/kdialogjobuidelegate.cpp a79eda86f8df34c6996a2f7285ba05466c6f8097 
> 
> Diff: http://git.reviewboard.kde.org/r/111081/diff/
> 
> 
> Testing
> -------
> 
> make test
> 
> 
> Thanks,
> 
> David Faure
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20130618/d7a785aa/attachment-0001.html>