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 <bcooks...@kde.org> wrote:
On Tue, Nov 5, 2019 at 11:11 PM Harald Sitter <sit...@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 <bcooks...@kde.org> wrote:
On Tue, Nov 5, 2019 at 1:20 AM Harald Sitter <sit...@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 <bcooks...@kde.org> wrote:
On Mon, Nov 4, 2019 at 10:53 PM David Edmundson
<da...@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
