835955 - Turn off tests running on fedora32/64 that are consistently green on ubuntu32/64 (crashtest, jsreftest, jetpack and marionette)

Status: RESOLVED FIXED

(Release Engineering :: General, defect)

Description

[Chris AtLee [:catlee]]

Currently the following tests are green on ubuntu32/64:
mochitest-2,3,4
mochitest-other
crashtest
jsreftest
marionette debug

With the exception of the chunked mochitests, let's stop running these on fedora32/64.

New tests that are green on ubuntu test platforms should be migrated when they're ready.

Comment 1

[John O'Duinn [:joduinn] (please use "needinfo?" flag)]

(In reply to Chris AtLee [:catlee] from comment #0)
> Currently the following tests are green on ubuntu32/64:
> mochitest-2,3,4
> mochitest-other
> crashtest
> jsreftest
> marionette debug
> 
> With the exception of the chunked mochitests, let's stop running these on
> fedora32/64.
from irc, but adding here to be explicit:

This bug will only disable fedora/fedora64 tests on mozilla-central/mozilla-inbound/{all-project-branches} and try. This is where we have most of our load, so we should quickly see improved wait times with this. 

Specifically, m-a/m-b/m-r/m-esr will eventually get this change as this change rides the trains - they are only a combined total of <10% of our load, so I only expect a minor incremental improvement with those branches.


This change does not need a downtime.

Comment 2

[Phil Ringnalda (:philor)]

Jetpack on trunk branches is safe to switch; hard to say about jetpack on the Jetpack tree without trying it, since that runs against things down to mozilla-release, though the accidental runs of jetpack (and everything else) against mozilla-aurora on the Birch tree are doing fine.

Comment 3

[Rail Aliiev [:rail]]

Attachment: Move green tests to Ubuntu

I'll comment the patch inline.

Comment 4

[Rail Aliiev [:rail]]

Comment on attachment 708108 [details] [diff] [review]
Move green tests to Ubuntu

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

::: mozilla-tests/config.py
@@ +551,5 @@
>              addSuite(suiteGroupName=suiteToAdd[3], newSuiteName=suiteToAdd[4],
>                       suiteList=BRANCHES[branch]['platforms'][suiteToAdd[0]][suiteToAdd[1]][type])
>  
> +
> +def key_in_dict(dictionary, *keys):

This function allows querying a dictionary in depth. So instead of

if 'linux64' in  BRANCHES[branch]['platforms'] and 'ubuntu64' in BRANCHES[branch]['platforms']['linux64'] and 'debug_unittest_suites' in BRANCHES[branch]['platforms']['linux64']['ubuntu64']:

or 

try:
    BRANCHES[branch]['platforms']['linux64']['ubuntu64']['debug_unittest_suites']
except KeyError:
    do eet


you would just use

if key_in_dict(BRANCHES[branch]['platforms'], 'linux64', 'ubuntu64', 'debug_unittest_suites'):
  do not eet

@@ +784,2 @@
>              ],
> +            'debug_unittest_suites': UNITTEST_SUITES['debug_unittest_suites'][:],

The idea is to have Ubuntu tests list to be the same as fedora at the beginning. Then we need to remove green Ubuntu suites from fedora and leave only green suites on Ubuntu. Cedar is an exception, we need everything running there to debug.

@@ -807,3 @@
>          },
>      },
> -    'ubuntu64': {

This is safe to remove. A noop (tested) operation. This platform somehow sneaked here instead of sneaking under linux64....

@@ +1231,5 @@
> +# Green tests, including mozharness based ones
> +# Tests listed as Ubuntu tests won't be enabled on Fedora
> +UBUNTU_OPT_UNITTEST = ["crashtest", "jsreftest", "jetpack"]
> +UBUNTU_DEBUG_UNITTEST = ["crashtest", "jsreftest", "jetpack", "marionette"]
> +

the rest should be straightforward

Comment 5

[Phil Ringnalda (:philor)]

Last night, we backed out a WebRTC patch for leaking on Fedora64 debug mochitest-3. Not Fedora64 and Ubuntu64, just Fedora64, so we need to understand what the Ubuntu VMs are failing to test about WebRTC before we switch over any suite that might wind up with any of its tests.

Comment 6

[Rail Aliiev [:rail]]

All mochitest chunks will be running on Fedora for now, since we don't want to hit an issue when the same test may be in different chunks on different branches.

Comment 7

[Chris AtLee [:catlee]]

Comment on attachment 708108 [details] [diff] [review]
Move green tests to Ubuntu

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

::: mozilla-tests/config.py
@@ +560,5 @@
> +        key, keys = keys[0], keys[1:]
> +        if key in dictionary:
> +            return key_in_dict(dictionary[key], *keys)
> +        else:
> +            return False

looks good...I think a more descriptive name would be good though. something like nested_haskey, or something to indicate this is different than a simple "key in dict" check.

@@ +1226,5 @@
>  
>  # MERGE DAY NOTE: remove v21 based branches from the list below
>  NON_UBUNTU_BRANCHES = ("mozilla-aurora", "mozilla-beta", "mozilla-release",
>                         "mozilla-esr10", "mozilla-esr17", "mozilla-b2g18",
>                         "mozilla-b2g18_v1_0_0")

add mozilla-b2g18_v1_0_1 here too

@@ +1290,5 @@
> +                elif key_in_dict(BRANCHES[branch]['platforms'], 'linux', 'fedora', 'opt_unittest_suites'):
> +                    try:
> +                        BRANCHES[branch]['platforms']['linux']['fedora']['opt_unittest_suites'].remove(suite)
> +                    except KeyError:
> +                        pass

all for of these are doing essentially the same thing, right? can you refactor them so you do something like

for x in [('linux', 'ubuntu32', 'fedora'), ('linux64', 'ubuntu64', 'fedora64')]:
   for y in [('opt_unittest_suites', UBUNTU_OPT_UNITTEST), ('debug_unittest_suites', UBUNTU_DEBUG_UNITTEST)]:
      ...

Comment 8

[Rail Aliiev [:rail]]

Attachment: Move green tests to Ubuntu v2

(In reply to Chris AtLee [:catlee] from comment #7)
> 
> looks good...I think a more descriptive name would be good though. something
> like nested_haskey, or something to indicate this is different than a simple
> "key in dict" check.

Makes sense. Done.
 
> add mozilla-b2g18_v1_0_1 here too

It is not in BRANCHES, but won't hurt the code. Added.
 
> all for of these are doing essentially the same thing, right? can you
> refactor them so you do something like
> 
> for x in [('linux', 'ubuntu32', 'fedora'), ('linux64', 'ubuntu64',
> 'fedora64')]:
>    for y in [('opt_unittest_suites', UBUNTU_OPT_UNITTEST),
> ('debug_unittest_suites', UBUNTU_DEBUG_UNITTEST)]:
>       ...

Yeah, I thought about that and being verbose. The new version looks readable though.

Comment 9

[Chris AtLee [:catlee]]

Comment on attachment 708325 [details] [diff] [review]
Move green tests to Ubuntu v2

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

::: mozilla-tests/config.py
@@ +1243,5 @@
> +            del BRANCHES[branch]['platforms']['linux64']['ubuntu64']
> +            BRANCHES[branch]['platforms']['linux64']['slave_platforms'] = ['fedora64']
> +        if 'linux' in BRANCHES[branch]['platforms']:
> +            del BRANCHES[branch]['platforms']['linux']['ubuntu32']
> +            BRANCHES[branch]['platforms']['linux']['slave_platforms'] = ['fedora']

nit: you could remove one level of indentation below by putting a 'continue' here and removing 'else:'

Comment 10

[Rail Aliiev [:rail]]

Comment on attachment 708325 [details] [diff] [review]
Move green tests to Ubuntu v2

http://hg.mozilla.org/build/buildbot-configs/rev/dfd7093cfe52

Comment 11

[Ed Morley [:emorley]]

Even given comment 6, are we confident enough we're not going to miss failures due to comment 5 (in the non chunked mochitest suites)?

Comment 12

[Rail Aliiev [:rail]]

(In reply to Ed Morley [:edmorley UTC+0] from comment #11)
> Even given comment 6, are we confident enough we're not going to miss
> failures due to comment 5 (in the non chunked mochitest suites)?

We're enabling the following suites only:
crashtest, jsreftest, jetpack and marionette

mochitest is not ready yet.

Comment 13

[Ed Morley [:emorley]]

Ok, to be more explicit: 
We've already had one example of a VM run suite not showing a failure that occurred on normal hardware. Whilst the suite is different from those transitioned so far, should we not investigate further, before we put even a subset of our linux eggs in the one proverbial basket?

Comment 14

[cmtalbert]

Henrik and Jason, can y'all take a look at why ubuntu 64 VMs didn't capture this leak?  See https://bugzilla.mozilla.org/show_bug.cgi?id=835955#c5 (comment 5 above)

The releng guys can hook y'all up with a VM for testing as well as a fedora 64 machine from the slave pool so that you're working in the same environment the tests run in.

My money is on the outdated libraries at play in the old fedora 64 slaves, but it would be good to verify that and ensure that the webRTC tests running in Ubuntu 64 VMs are actually running properly (they could not be leaking because they aren't running).

Thanks for digging into this.

Comment 15

[Ed Morley [:emorley]]

(In reply to Clint Talbert ( :ctalbert ) from comment #14)
> Henrik and Jason, can y'all take a look at why ubuntu 64 VMs didn't capture
> this leak?  See https://bugzilla.mozilla.org/show_bug.cgi?id=835955#c5
> (comment 5 above)

To save some time, the leak log (and backout commit) comment 5 is referring to can be found here:
https://bugzilla.mozilla.org/show_bug.cgi?id=818670#c25

Comment 16

[Rail Aliiev [:rail]]

In production

Comment 17

[Henrik Skupin [:whimboo][⌚️UTC+1]]

In regards to comment 14 and the fixed state of the bug I assume no further work is necessary here?

Comment 18

[Ed Morley [:emorley]]

Comment 14 is still needed; this bug needs to either be reopened or a dependant filed.

Comment 19

[Rail Aliiev [:rail]]

(In reply to Ed Morley [:edmorley UTC+0] from comment #18)
> Comment 14 is still needed; this bug needs to either be reopened or a
> dependant filed.

I filed bug 837017 to track enabling remaining tests.

Comment 20

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to Phil Ringnalda (:philor) from comment #5)
> Last night, we backed out a WebRTC patch for leaking on Fedora64 debug
> mochitest-3. Not Fedora64 and Ubuntu64, just Fedora64, so we need to
> understand what the Ubuntu VMs are failing to test about WebRTC before we
> switch over any suite that might wind up with any of its tests.

Phil, do you have a reference to it? It might help us to setup the right environment for testing.

(In reply to Rail Aliiev [:rail] from comment #19)
> I filed bug 837017 to track enabling remaining tests.

So that's for Ubuntu but as Clint mentioned we have to check for a leak on the Fedora machines. Do we need another follow-up bug?

Comment 21

[Ed Morley [:emorley]]

(In reply to Rail Aliiev [:rail] from comment #19)
> I filed bug 837017 to track enabling remaining tests.

That's not what comment 17 / comment 18 are referring to.

I don't believe we should have switched *any* test suites (ie not landed the bug we're in now) until we figure out what went wrong. Whereas bug 837017 is about switching on the rest of the test suites.

(In reply to Henrik Skupin (:whimboo) from comment #20)
> Phil, do you have a reference to it? It might help us to setup the right
> environment for testing.

See comment 15.

Comment 22

[Rail Aliiev [:rail]]

I'm not sure what kind of information is needed from me, so cleaning up the needinfo? flag. Feel free to reopen the bug if you believe that we have something left here.