<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/111081/">http://git.reviewboard.kde.org/r/111081/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On June 18th, 2013, 2:06 p.m. UTC, <b>Kevin Ottens</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://git.reviewboard.kde.org/r/111081/diff/1/?file=164098#file164098line209" style="color: black; font-weight: bold; text-decoration: underline;">kio/kio/job.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void JobPrivate::slotSpeed( KJob*, unsigned long speed )</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">196</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">return</span> <span class="n"><span class="hl">uiDelegat</span>e</span><span class="p">()</span> <span class="o">!=</span> <span class="mi">0</span><span class="p">;</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">209</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">return</span> <span class="n"><span class="hl">interactionInterfac</span>e</span><span class="p">()</span> <span class="o">!=</span> <span class="mi">0</span><span class="p">;</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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)
</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Well spotted. I removed the method now, added a note in the porting file, and used interactionInterface() in CopyJob.</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On June 18th, 2013, 2:06 p.m. UTC, <b>Kevin Ottens</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://git.reviewboard.kde.org/r/111081/diff/1/?file=164106#file164106line264" style="color: black; font-weight: bold; text-decoration: underline;">kio/tests/jobtest.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void JobTest::copyLocalFile( const QString& src, const QString& dest )</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">264</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">job</span><span class="o">-></span><span class="n">setInteractionInterface</span><span class="p">(</span><span class="mi">0</span><span class="p">);</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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...
</pre>
<br />
<p>- David</p>
<br />
<p>On June 18th, 2013, 7:39 a.m. UTC, David Faure wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for KDE Frameworks.</div>
<div>By David Faure.</div>
<p style="color: grey;"><i>Updated June 18, 2013, 7:39 a.m.</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">make test</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>KDE5PORTING.html <span style="color: grey">(9cac0c69861b999d968c7af5bdeb113b17086b86)</span></li>
<li>kio/kio/copyjob.cpp <span style="color: grey">(e403735f05d644463122728af3a0694454d31367)</span></li>
<li>kio/kio/job.cpp <span style="color: grey">(365752866ef44d79b42c41af38b446c9fd407c7b)</span></li>
<li>kio/kio/job_p.h <span style="color: grey">(e1dd97995825a372d81a8098e0da38bb821b7a20)</span></li>
<li>kio/kio/jobclasses.h <span style="color: grey">(27a0f12788d35b95934ac714b58df99478e36c31)</span></li>
<li>kio/kio/jobuidelegate.h <span style="color: grey">(87d9d2777f8bd28e417f54900bf85826fef7fa9b)</span></li>
<li>kio/kio/jobuidelegate.cpp <span style="color: grey">(9f7e1b2093bb22fff5105bdc958ef519bbee8fcb)</span></li>
<li>kio/kio/renamedialog.h <span style="color: grey">(2b9efe21bb5266ced199d2be9b2f8e45df2be4d3)</span></li>
<li>kio/kio/skipdialog.h <span style="color: grey">(93dd30958377fb997a9f10fa2c3cba7702b14350)</span></li>
<li>kio/kio/skipdialog.cpp <span style="color: grey">(98251e058704ef7b8f7fc9b349d8da95c121d2ae)</span></li>
<li>kio/tests/jobtest.cpp <span style="color: grey">(7a50857eee19faee995ebbe30e5ba0f954cc858d)</span></li>
<li>kio/tests/kdirmodeltest.cpp <span style="color: grey">(6d9f89070d8579bccb494cbeca080498627e2b1e)</span></li>
<li>kio/tests/kiotesthelper.h <span style="color: grey">(6ecf13409d0117148cc52c774aa5fced37a28e1d)</span></li>
<li>staging/kjobwidgets/src/kdialogjobuidelegate.h <span style="color: grey">(f33d34911db53f6e1a90589e70bc9396f049ffc4)</span></li>
<li>staging/kjobwidgets/src/kdialogjobuidelegate.cpp <span style="color: grey">(a79eda86f8df34c6996a2f7285ba05466c6f8097)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/111081/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>