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

David Bienvenu bienvenu at davidbienvenu.org
Fri Apr 1 22:06:49 UTC 2011


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




More information about the tb-planning mailing list