Re: Notice of intention to remove tests from KCrash and KNotifications

2019-11-09 Thread Ben Cooksley
On Thu, Nov 7, 2019 at 1:18 AM Harald Sitter wrote: > > On Wed, Nov 6, 2019 at 8:07 AM Ben Cooksley wrote: > > > > On Tue, Nov 5, 2019 at 11:50 PM Harald Sitter wrote: > > > > > > I get where you are coming from, but honestly, none of this makes > > > pushing unreviewed commits that disable the

Re: Notice of intention to remove tests from KCrash and KNotifications

2019-11-06 Thread Harald Sitter
On Wed, Nov 6, 2019 at 8:07 AM Ben Cooksley wrote: > > On Tue, Nov 5, 2019 at 11:50 PM 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

Re: Notice of intention to remove tests from KCrash and KNotifications

2019-11-05 Thread Ben Cooksley
On Tue, Nov 5, 2019 at 11:50 PM 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

Re: Notice of intention to remove tests from KCrash and KNotifications

2019-11-05 Thread Ben Cooksley
On Wed, 6 Nov 2019, 01:02 Harald Sitter, 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

Re: Notice of intention to remove tests from KCrash and KNotifications

2019-11-05 Thread Harald Sitter
I think on a CI level we can only disable test executation as a whole, so that's an even bigger sledgehammer ;) 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

Re: Notice of intention to remove tests from KCrash and KNotifications

2019-11-05 Thread Adrian Chaves
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

Re: Notice of intention to remove tests from KCrash and KNotifications

2019-11-05 Thread Harald Sitter
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

Re: Notice of intention to remove tests from KCrash and KNotifications

2019-11-05 Thread Ben Cooksley
On Tue, Nov 5, 2019 at 11:11 PM Harald Sitter 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

Re: Notice of intention to remove tests from KCrash and KNotifications

2019-11-05 Thread Harald Sitter
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 :| On Tue, Nov 5, 2019 at 10:24 AM Ben Cooksley wrote: > > On Tue, Nov 5, 2019 at 1:20 AM Harald Sitter wrote:

Re: Notice of intention to remove tests from KCrash and KNotifications

2019-11-05 Thread Ben Cooksley
On Tue, Nov 5, 2019 at 1:20 AM Harald Sitter 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

Re: Notice of intention to remove tests from KCrash and KNotifications

2019-11-04 Thread Harald Sitter
Wouldn't the more appropriate workaround then be to disable the test on windows? On Mon, Nov 4, 2019 at 10:57 AM Ben Cooksley wrote: > > On Mon, Nov 4, 2019 at 10:53 PM David Edmundson > wrote: > > > > Given kcrashtest passes locally, can you please confirm that by > > "remove" you mean disable

Re: Notice of intention to remove tests from KCrash and KNotifications

2019-11-04 Thread Ben Cooksley
On Mon, Nov 4, 2019 at 10:53 PM David Edmundson 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

Re: Notice of intention to remove tests from KCrash and KNotifications

2019-11-04 Thread David Edmundson
Given kcrashtest passes locally, can you please confirm that by "remove" you mean disable and not remove. David

Notice of intention to remove tests from KCrash and KNotifications

2019-11-04 Thread Ben Cooksley
Hi all, The following is notice that the following tests have been identified as causing issues to the operation of the CI system, and as such will be subject to removal to ensure the smooth running of the CI system. The below tests cause the execution workflow to indefinitely hang in all