<div dir="auto"><div><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, 6 Nov 2019, 01:02 Harald Sitter, <<a href="mailto:sitter@kde.org">sitter@kde.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">I think on a CI level we can only disable test executation as a whole,<br>
so that's an even bigger sledgehammer ;)<br></blockquote></div></div><div dir="auto"><br></div><div dir="auto">That is correct.</div><div dir="auto"><br></div><div dir="auto">That was the hammer that extra CMake modules was hit with back in 2017, when a similar issue occurred with a broken test there, and once again nobody responded when the issue was reported.</div><div dir="auto"><br></div><div dir="auto">In the time since then, numerous tests broke with nobody noticing until I got a request just recently to reinstate running of tests for ECM as the offending test had been removed. </div><div dir="auto"><br></div><div dir="auto">So I think that is a hammer that we should definitely avoid using.</div><div dir="auto"><br></div><div dir="auto">Cheers,</div><div dir="auto">Ben</div><div dir="auto"><br></div><div dir="auto"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
For KCrash at least we've now implemented the aforementioned<br>
workaround where the broken test is disabled for windows. For<br>
knotifications I trust Kai will come up with a solution once he's back<br>
from QtWS.<br>
<br>
On Tue, Nov 5, 2019 at 12:10 PM Adrian Chaves <<a href="mailto:adrian@chaves.io" target="_blank" rel="noreferrer">adrian@chaves.io</a>> wrote:<br>
><br>
> Would it make sense to re-enable those tests in the code repositories<br>
> but disable CI for the corresponding repositories until someone<br>
> addressed the issues affecting overall CI?<br>
><br>
> On 2019-11-05 11:50, Harald Sitter wrote:<br>
><br>
> > I get where you are coming from, but honestly, none of this makes<br>
> > pushing unreviewed commits that disable the entire test collection of<br>
> > an entire framework any more correct. If it had only affected the one<br>
> > test I wouldn't mind so much, but since it hasn't it only goes to<br>
> > proof that we have reviews for a reason. In particular for repos that<br>
> > are part of frameworks where we rely on tests to tell us if the<br>
> > frameworks are good for the monthly release.<br>
> ><br>
> > There are many shades of grey between sending a "someone please fix"<br>
> > mail to a mailing list and the nuclear option you implemented. The<br>
> > most notable one being that you can ask someone who worked on the<br>
> > repo, or tests recently "Hey, X, can you please disable the test on<br>
> > windows because its impairing CI?" to which the answer is probably yes<br>
> > because you'd not only address a very specific person but also the<br>
> > task is doable in less than 5 minutes.<br>
> > I understand that there's an element of cat herding in this, but<br>
> > quality must matter for our products, and the quality of frameworks is<br>
> > very tightly linked to autotesting and reviews because of how we<br>
> > release them.<br>
> ><br>
> > On Tue, Nov 5, 2019 at 11:24 AM Ben Cooksley <<a href="mailto:bcooksley@kde.org" target="_blank" rel="noreferrer">bcooksley@kde.org</a>> wrote:<br>
> > On Tue, Nov 5, 2019 at 11:11 PM Harald Sitter <<a href="mailto:sitter@kde.org" target="_blank" rel="noreferrer">sitter@kde.org</a>> wrote:<br>
> > Perhaps you need to find a minion to do these changes for you then or<br>
> > read up on cmake and/or put these changes through review, because for<br>
> > KCrash you also disabled and unrelated test :|<br>
> > It would be nice if people would take action to either disable the<br>
> > offending tests or correct the breakage within the tests when I first<br>
> > mention it.<br>
> ><br>
> > Unfortunately as people have not been doing so and because it is<br>
> > causing me issues at the whole-of-KDE level (and therefore inflicting<br>
> > harm on not just this Framework, but on all of KDE by delaying the CI<br>
> > system from completing builds for other projects) i'm forced to take<br>
> > rather heavy handed approaches to resolving the issue, which sometimes<br>
> > has the effect of creating collateral damage (which I consider<br>
> > acceptable in this instance, as the only one damaged is the project<br>
> > that failed to respond).<br>
> ><br>
> > While i'd rather not do this, the cost of not doing so is much<br>
> > greater, so taking a heavy handed approach to projects that fail to<br>
> > take corrective action when corrected will continue to be necessary.<br>
> ><br>
> > The other option of course is to simply terminate providing CI<br>
> > coverage of any form to projects that fail to take corrective action<br>
> > (for all platforms).<br>
> ><br>
> > Regards,<br>
> > Ben<br>
> ><br>
> > On Tue, Nov 5, 2019 at 10:24 AM Ben Cooksley <<a href="mailto:bcooksley@kde.org" target="_blank" rel="noreferrer">bcooksley@kde.org</a>> wrote:<br>
> > On Tue, Nov 5, 2019 at 1:20 AM Harald Sitter <<a href="mailto:sitter@kde.org" target="_blank" rel="noreferrer">sitter@kde.org</a>> wrote:<br>
> > Wouldn't the more appropriate workaround then be to disable the test on<br>
> > windows?<br>
> > If one had the appropriate knowledge of CMake to do so, quite possibly.<br>
> ><br>
> > Given that I don't however, and others haven't made the necessary<br>
> > changes (and nobody has taken action when I have mentioned these tests<br>
> > as causing issues) disabling the tests everywhere is the simplest path<br>
> > forward and allows the CI system to operate correctly for everyone<br>
> > rather than be disrupted by these two offending tests.<br>
> ><br>
> > Cheers,<br>
> > Ben<br>
> ><br>
> > On Mon, Nov 4, 2019 at 10:57 AM Ben Cooksley <<a href="mailto:bcooksley@kde.org" target="_blank" rel="noreferrer">bcooksley@kde.org</a>> wrote:<br>
> > On Mon, Nov 4, 2019 at 10:53 PM David Edmundson<br>
> > <<a href="mailto:david@davidedmundson.co.uk" target="_blank" rel="noreferrer">david@davidedmundson.co.uk</a>> wrote:<br>
> > Given kcrashtest passes locally, can you please confirm that by<br>
> > "remove" you mean disable and not remove.<br>
> > I mean remove.<br>
> ><br>
> > This test is highly dangerous and enters into a fork loop on Windows,<br>
> > necessitating use of an administrator level console prompt to recover<br>
> > the system.<br>
> > Fortunately the grand-parent process terminates after it's child has<br>
> > successfully forked, which is the only thing stopping this test from<br>
> > being a fork bomb and totally taking down the system.<br>
> ><br>
> > David<br>
> > Regards,<br>
> > Ben<br>
</blockquote></div></div></div>