<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="https://git.reviewboard.kde.org/r/115759/">https://git.reviewboard.kde.org/r/115759/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On February 15th, 2014, 3:13 p.m. UTC, <b>Albert Astals Cid</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="https://git.reviewboard.kde.org/r/115759/diff/1/?file=244162#file244162line64" style="color: black; font-weight: bold; text-decoration: underline;">tests/searchtest.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; ">class SearchTest : public QObject</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">64</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">std</span><span class="o">::</span><span class="n">cerr</span> <span class="o"><<</span> <span class="s">"ERROR: text.size() != rect.size()"</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;">please use kDebug and remove the iostream include</pre>
</blockquote>
<p>On February 15th, 2014, 11:10 p.m. UTC, <b>Jaan Vajakas</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Actually I wanted something like QCOMPARE or an exception, so that the test were halted and error message shown. But QCOMPARE unfortunately doesn't work inside functions that are not tests themselves and when I tried exception throwing I got a compiler error that exceptions are disabled (and I don't know how the Qt test framework would handle them even if they were enabled). Then I chose stderr instead of kDebug since kDebug output can be disabled in kdebugdialog but I thought the test's reason of failure should always be shown (like the QCOMPARE error messages). But this is an unimportant error message anyway (it can only appear if the tests in searchtest.cpp themselves are incorrectly modified), so I don't oppose kDebug either. Is it a good style to never use std::cout and std::cerr, even in tests?</pre>
</blockquote>
<p>On February 16th, 2014, 11 p.m. UTC, <b>Albert Astals Cid</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">We just don't use iostreams much. If you need QCOMPARE just make the function non static and bring it to the SearchTest object as non slot (so it's not a test itself) and it should work.</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;">Now in revision r2 I use QCOMPARE. Actually from my tests with Qt 4.8.5 it seemed there is no difference in the behavior of QCOMPARE, whether it is called from a class member function or a global function: in either case it prints an error message and returns from the function, but does not abort the calling test function (i. e. does not behave like throwing an exception). But either way this is undocumented behavior, as the documentation ( http://qt-project.org/doc/qt-4.8/qtest.html#QCOMPARE ) says "Note: This macro can only be used in a test function that is invoked by the test framework.". So actually now I am thinking that perhaps it would be better if I refactor my function into a macro – then QCOMPARE would be used in accordance with the documentation.</pre>
<br />
<p>- Jaan</p>
<br />
<p>On February 22nd, 2014, 12:50 p.m. UTC, Jaan Vajakas wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://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 Okular.</div>
<div>By Jaan Vajakas.</div>
<p style="color: grey;"><i>Updated Feb. 22, 2014, 12:50 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="http://bugs.kde.org/show_bug.cgi?id=326207">326207</a>
</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
okular
</div>
<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;">Here is a patch to fix Bug 326207. It was a simple bug in the XY Cut layout recognition code that made it too eager to see columns everywhere. I also removed the dependence of the layout analysis algorithms on the display DPI (introduced by the recently added feature of using KScreen) to make their behavior more predictable and reproducible. I added tests for this bug and also refactored SearchTest a little to use QVectors instead of bare arrays.</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;">added automatic tests and tested manually on some PDF files</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>core/textpage.cpp <span style="color: grey">(a95f7c6)</span></li>
<li>tests/searchtest.cpp <span style="color: grey">(caceb6f)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/115759/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>