929447 - Please schedule Android 4 reftests on all trunk trees and make them ride the trains

Status: RESOLVED FIXED

(Release Engineering :: General, defect)

Description

[Dan Minor [:dminor]]

Android 4 reftests are currently enabled on Cedar and Try and are currently green on Cedar. I think it is time to run them on every tree that Android 2.2 reftests are run.

Most of the failures that have shown up since they have been run on Cedar have been non-intermittent and related to already fuzzy tests that need to be fuzzier (see bug 927085, bug 921021, bug 900542).

With the low volume of commits to Cedar it is difficult to tell if these new failures are due to changes to rendering/layout code, or if a bug is causing changes to rendering results over time.

If developers are causing these tests to fail, it is better that they investigate (and tweak the manifests if necessary), rather than me doing it well after the fact. If there is a rendering bug, this should uncover it quickly, and we can hide and/or disable the reftests again while it is investigated.

Comment 1

[Kim Moir [:kmoir] ET]

Attachment: bug929447.patch

Comment 2

[Justin Wood (:Callek)]

Comment on attachment 820336 [details] [diff] [review]
bug929447.patch

Review of attachment 820336 [details] [diff] [review]:
-----------------------------------------------------------------

r- due to IRC convo about wanting this to ride trains.

Comment 3

[Kim Moir [:kmoir] ET]

So gecko_version isn't defined for mobile versions besides the esrs (see mobile_config.py starting at line 32).  What should it be for each branch?

Comment 4

[Kim Moir [:kmoir] ET]

from sfink

kmoir: when gecko_version is not defined, it's considered to be later than any version that *is* defined. In other words, the default means "most recent", and any explicit setting means "locked to older version N".

Comment 5

[Kim Moir [:kmoir] ET]

More info

sfink	kmoir: when gecko_version is not defined, it's considered to be later than any version that *is* defined. In other words, the default means "most recent", and any explicit setting means "locked to older version N".
kmoir	sfink: thanks. My code was not finding any branches that didn't have it defined, I'm working fixing that	
kmoir: yeah, the usual pattern is 1. set something = current_behavior; 2. if version < K, set something = older_behavior
sfink	kmoir: but there are several that also do if version is at least K, set something = new_behavior
kmoir	okay
sfink	kmoir: the first is done with items_before, the second with items_at_least
sfink	I really should document it better, especially considering how long it took me to get my head around it

Comment 6

[Kim Moir [:kmoir] ET]

Attachment: bug929447.patch

tested in staging

Comment 7

[Justin Wood (:Callek)]

Comment on attachment 821854 [details] [diff] [review]
bug929447.patch

are you sure this is the tested patch?

looks like this one removes it from everything !== cedar/try (and less than gecko 26)

Comment 8

[Kim Moir [:kmoir] ET]

Right that's what we want to do.  

Don't run these tests on branches where gecko < 26 and in that case only run on cedar and try.  

Do run the tests for all other branches where gecko >= 26

http://dev-master01.build.scl1.mozilla.com:8036/builders

Thus with this patch they will run on m-c and m-i and some other project branches, today they don't run there.

Comment 9

[Justin Wood (:Callek)]

Comment on attachment 821854 [details] [diff] [review]
bug929447.patch

Review of attachment 821854 [details] [diff] [review]:
-----------------------------------------------------------------

ahhh right, for some reason I was thinking of items_before the wrong way around and confusing myself.

::: mozilla-tests/mobile_config.py
@@ +1142,2 @@
>      if branch in ('cedar', 'try'):
>          continue

remove |if branch in... continue| given the new logic won't need that anymore

Comment 10

[Kim Moir [:kmoir] ET]

Attachment: bug929447-2.patch

removed if branch line continue

Comment 11

[Justin Wood (:Callek)]

something here is in production

Comment 12

[Dan Minor [:dminor]]

"Every" tree was a bad idea for which I take full responsibility. Once I get the reftests green again (bug 930985) we should turn them on just for inbound and mozilla-central.

Comment 13

[Kim Moir [:kmoir] ET]

Comment on attachment 821905 [details] [diff] [review]
bug929447-2.patch

dminor asked me to revert this because tests are not green again

Comment 14

[Kim Moir [:kmoir] ET]

reverted in production via reconfig

Comment 15

[Phil Ringnalda (:philor)]

And what he means by "inbound and mozilla-central" is absolutely not that thing which nobody ever means even when they say it, he means "for all trunk trees, and then make them ride the trains after that."

Comment 16

[Dan Minor [:dminor]]

Thanks Philor!

Comment 17

[Dan Minor [:dminor]]

Latest run on Cedar today was green: https://tbpl.mozilla.org/?tree=Cedar&showall=1&rev=9aa828994230, so I think this can go forward.

Once these are scheduled, I'll watch the results and update the manifests as necessary.

Comment 18

[Dan Minor [:dminor]]

Looks like we have one unexpected result on Cedar as of last Friday. I'll file a bug to fix.

Callek, will you have time to look at this this quarter?

Comment 19

[Justin Wood (:Callek)]

(In reply to Dan Minor [:dminor] from comment #18)
> Looks like we have one unexpected result on Cedar as of last Friday. I'll
> file a bug to fix.
> 
> Callek, will you have time to look at this this quarter?

I *think* so (note: "This Quarter" means effectively "in the next 2 weeks", given the end-of-month vaca)

Comment 20

[Dan Minor [:dminor]]

Attachment: Schedule panda reftests to ride the trains

Callek, mcote asked me to put together a patch for you to take a look at.

I removed the current code that schedules on Cedar and Try and replaced it with code based on what was done recently for cpp unittests.

Comment 21

[Justin Wood (:Callek)]

Comment on attachment 8349585 [details] [diff] [review]
Schedule panda reftests to ride the trains

Review of attachment 8349585 [details] [diff] [review]:
-----------------------------------------------------------------

this should do what we want

Comment 22

[Dan Minor [:dminor]]

Thanks, things are looking green again on Cedar, so pushed to: https://hg.mozilla.org/build/buildbot-configs/rev/2d4437269255

Comment 23

[[e-mail] (:bhearsum)]

in production