791389 - Split mochitest-browser-chrome out of mochitest-other

Status: RESOLVED FIXED

(Release Engineering :: General, defect, P3)

Description

[(no longer active)]

Debug Moth unit tests take more than an hour.  Opt ones take around 40 minutes.  This hurts end to end times, so I think we should split them.

Comment 1

[Ed Morley [:emorley]]

WinXP debug m-oth now takes 2 hours. 

This/even the 1 hour runtime on other platforms causes issues for sheriffs since:
1) Turnaround time for retriggers is painful when bisecting (and wasteful, since most of the time we don't need to retrigger everything).
2) Log size for m-oth is excessive, causing TBPL to timeout more frequently when fetching/parsing logs.
3) Even in non-failure/bisection cases, increases end-to-end time when not at max infra load, due to not running the m-oth subsuites in parallel.

As such, please can we bump the priority on this? :-)

Comment 2

[Ed Morley [:emorley]]

Filed bug 794420 to investigate the WinXP increase.

Comment 3

[[e-mail] (:bhearsum)]

If someone (from RelEng or otherwise) wants to take a stab at this, here's a rough outline of what needs to be done:
* Adjust suite names in buildapi: https://mxr.mozilla.org/build-central/source/buildapi/buildapi/model/testruns.py#76
* Adjust suites names for a11y project branch: https://mxr.mozilla.org/build-central/source/buildbot-configs/mozilla/project_branches.py#22
* Split up mochitest-other into two suites in mozilla-tests/config.py: https://mxr.mozilla.org/build-central/source/buildbot-configs/mozilla-tests/config.py#380 and #392
* Split up mochitest-other in mozilla/config.py: https://mxr.mozilla.org/build-central/source/buildbot-configs/mozilla/config.py#45
* Update try parser unit tests: https://mxr.mozilla.org/build-central/source/buildbotcustom/test/test_try_parser.py#13
* Update the try chooser webpage: https://wiki.mozilla.org/ReleaseEngineering/How_To/Update_the_Try_Syntax

Comment 4

[Ed Morley [:emorley]]

That's useful, thank you :-)

So what do we think?
mochitest-browser-chrome
and
mochitest-other: mochitest-chrome, mochitest-a11y, mochitest-ipcplugins

As a point of reference...

WinXP debug:
mochitest-chrome: 39 mins, 13 secs
mochitest-browser-chrome: 1 hrs, 42 secs
mochitest-ally: 5 mins, 40 secs
mochitest-ipcplugins: 1 mins, 8 secs

OS X 10.6 opt:
mochitest-chrome: 10 mins, 5 secs
mochitest-browser-chrome: 20 mins, 46 secs
mochitest-ally: <not run on most repos>
mochitest-ipcplugins: 24 secs

Comment 5

[(no longer active)]

(In reply to comment #4)
> So what do we think?
> mochitest-browser-chrome
> and
> mochitest-other: mochitest-chrome, mochitest-a11y, mochitest-ipcplugins

Sounds reasonable.

Comment 6

[Ed Morley [:emorley]]

In addition to the list of changes in comment 3, we'll also need to update:
* OrangeFactor: 
  - https://hg.mozilla.org/automation/orangefactor/file/c492511a8bb6/html/scripts/woo.utils.js#l21
* Possibly TBPL, though we had 'b-c' support for mobile when it used to run it (the scattered regex in this file is sadfaces making :-/): 
  - https://hg.mozilla.org/users/mstange_themasta.com/tinderboxpushlog/file/9c7f496beb97/js/Data.js#l724
  - https://hg.mozilla.org/users/mstange_themasta.com/tinderboxpushlog/file/9c7f496beb97/js/Data.js#l748
* logparser: 
  - https://hg.mozilla.org/automation/logparser/file/025da2685aeb/logparser/config.py#l12
  - https://hg.mozilla.org/automation/logparser/file/025da2685aeb/logparser/scraper.py#l70

Comment 7

[Ed Morley [:emorley]]

Attachment: buildbotcustom v1

Not sure if "mochitest-bc" or "mochitest-b-c" is preferred for trychooser, I went with the former.

Comment 8

[Ed Morley [:emorley]]

Attachment: buildbot-configs v1

In addition to the changes listed in comment 3; mozharness_unittest_suites needed adjusting.

Comment 9

[Ed Morley [:emorley]]

Attachment: buildapi v1

Comment 10

[Ed Morley [:emorley]]

On closer inspection, TBPL doesn't need updating since the existing 'b-c' entry (back from when mobile used to use it), will cover us:
https://hg.mozilla.org/users/mstange_themasta.com/tinderboxpushlog/file/9c7f496beb97/js/Data.js#l748

I've filed dependant bugs for anything that can be landed pre-emptively (trychooser, orangefactor, logparser, autotools), to reduce noise in this bug & reduce the number of things we have to land simultaneously.

All of the other necessary changes are made by the patches in this bug. I believe that covers everything :-)

Comment 11

[[e-mail] (:bhearsum)]

Comment on attachment 668976 [details] [diff] [review]
buildbotcustom v1

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

https://hg.mozilla.org/build/tools/file/f302af7cb4b5/trychooser/index.html will need to be updated too, to update the syntax builder.

::: try_parser.py
@@ +19,5 @@
>          elif u == 'mochitest-o':
>              for v in valid_suites:
>                  if re.search(u,v):
>                      test_suites.append(v)
> +        elif u == 'mochitest-bc':

Shouldn't this be "mochitest-b-c" since that's what TBPL uses?

Comment 12

[Ed Morley [:emorley]]

(In reply to Ben Hearsum [:bhearsum] from comment #11)
> Comment on attachment 668976 [details] [diff] [review]
> buildbotcustom v1
>
> > +        elif u == 'mochitest-bc':
> 
> Shouldn't this be "mochitest-b-c" since that's what TBPL uses?

I contemplated that, but I think I prefer 'bc' and so as a follow up to this bug will be switching TBPL to use that instead of 'b-c' (if anything it takes up less space on every platform row on TBPL).

Comment 13

[Chris AtLee [:catlee]]

Has anybody thought of what impact this has (if any) to release branches?

Comment 14

[Ed Morley [:emorley]]

(In reply to Chris AtLee [:catlee] from comment #13)
> Has anybody thought of what impact this has (if any) to release branches?

This should split them on all surely? However I don't see that this should cause any problems  :-)
(I was also going to wait until the chemspill dust had settled)

Comment 15

[Ed Morley [:emorley]]

bhearsum, don't suppose I can get a review on the other patch? :-)

Comment 16

[[e-mail] (:bhearsum)]

(In reply to Ed Morley [:edmorley UTC+1] from comment #14)
> (In reply to Chris AtLee [:catlee] from comment #13)
> > Has anybody thought of what impact this has (if any) to release branches?
> 
> This should split them on all surely? However I don't see that this should
> cause any problems  :-)
> (I was also going to wait until the chemspill dust had settled)

We've had problems in the past where changing the ordering of test suites has altered results. However, I'm not super concerned about that. Let's go ahead with this and back out if it causes problems.

Comment 17

[Ed Morley [:emorley]]

Thank you for the reviews :-)

https://hg.mozilla.org/build/buildbotcustom/rev/43f91581b61a
https://hg.mozilla.org/build/buildbot-configs/rev/0200815fcf3e
https://hg.mozilla.org/build/buildapi/rev/09c834b65f93

Comment 18

[Ed Morley [:emorley]]

In production & looking good :-)

https://tbpl.mozilla.org/?rev=cf1bbed46731&jobname=browser-chrome

Comment 19

[[e-mail] (:bhearsum)]

Nice work here Ed!