Closed Bug 929447 Opened 7 years ago Closed 7 years ago

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

Categories

(Release Engineering :: General, defect)

x86_64
Linux
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: dminor, Assigned: dminor)

References

Details

Attachments

(1 file, 3 obsolete files)

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.
Attached patch bug929447.patch (obsolete) — Splinter Review
Attachment #820336 - Flags: review?(bugspam.Callek)
Assignee: nobody → kmoir
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.
Attachment #820336 - Flags: review?(bugspam.Callek) → review-
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?
Flags: needinfo?(bugspam.Callek)
Flags: needinfo?(bugspam.Callek) → needinfo?(bhearsum)
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".
Flags: needinfo?(bhearsum)
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
Attached patch bug929447.patch (obsolete) — Splinter Review
tested in staging
Attachment #820336 - Attachment is obsolete: true
Attachment #821854 - Flags: review?(bugspam.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)
Attachment #821854 - Flags: review?(bugspam.Callek) → review-
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 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
Attachment #821854 - Flags: review- → review+
Attached patch bug929447-2.patch (obsolete) — Splinter Review
removed if branch line continue
Attachment #821905 - Flags: checked-in+
Depends on: 930985
something here is in production
"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.
Summary: Please schedule Android 4 reftests on every tree that Android 2.2 reftests run → Please schedule Android 4 reftests on m-c and inbound
Comment on attachment 821905 [details] [diff] [review]
bug929447-2.patch

dminor asked me to revert this because tests are not green again
Attachment #821905 - Flags: checked-in+ → checked-in-
reverted in production via reconfig
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."
Summary: Please schedule Android 4 reftests on m-c and inbound → Please schedule Android 4 reftests on all trunk trees and make them ride the trains
Thanks Philor!
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.
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?
Flags: needinfo?(bugspam.Callek)
Depends on: 945273
(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)
Assignee: kmoir → bugspam.Callek
Flags: needinfo?(bugspam.Callek)
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.
Attachment #821854 - Attachment is obsolete: true
Attachment #821905 - Attachment is obsolete: true
Attachment #8349585 - Flags: review?(bugspam.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
Attachment #8349585 - Flags: review?(bugspam.Callek) → review+
Assignee: bugspam.Callek → dminor
Depends on: 957720
Thanks, things are looking green again on Cedar, so pushed to: https://hg.mozilla.org/build/buildbot-configs/rev/2d4437269255
in production
Depends on: 961575
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Component: General Automation → General
You need to log in before you can comment on or make changes to this bug.