Disabling a few mochitests this week, and upcoming changes next week to browser-chrome

Ed Morley emorley at mozilla.com
Tue Aug 19 16:06:40 UTC 2014


On 19/08/2014 16:46, Gavin Sharp wrote:
> A few issues here:
> - This particular case (we're making broad changes to how the tests
> run that causes many new failures) was not what that policy was meant
> to cover. We need some leeway to handle this situation differently.

Agreed that this case is possibly slightly different - however this 
change is merely exposing already flaky tests - and this change is very 
much overdue. If the tests mentioned are really that important, then 
could we please get people allocated to fixing them? You can't exactly 
say that Joel hasn't given sufficient notification to the lists & yet 
we've had virtually no interested in resolving the issues.

> - This policy should probably clarify that any test disabling patches
> still require module owner/peer review.

The policy does state:
"In the rare case we are disabling the majority of the tests (either at 
once or slowly over time) for a given feature, we need to get the module 
owner to sign off on the current state of the tests."

However it intentionally doesn't state it for smaller cases (I'm not 
including the above in this). Ultimately disabling a test is a last 
resort, and if done according to the policy, will have been performed after:
"In the case we go another 2 days with no response from a module owner, 
we will disable the test."

...in which case either the module owner will ignore the review request 
(which makes the policy pointless, since the whole idea was to set 
expectations and consequences that would actually be enforced), give it 
r- (which isn't acceptable, given they've not acted on the bug in a 
reasonable timeframe), or give it r+ (in which case, great, but this is 
the state we should be in anyway).

Ultimately no one individual test should have the right to destroy the 
SnR for a suite - and if we don't have this policy as a rapid (and 
enforceable in practice) means to control rogue tests, then the only 
other hammer the sheriffs have is to hide entire suites.

Kind regards,

Ed



More information about the firefox-dev mailing list