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

Ehsan Akhgari ehsan.akhgari at gmail.com
Fri Apr 1 02:09:28 UTC 2011


I think this is a very good suggestion.  FWIW, on mozilla-central, we
have a policy that patches which only touch parts of a test (or
otherwise only touch things which are not included in the final binary
builds shipped to users) are not part of the build (NPOTB), and can
always land, regardless of approval requirements and other
restrictions put on mozilla-central.  The only exception to that is
when the tree is CLOSED because of something going terribly wrong (for
example, a build failure).

Cheers,
--
Ehsan
<http://ehsanakhgari.org/>



On Thu, Mar 31, 2011 at 9:58 PM, Andrew Sutherland
<asutherland at asutherland.org> 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