Notice of intention to remove tests from KCrash and KNotifications

[Ben Cooksley]

On Wed, 6 Nov 2019, 01:02 Harald Sitter, <sitter at kde.org> wrote:

> I think on a CI level we can only disable test executation as a whole,
> so that's an even bigger sledgehammer ;)
>

That is correct.

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.

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.

So I think that is a hammer that we should definitely avoid using.

Cheers,
Ben


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