Re: Notice of intention to remove tests from KCrash and KNotifications
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 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. > > This is really not true. > We got the workaround for KCrash landed in not even half a business > day - including review. > Unfortunately it is certainly my perception - which is based off of my emails on the subject not receiving any response. This experience is one that hasn't only happened with Frameworks - it has also happened (several times) with PIM. Taking direct action against a project is pretty much guaranteed to trigger a fairly rapid response (which is exactly what happened in this case). It should be noted that saying i'm going to take action usually results in either no response, or a response along the lines of "no, you can't do that" rather than the issue actually being addressed (which once again, is what happened in this case too) > > > 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) > > I did not express myself well enough. > What I'd like you to do is talk to someone, not anyone, a very > specific someone of your choosing, to take care of the issue for you. > IOW: The sysadmins should delegate to a developer if the sysadmins > find themselves unable to accurately deal with a code problem (such as > a failing test that needs deactivating). If you tell an actually > relevant developer that's obviously better, if you don't have time to > look at the git log then that's fine too. > > There are two different work items to differentiate here: > > 1. fixing something > 2. disabling something because a fix was not made in an acceptable time frame > > 1 likely requires domain-knowledge of the specific framework and > probably quit a bit of time. > 2 would generally only require understanding of cmake and a fairly
Re: Notice of intention to remove tests from KCrash and KNotifications
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 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. This is really not true. We got the workaround for KCrash landed in not even half a business day - including review. > > 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) I did not express myself well enough. What I'd like you to do is talk to someone, not anyone, a very specific someone of your choosing, to take care of the issue for you. IOW: The sysadmins should delegate to a developer if the sysadmins find themselves unable to accurately deal with a code problem (such as a failing test that needs deactivating). If you tell an actually relevant developer that's obviously better, if you don't have time to look at the git log then that's fine too. There are two different work items to differentiate here: 1. fixing something 2. disabling something because a fix was not made in an acceptable time frame 1 likely requires domain-knowledge of the specific framework and probably quit a bit of time. 2 would generally only require understanding of cmake and a fairly small time investment. Pretty much any dev can do task 2. You just need to make someone care, and the easiest way is to talk to a buddy directly and have them have a look. Like if you ask me to deal with this for sysadmins I'll definitely do. So, you have my permission to ask me to disable tests on your behalf ;) In short the process I would like is: 1. mail to list "yo, this framework has broken test xyz that is severely impairing CI. plz fix" 2. if nothing happens ask someone (e.g. me) to force an action on the repo (i.e. disable a specific test) 3. if the someone starts ghosting you you can still go in and do what you did the other day If you do 2 that has little overhead and a good chance of netting you less work, not more. HS
Re: Notice of intention to remove tests from KCrash and KNotifications
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 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 wrote: > > > > 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 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 wrote: > > > > > > > > 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 nobody has taken action when I have
Re: Notice of intention to remove tests from KCrash and KNotifications
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 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 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 > wrote: > > > 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 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 > wrote: > > > 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 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 > 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 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 >
Re: Notice of intention to remove tests from KCrash and KNotifications
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 he's back from QtWS. On Tue, Nov 5, 2019 at 12:10 PM Adrian Chaves 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 wrote: > > 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 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 wrote: > > 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 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 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 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
Re: Notice of intention to remove tests from KCrash and KNotifications
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 wrote: 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 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 wrote: 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 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 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 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
Re: Notice of intention to remove tests from KCrash and KNotifications
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 wrote: > > 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 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 wrote: > > > > > > 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 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 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 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
Re: Notice of intention to remove tests from KCrash and KNotifications
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 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 wrote: > > > > 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 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 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 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
Re: Notice of intention to remove tests from KCrash and KNotifications
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: > > > > 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 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 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
Re: Notice of intention to remove tests from KCrash and KNotifications
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 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 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 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
Re: Notice of intention to remove tests from KCrash and KNotifications
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 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
Re: Notice of intention to remove tests from KCrash and KNotifications
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 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
Re: Notice of intention to remove tests from KCrash and KNotifications
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
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 instances when they are started, and thus cause the job in question to indefinitely occupy the CI worker until manual action is taken to correct the issue. In the most recent instance, this wasn't corrected until 36 hours after the blockage began, which has had the effect of stopping all Windows builds across the entire CI system for the period in question (a backlog which the system is currently working to clear) This issue has been occurring at a minimum for several months now. The offending tests are: - 'kcrashtest' (executable at fault: test_crasher.exe) in KCrash - 'KNotificationTest' (executable at fault: dbus-daemon.exe) in KNotifications These issues have previously been raised, and as unfortunately no solution has yet been found, the only option now is to remove these tests from the codebase. Regards, Ben Cooksley KDE Sysadmin
