Re: treating B2G device tests as tier 1
This has irked me before too. An obvious compromise would that the backout proceeds, but it must include a test that would have failed on CI when the patch was landed. This puts the onus on the owners of the broken functionality to make sure that this supposedly-critical functionality has basic smoketest coverage, and gives the developer of the backed-out patch a way to verify that their code is correct on CI. Backouts should require tests. On Thu, Oct 16, 2014 at 4:33 AM, Ehsan Akhgari ehsan.akhg...@gmail.com wrote: On 2014-10-15, 10:19 PM, Karl Tomlinson wrote: Jonas Sicking writes: But any type of regression is cause for backout. While I agree regressions are bad, this isn't the usual process. If it were, then I wouldn't bother filing bugs, but merely back out the offending change. There is some kind test for whether the regression costs more than the improvements made, but it comes down to a judgement call from the module owner AIUI. Regressions that sit in the tree make it dramatically much harder to write and test other patches. It's generally much better to back the offending patch out to allow everyone else to go at full speed. Perhaps it is, but this would be quite a change in process. Some kind of policy or guidelines would be helpful, or it could well get out of control. Backouts usually cause regressions too. In my experience, regressions that break something in a way that makes dogfooding difficult are open to a backout without questions asked policy (but often times in practice we'd try to reach out to the author and the folks who know the area of the code). For other regressions we typically file follow-up bugs. Note that the definition of what makes dogfooding difficult is has not been entirely consistent all the time. Cheers, Ehsan ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: treating B2G device tests as tier 1
On 2014-10-16, 2:51 AM, Bobby Holley wrote: This has irked me before too. An obvious compromise would that the backout proceeds, but it must include a test that would have failed on CI when the patch was landed. This puts the onus on the owners of the broken functionality to make sure that this supposedly-critical functionality has basic smoketest coverage, and gives the developer of the backed-out patch a way to verify that their code is correct on CI. Backouts should require tests. I don't think it's reasonable to assume that the person doing the backout has the time or the expertise to add a test for the broken functionality. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: treating B2G device tests as tier 1
On Thu, Oct 16, 2014 at 10:04 AM, Ehsan Akhgari ehsan.akhg...@gmail.com wrote: On 2014-10-16, 2:51 AM, Bobby Holley wrote: This has irked me before too. An obvious compromise would that the backout proceeds, but it must include a test that would have failed on CI when the patch was landed. This puts the onus on the owners of the broken functionality to make sure that this supposedly-critical functionality has basic smoketest coverage, and gives the developer of the backed-out patch a way to verify that their code is correct on CI. Backouts should require tests. I don't think it's reasonable to assume that the person doing the backout has the time or the expertise to add a test for the broken functionality. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform I don't think that's what bholley is asking for. The onus should be on the developers of the relevant app/feature/whatever to write a test that catches the regression. - Kyle ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: treating B2G device tests as tier 1
On Thu, Oct 16, 2014 at 7:04 PM, Ehsan Akhgari ehsan.akhg...@gmail.com wrote: I don't think it's reasonable to assume that the person doing the backout has the time or the expertise to add a test for the broken functionality. Not the sheriff certainly, but I think if the regression is severe enough to warrant this action, the product owners (who are generally the ones who request the backout) can find the resources to make that happen. There will be situations where this is unrealistically difficult for one reason or another. But I'd rather put the onus on the product owners to ask for that exception, and presumably offer human resources to help the developer update and test their patch. If a team pulls this card, they should have a responsibility to help get the patch relanded in a timely manner. bholley ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: treating B2G device tests as tier 1
On 2014-10-16, 1:52 PM, Bobby Holley wrote: On Thu, Oct 16, 2014 at 7:04 PM, Ehsan Akhgari ehsan.akhg...@gmail.com mailto:ehsan.akhg...@gmail.com wrote: I don't think it's reasonable to assume that the person doing the backout has the time or the expertise to add a test for the broken functionality. Not the sheriff certainly, but I think if the regression is severe enough to warrant this action, the product owners (who are generally the ones who request the backout) can find the resources to make that happen. Who are the product owners exactly? Usually what happens in these cases is some discussion on IRC, followed by trying to ping the author/reviewer, followed by a backout either by a sheriff or another individual such as myself. There will be situations where this is unrealistically difficult for one reason or another. But I'd rather put the onus on the product owners to ask for that exception, and presumably offer human resources to help the developer update and test their patch. Again, I'm not sure who specifically you're referring to as the bearer of this responsibility. If a team pulls this card, they should have a responsibility to help get the patch relanded in a timely manner. I disagree. If someone breaks Nightly on desktop for example to an extent where it cannot be used for dogfooding, and I back them out to help out our Nightly users and keep the testing product usable so that other regressions can be caught with it, why should I feel responsible for relanding their patch in a timely manner? ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: treating B2G device tests as tier 1
On 16 October 2014 20:55, Ehsan Akhgari ehsan.akhg...@gmail.com wrote: On 2014-10-16, 1:52 PM, Bobby Holley wrote: On Thu, Oct 16, 2014 at 7:04 PM, Ehsan Akhgari ehsan.akhg...@gmail.com mailto:ehsan.akhg...@gmail.com wrote: I don't think it's reasonable to assume that the person doing the backout has the time or the expertise to add a test for the broken functionality. Not the sheriff certainly, but I think if the regression is severe enough to warrant this action, the product owners (who are generally the ones who request the backout) can find the resources to make that happen. Who are the product owners exactly? Usually what happens in these cases is some discussion on IRC, followed by trying to ping the author/reviewer, followed by a backout either by a sheriff or another individual such as myself. There will be situations where this is unrealistically difficult for one reason or another. But I'd rather put the onus on the product owners to ask for that exception, and presumably offer human resources to help the developer update and test their patch. Again, I'm not sure who specifically you're referring to as the bearer of this responsibility. If a team pulls this card, they should have a responsibility to help get the patch relanded in a timely manner. I disagree. If someone breaks Nightly on desktop for example to an extent where it cannot be used for dogfooding, and I back them out to help out our Nightly users and keep the testing product usable so that other regressions can be caught with it, why should I feel responsible for relanding their patch in a timely manner? The someone is the person that wrote the feature that was broken but no tests caught it I believe the general idea is that as a peer / module owner / product owner, I have the responsibility to write tests that ensure my feature works, and if it is broken by upstream changes that landed because automation didnt find anything wrong with it, then its my responsibility to ensure that tests are written so it doesnt get regressed in the same way again and automation can catch it. Otherwise with no visibility I am putting the reponsibility onto every other upstream developer to hopefully not break my code without any context for them to even know when they have done so. This is summed up in the meme: http://mozillamemes.tumblr.com/post/26210699924/you-reap-what-you-sow ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: treating B2G device tests as tier 1
On 2014-10-16, 3:56 PM, Dale Harvey wrote: On 16 October 2014 20:55, Ehsan Akhgari ehsan.akhg...@gmail.com mailto:ehsan.akhg...@gmail.com wrote: On 2014-10-16, 1:52 PM, Bobby Holley wrote: On Thu, Oct 16, 2014 at 7:04 PM, Ehsan Akhgari ehsan.akhg...@gmail.com mailto:ehsan.akhg...@gmail.com mailto:ehsan.akhgari@gmail.__com mailto:ehsan.akhg...@gmail.com wrote: I don't think it's reasonable to assume that the person doing the backout has the time or the expertise to add a test for the broken functionality. Not the sheriff certainly, but I think if the regression is severe enough to warrant this action, the product owners (who are generally the ones who request the backout) can find the resources to make that happen. Who are the product owners exactly? Usually what happens in these cases is some discussion on IRC, followed by trying to ping the author/reviewer, followed by a backout either by a sheriff or another individual such as myself. There will be situations where this is unrealistically difficult for one reason or another. But I'd rather put the onus on the product owners to ask for that exception, and presumably offer human resources to help the developer update and test their patch. Again, I'm not sure who specifically you're referring to as the bearer of this responsibility. If a team pulls this card, they should have a responsibility to help get the patch relanded in a timely manner. I disagree. If someone breaks Nightly on desktop for example to an extent where it cannot be used for dogfooding, and I back them out to help out our Nightly users and keep the testing product usable so that other regressions can be caught with it, why should I feel responsible for relanding their patch in a timely manner? The someone is the person that wrote the feature that was broken but no tests caught it No, the someone is the person who wrote a patch which survived tests on our infra but broke a product in a way that made it undogfood-able. The specific functionality that they broke may be their own area, someone else's or some ancient piece of code that nobody really owns. I believe the general idea is that as a peer / module owner / product owner, I have the responsibility to write tests that ensure my feature works, and if it is broken by upstream changes that landed because automation didnt find anything wrong with it, then its my responsibility to ensure that tests are written so it doesnt get regressed in the same way again and automation can catch it. Otherwise with no visibility I am putting the reponsibility onto every other upstream developer to hopefully not break my code without any context for them to even know when they have done so. This is summed up in the meme: http://mozillamemes.tumblr.com/post/26210699924/you-reap-what-you-sow Sure. But you're just describing why tests are useful and an absolute necessity. :-) I think what Bobby was asking for is a much stronger ask that is not really attainable. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: treating B2G device tests as tier 1
On Thu, Oct 16, 2014 at 1:01 PM, Ehsan Akhgari ehsan.akhg...@gmail.com wrote: On 2014-10-16, 3:56 PM, Dale Harvey wrote: On 16 October 2014 20:55, Ehsan Akhgari ehsan.akhg...@gmail.com mailto:ehsan.akhg...@gmail.com wrote: On 2014-10-16, 1:52 PM, Bobby Holley wrote: On Thu, Oct 16, 2014 at 7:04 PM, Ehsan Akhgari ehsan.akhg...@gmail.com mailto:ehsan.akhg...@gmail.com mailto:ehsan.akhgari@gmail.__com mailto:ehsan.akhg...@gmail.com wrote: I don't think it's reasonable to assume that the person doing the backout has the time or the expertise to add a test for the broken functionality. Not the sheriff certainly, but I think if the regression is severe enough to warrant this action, the product owners (who are generally the ones who request the backout) can find the resources to make that happen. Who are the product owners exactly? Usually what happens in these cases is some discussion on IRC, followed by trying to ping the author/reviewer, followed by a backout either by a sheriff or another individual such as myself. There will be situations where this is unrealistically difficult for one reason or another. But I'd rather put the onus on the product owners to ask for that exception, and presumably offer human resources to help the developer update and test their patch. Again, I'm not sure who specifically you're referring to as the bearer of this responsibility. If a team pulls this card, they should have a responsibility to help get the patch relanded in a timely manner. I disagree. If someone breaks Nightly on desktop for example to an extent where it cannot be used for dogfooding, and I back them out to help out our Nightly users and keep the testing product usable so that other regressions can be caught with it, why should I feel responsible for relanding their patch in a timely manner? The someone is the person that wrote the feature that was broken but no tests caught it No, the someone is the person who wrote a patch which survived tests on our infra but broke a product in a way that made it undogfood-able. The specific functionality that they broke may be their own area, someone else's or some ancient piece of code that nobody really owns. You're making this about something it's not. This is about undertested b2g apps. Nothing more, nothing less. - Kyle ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: treating B2G device tests as tier 1
On 10/16/14, 4:01 PM, Ehsan Akhgari wrote: Sure. But you're just describing why tests are useful and an absolute necessity. :-) I think what Bobby was asking for is a much stronger ask that is not really attainable. I think what Bobby was actually asking for is this: If a patch lands and is green in continuous integration but then we have some other tests somewhere (hidden, run manually, whatever) that start failing because of this patch, and we deem those failures sufficiently dire to back out the patch, then the owners of these secret-but-critical tests should share some responsibility for enabling the patch to reland. Not least because otherwise it's not clear how to proceed. Certainly that's what _I_ want. -Boris ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: treating B2G device tests as tier 1
On 2014-10-16, 4:20 PM, Boris Zbarsky wrote: On 10/16/14, 4:01 PM, Ehsan Akhgari wrote: Sure. But you're just describing why tests are useful and an absolute necessity. :-) I think what Bobby was asking for is a much stronger ask that is not really attainable. I think what Bobby was actually asking for is this: If a patch lands and is green in continuous integration but then we have some other tests somewhere (hidden, run manually, whatever) that start failing because of this patch, and we deem those failures sufficiently dire to back out the patch, then the owners of these secret-but-critical tests should share some responsibility for enabling the patch to reland. Not least because otherwise it's not clear how to proceed. Certainly that's what _I_ want. I wholeheartedly agree, but I couldn't read between the lines of Bobby's email well enough, apparently! ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
treating B2G device tests as tier 1
Hi, This morning tomcat decided to back bug 982842 and a bunch of dependant bugs out for breaking some of the gaia device tests. As I understand things, this is not the first time something like that has happened. However I think that was a mistake, it treated those tests as tier 1 when they pretty clearly do not meet the requirements in https://wiki.mozilla.org/Sheriffing/Job_Visibility_Policy for tier 1 tests. Those tests aren't even on treeherder / tbpl, much less runnable from try. Also https://developer.mozilla.org/en-US/Firefox_OS/Platform/Automated_testing/Gaia_unit_tests doesn't document how these tests can be run with an emulator or device. The visibility rules exist in part to make sure that tier 1 tests can be easily reproduced and confirmed to work, however these tests don't come close to being easy to run. The best explanation I've heard for this state of afairs is that people want to get these tests to meet the requirements, but given that we have accepted this excuse for very few tests in the past I don't think that's good enough here. So, until someone gets these tests to be visible on treeherder I don't think we should treat them as a pseudo tier 1 test suite. Trev signature.asc Description: Digital signature ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: treating B2G device tests as tier 1
On 10/15/2014 5:15 PM, Trevor Saunders wrote: Hi, This morning tomcat decided to back bug 982842 and a bunch of dependant bugs out for breaking some of the gaia device tests. As I understand things, this is not the first time something like that has happened. However I think that was a mistake, it treated those tests as tier 1 when they pretty clearly do not meet the requirements in https://wiki.mozilla.org/Sheriffing/Job_Visibility_Policy for tier 1 tests. Those tests aren't even on treeherder / tbpl, much less runnable from try. Also https://developer.mozilla.org/en-US/Firefox_OS/Platform/Automated_testing/Gaia_unit_tests doesn't document how these tests can be run with an emulator or device. The visibility rules exist in part to make sure that tier 1 tests can be easily reproduced and confirmed to work, however these tests don't come close to being easy to run. The best explanation I've heard for this state of afairs is that people want to get these tests to meet the requirements, but given that we have accepted this excuse for very few tests in the past I don't think that's good enough here. So, until someone gets these tests to be visible on treeherder I don't think we should treat them as a pseudo tier 1 test suite. Trev Oftentimes, when on-device tests break, there are also real regressions on the phones themselves. At which point, how is it any different from when we backout a patch for nightly bustage that our automation didn't catch? Note that I'm not offering any opinion on how sensible it is that we lack the ability to catch regressions like these in our CI, but I'm not agreeing that backing out was the wrong decision under the circumstances. QA has always had the ability to request backouts for functional regressions regardless of what product they're affecting. -Ryan ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: treating B2G device tests as tier 1
On 15/10/2014 22:15, Trevor Saunders wrote: However I think that was a mistake, it treated those tests as tier 1 when they pretty clearly do not meet the requirements in https://wiki.mozilla.org/Sheriffing/Job_Visibility_Policy ... The visibility rules exist in part to make sure that tier 1 tests can be easily reproduced and confirmed to work Just to add to the points that Ryan made, I should clarify that the visibility policy above doesn't apply here, so any discussion as to whether this case should have resulted in a backout is orthogonal to the policy itself. Whilst the requirements on that page have the added benefit of improving the rigour of our testing the ease by which developers can debug issues that resulted in a backout - they aren't its primary purpose. ie: The policy states the requirements for a job being visible (and thus impacting sheriffs devs day in, day out), not the bar that always has to be met before a backout is performed. Best wishes, Ed ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: treating B2G device tests as tier 1
On Wed, Oct 15, 2014 at 05:36:00PM -0400, Ryan VanderMeulen wrote: On 10/15/2014 5:15 PM, Trevor Saunders wrote: Hi, This morning tomcat decided to back bug 982842 and a bunch of dependant bugs out for breaking some of the gaia device tests. As I understand things, this is not the first time something like that has happened. However I think that was a mistake, it treated those tests as tier 1 when they pretty clearly do not meet the requirements in https://wiki.mozilla.org/Sheriffing/Job_Visibility_Policy for tier 1 tests. Those tests aren't even on treeherder / tbpl, much less runnable from try. Also https://developer.mozilla.org/en-US/Firefox_OS/Platform/Automated_testing/Gaia_unit_tests doesn't document how these tests can be run with an emulator or device. The visibility rules exist in part to make sure that tier 1 tests can be easily reproduced and confirmed to work, however these tests don't come close to being easy to run. The best explanation I've heard for this state of afairs is that people want to get these tests to meet the requirements, but given that we have accepted this excuse for very few tests in the past I don't think that's good enough here. So, until someone gets these tests to be visible on treeherder I don't think we should treat them as a pseudo tier 1 test suite. Trev Oftentimes, when on-device tests break, there are also real regressions on the phones themselves. At which point, how is it any different from when we backout a patch for nightly bustage that our automation didn't catch? For one thing most things we regress but don't catch with tests we fix in place. IME its only when things are horribly broken that we revert things immediately, and there doesn't seem to be evidence for this being more than a regression of some sort. Note that I'm not offering any opinion on how sensible it is that we lack the ability to catch regressions like these in our CI, but I'm not agreeing that backing out was the wrong decision under the circumstances. QA has always had the ability to request backouts for functional regressions regardless of what product they're affecting. I'm not aware of a rule stating that, and I tend to disagree with it assuming it does exist somewhere. As for Ed's point on visibility rules not being primarily about backout that's reasonable, but I'm not aware of a better document, links welcome :) Trev -Ryan ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform signature.asc Description: Digital signature ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: treating B2G device tests as tier 1
On Wed, Oct 15, 2014 at 2:15 PM, Trevor Saunders tbsaunde+mozi...@tbsaunde.org wrote: This morning tomcat decided to back bug 982842 and a bunch of dependant bugs out for breaking some of the gaia device tests. As I understand things, this is not the first time something like that has happened. However I think that was a mistake, it treated those tests as tier 1 when they pretty clearly do not meet the requirements in https://wiki.mozilla.org/Sheriffing/Job_Visibility_Policy for tier 1 tests. Those tests aren't even on treeherder / tbpl, much less runnable from try. Also https://developer.mozilla.org/en-US/Firefox_OS/Platform/Automated_testing/Gaia_unit_tests doesn't document how these tests can be run with an emulator or device. The visibility rules exist in part to make sure that tier 1 tests can be easily reproduced and confirmed to work, however these tests don't come close to being easy to run. The best explanation I've heard for this state of afairs is that people want to get these tests to meet the requirements, but given that we have accepted this excuse for very few tests in the past I don't think that's good enough here. So, until someone gets these tests to be visible on treeherder I don't think we should treat them as a pseudo tier 1 test suite. There's nothing magical about functionality that's tested in our automation. I.e. it's not on average any more or less important that a lot of other functionality that's not tested there. The only thing that's different about things in automation is that it's much easier for us to see when it breaks, and what caused the breakage. But any type of regression is cause for backout. When patches that cause regressions are allowed to sit in the tree that causes lost productivity for a lot of people. It definitely sucks a lot that we don't have automated testing for more stuff in B2G. It's a long slug but it's slowly getting better. This is a huge problem not just for people like you who have a harder time to see if you cause regression. But it's an even bigger problem for B2G developers who are having to work on a code base that is constantly seeing regressions. Regressions that sit in the tree make it dramatically much harder to write and test other patches. It's generally much better to back the offending patch out to allow everyone else to go at full speed. / Jonas ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: treating B2G device tests as tier 1
Jonas Sicking writes: But any type of regression is cause for backout. While I agree regressions are bad, this isn't the usual process. If it were, then I wouldn't bother filing bugs, but merely back out the offending change. There is some kind test for whether the regression costs more than the improvements made, but it comes down to a judgement call from the module owner AIUI. Regressions that sit in the tree make it dramatically much harder to write and test other patches. It's generally much better to back the offending patch out to allow everyone else to go at full speed. Perhaps it is, but this would be quite a change in process. Some kind of policy or guidelines would be helpful, or it could well get out of control. Backouts usually cause regressions too. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: treating B2G device tests as tier 1
On 2014-10-15, 10:19 PM, Karl Tomlinson wrote: Jonas Sicking writes: But any type of regression is cause for backout. While I agree regressions are bad, this isn't the usual process. If it were, then I wouldn't bother filing bugs, but merely back out the offending change. There is some kind test for whether the regression costs more than the improvements made, but it comes down to a judgement call from the module owner AIUI. Regressions that sit in the tree make it dramatically much harder to write and test other patches. It's generally much better to back the offending patch out to allow everyone else to go at full speed. Perhaps it is, but this would be quite a change in process. Some kind of policy or guidelines would be helpful, or it could well get out of control. Backouts usually cause regressions too. In my experience, regressions that break something in a way that makes dogfooding difficult are open to a backout without questions asked policy (but often times in practice we'd try to reach out to the author and the folks who know the area of the code). For other regressions we typically file follow-up bugs. Note that the definition of what makes dogfooding difficult is has not been entirely consistent all the time. Cheers, Ehsan ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
