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

Blake Winton bwinton at mozilla.com
Thu Apr 21 15:21:34 UTC 2011


I guess we're unanimous in our approval, then.  :)

Later,
Blake.

On 11-04-03 21:53 , Dan Mosedale wrote:
 > 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

-- 
Blake Winton   Thunderbird Front End Lead
bwinton at mozilla.com



More information about the tb-planning mailing list