Notice of intention to remove tests from KCrash and KNotifications

[Ben Cooksley]

On Tue, Nov 5, 2019 at 11:50 PM Harald Sitter <sitter at kde.org> 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.

Unfortunately reviews require responsive developers.

In this instance, both Frameworks have been found to have developers
which are not responsive (because my previous requests have been
ignored), so sending a patch for review would have been pointless -
because there would have been nobody to review it.

Therefore bypassing review and taking any necessary action against the
offending code (even if that does happen to be all of the tests) to
correct the situation is the only viable option forward.

>
> 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.

This would require spending more time on the issue, which is something
i'd very much rather avoid.

It is expensive enough having to login to the CI builders to clear out
these jobs when they get stuck (looking at anything that uses Akonadi
especially here, along with these two Frameworks) and then asking the
project lists to please fix their faulty code (which is then ignored,
indicating that the concept of community maintained does not really
work).

Having to then lookup someone and then forward it on to them requires
yet more time, with no guarantee that it will work - especially
because some contributors are still hostile to any platform that is
not FOSS and outright refuse to do anything to help those platforms.

In the event that it does not work - then what would I do? Pick on the
next person in the list (requiring yet more time)?

Sorry, but that is simply too expensive, and if that is the policy
you'd like to run with, then i'll stick to stripping projects of their
test execution privileges outright across all platforms in the event
they cause issues like this (which if ECM is anything to go by, is
something people won't even notice even if the commit that puts that
in effect is CC'ed to the project mailing list, which means that
nobody will notice when the tests break because the CI system won't be
running them)

Regards,
Ben

>
> 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