<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/122206/">https://git.reviewboard.kde.org/r/122206/</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 1st, 2015, 2:33 p.m. UTC, <b>David Faure</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Let me get this straight.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This patch makes the option BUILD_TESTING work, i.e. skip testing if not set.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The solution that was committed to kwin, <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">additionally</em> sets BUILD_TESTING to false if Qt5Test isn't available. This doesn't prevent setting the option to false manually if someone has Qt5Test but doesn't want to build the tests.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Is this an accurate description of the issue?</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I would add that, IMHO, all frameworks should handle this the same way.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I had a further look and they currently don't.
<em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;"> kservice, kimageformats, ktexteditor... do it like this patch (manual option).
</em> kcoreaddons, karchive... don't have the option, they just skip autotests if Qt5Test is not found.
* ki18n, kguiaddons, kdbusaddons, threadweaver compile autotests unconditionally.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I'm confused - how did anyone without Qt5Test make it all the way to kio, which depends on kdbusaddons and ki18n, then?</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Unless I missed something, I would say that by default tests should be compiled if Qt5Test is present (developers) and should not be compiled if Qt5Test is absent or if it is explicitly desired not to build tests (packagers).</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Therefore:
1) doing both steps is good (i.e. doing like kwin).
2) it should be done in all frameworks (that have autotests, which is most of them)
Anyone up to the task?</p></pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I guess what I was really trying to do initially - besides changing as little as possible - was mimicking the existing behaviour as much as possible. That means bailing out when Qt5Test is not found even though BUILD_TESTING was requested, so the user is informed about a dependency problem. The kwin solution will silently not build the tests, as far as I see it. If this is fine from a developer/packager point of view, I have a new patch for kio ready that goes the kwin way.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">@David, why it was discovered in kio, that's easy, you do some packaging for fun and get told your submission should not hard-depend on Qt5Test, then you uninstall that to look for any fallout and kio happens to be one of the next few packages to receive new commits, thus rebuild fails (because the existing test-removing automagic did not work).</p></pre>
<br />
<p>- Andreas</p>
<br />
<p>On January 22nd, 2015, 7:48 p.m. UTC, Andreas Sturmlechner wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
<div>Review request for KDE Frameworks.</div>
<div>By Andreas Sturmlechner.</div>
<p style="color: grey;"><i>Updated Jan. 22, 2015, 7:48 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kio
</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">[kio] Make tests optional
This is a small patch to CMakeLists.txt to only depend on Qt5Test if BUILD_TESTING.</p></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>CMakeLists.txt <span style="color: grey">(7fe0be5d4b2d7d9475a7844b4f8d93fc2f0a00c3)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/122206/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>