proposal for blanket authorized orange fixes for mail & mailnews meeting specific constraints

Dan Mosedale dmose at mozilla.org
Mon Apr 4 01:53:30 UTC 2011


Agreed; sounds good to me as well.

On 4/1/11 3:06 PM, David Bienvenu wrote:
> This all sounds good to me.
>
> - David
>
> On 3/31/2011 6:58 PM, Andrew Sutherland wrote:
>> I would propose that we formally introduce something like 
>> rs=simple-orange-fix for orange fixes in mail/ and mailnews/ that 
>> meet the following constraints:
>>
>> 1) The patch is fixing an intermittent orange test failure.
>> 2) The patch is a change exclusively to test logic.
>> 3) The patch has identified a likely problem and fixes it; no 
>> commenting out tests/changing what a test tests/adding huge 
>> timeouts.  (You could still ask a reviewer to approve such things, 
>> though.)
>> 4) The patch does not change public test frameworks, specifically, 
>> nothing in mailnews/test/resources/ or 
>> mail/test/mozmill/shared-modules/.
>> 5) For mozmill tests, the patch has passed on the try server.  For 
>> xpcshell tests, it has either passed on try or has been run locally 
>> on the appropriate platform(s).  Specifically, if the xpcshell test 
>> will do different things on Windows than it will do on OS X or Linux, 
>> it must be run on Windows and one of OS X/Linux.
>> 6) The author of the patch is the module owner or a peer of the 
>> module to which the test corresponds.
>> 7) A bug is still required, even if it is immediately marked fixed.
>>
>> I would additionally propose a similar thing called 
>> rs=orange-debugging which has the following constraints:
>> 1) Is trying to fix an intermittent orange test failure.
>> 2) Only touches test logic.
>> 3) Only adds/modifies logging statements, although the logs can be 
>> conditional.
>> 4) Must have been run at least once on the try server.  (I have done 
>> some mozmill orange fixing where the try server would not duplicate 
>> the orange failures of the trunk and so in order to get the enhanced 
>> logging, the changes need to go into the trunk.)
>> 5) Patch is by module owner/peer for the affected test.
>> 6) There must be an associated bug tracking the orange bug to be fixed.
>>
>> The basic idea is that we want to make/keep the bar low to fix 
>> intermittent oranges.  In many cases, it may not even be obvious what 
>> is happening, so being able to get additional logging into the tree 
>> easily is useful.  The rationale for avoiding the review step is that 
>> in many cases the exact nuances of the failure may be complex and 
>> there's not a lot of benefit for a reviewer to either perform a 
>> cursory review or spend a lot of time getting up to speed on what the 
>> failure.  (And the idea is that silly errors like typos will get 
>> weeded out by having run the tests locally and/or on the try server.)
>>
>> For example, I just (hopefully) fixed a gloda intermittent orange 
>> test failure bug whose failure was an artifact of the way the test 
>> was attempting to approximate the sequence of [user triggers 
>> compaction, user quits before compaction is processed, indexing sweep 
>> at next startup completes compaction processing].  While it would be 
>> awesome if there was someone who was sufficiently up-to-speed on the 
>> gloda internals to be able to easily know if I was screwing up with 
>> my fix or fixing it the wrong way, it would take a lot of research 
>> for them to get up-to-speed enough to do that, and that effort would 
>> be better spent by learning about gloda by fixing low-hanging bugs.
>>
>> We would document these on 
>> https://developer.mozilla.org/en/Mailnews_and_Mail_code_review_requirements 
>> assuming we're all good with the changes.
>>
>> Andrew
>> _______________________________________________
>> tb-planning mailing list
>> tb-planning at mozilla.org
>> https://mail.mozilla.org/listinfo/tb-planning
>
> _______________________________________________
> tb-planning mailing list
> tb-planning at mozilla.org
> https://mail.mozilla.org/listinfo/tb-planning



More information about the tb-planning mailing list