984930 - Create mochitest-dt and move all mochitests under browser/devtools into that suite

Status: RESOLVED FIXED

(Testing :: Mochitest, defect)

Description

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Due to us hitting the global timeout in mochitest-bc this is the perfect time for us to move our devtools mochitests into a new test suite.

Can we create a new test suite mochitest-dt and move all mochitests from browser/devtools from mochitest-bc to mochitest-dt?

Comment 1

[(not currently active) Ted Mielczarek]

This is going to require two bits of work:
1) Define the suite in some way. Are we going to use separate manifests for the tests? Are we simply going to have the test harness filter out all the tests under browser/devtools for a normal browser-chrome run? However you choose to do this, you'll need to add some way to invoke this separate "mochitest-dt" suite in the Mochitest harness:
http://mxr.mozilla.org/mozilla-central/source/testing/mochitest/runtests.py
(you can look for "ipcplugins" for something similar we do already)
You'll also want to add a mach command to run this test suite:
http://mxr.mozilla.org/mozilla-central/source/testing/mochitest/mach_commands.py#580

2) Once you have that working, get the mozharness+buildbot configuration changes necessary to enable a new suite. That's all in Release Engineering's domain, you might want to file a bug on them right now to get that process started.

Comment 2

[Joel Maher ( :jmaher ) (UTC -8)]

can we agree on the ideal method for splitting these out via the harness/manifests?

Comment 3

[Dave Townsend [:mossop]]

Rather than splitting out devtools and waiting till something else makes bc last too long should we just split bc into slices like plain mochitest already does?

Comment 4

[(not currently active) Ted Mielczarek]

That's bug 819963. Turns out we have a lot of test interdependencies. jmaher has been working on those. philor has expressed interest in moving the devtools tests into their own chunk for a while, since they apparently have a bad track record of being flaky.

Comment 5

[Joel Maher ( :jmaher ) (UTC -8)]

:mossop, I have looked into this and I would need to disable 70 tests to make that happen more reliably.  The majority of the tests are in devtools :(

I haven't done the work because I need to file 70 bugs and deal with all the coordination required to either fix the bug (get more logs, etc.) which requires a lot of randomization and waiting vs just disabling everything.  Most developers are more apt to attempt to fix it first prior to a test being disabled.

If there is support for mass disabling and then filing the bugs after the fact, I could prioritize my work on that and make browser-chrome support chunks in the next week or two!

Comment 6

[Gregory Szorc [:gps]]

The technical debt in the reliability of browser chrome tests seems to been spilling over into automation more frequently lately. I think these issues should be raised to [e-mail] and possibly raised at the next Firefox team meeting and with Firefox team management. CC'ing a few managers.

Comment 7

[Dave Townsend [:mossop]]

(In reply to Gregory Szorc [:gps] from comment #6)
> The technical debt in the reliability of browser chrome tests seems to been
> spilling over into automation more frequently lately. I think these issues
> should be raised to [e-mail] and possibly raised at the next
> Firefox team meeting and with Firefox team management. CC'ing a few managers.

Please do, I've not heard anything about this before.

Comment 8

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

(In reply to Ted Mielczarek [:ted.mielczarek] from comment #1)
> This is going to require two bits of work:
> 1) Define the suite in some way. Are we going to use separate manifests for
> the tests? Are we simply going to have the test harness filter out all the
> tests under browser/devtools for a normal browser-chrome run?

I think it is best if the harness filters out all browser-chrome tests under browser/devtools for a normal browser-chrome run?

> However you
> choose to do this, you'll need to add some way to invoke this separate
> "mochitest-dt" suite in the Mochitest harness:
> http://mxr.mozilla.org/mozilla-central/source/testing/mochitest/runtests.py
> (you can look for "ipcplugins" for something similar we do already)

Yup, planning on it.

> You'll also want to add a mach command to run this test suite:
> http://mxr.mozilla.org/mozilla-central/source/testing/mochitest/
> mach_commands.py#580
> 

You mean as opposed to ./mach mochitest-browser browser/devtools ?

We could use ./mach devtools-test but these are still browser-chrome tests.

> 2) Once you have that working, get the mozharness+buildbot configuration
> changes necessary to enable a new suite. That's all in Release Engineering's
> domain, you might want to file a bug on them right now to get that process
> started.

Bug 985350 logged.

Comment 9

[(not currently active) Ted Mielczarek]

(In reply to Michael Ratcliffe [:miker] [:mratcliffe] from comment #8)
> > You'll also want to add a mach command to run this test suite:
> > http://mxr.mozilla.org/mozilla-central/source/testing/mochitest/
> > mach_commands.py#580
> > 
> 
> You mean as opposed to ./mach mochitest-browser browser/devtools ?
> 
> We could use ./mach devtools-test but these are still browser-chrome tests.

If you're naming the test something different on tinderbox, we need at least an alias to run it via mach. We have aliases in the "mach test" command, but I don't think they support an alias that evaluates to a subdirectory currently:
http://mxr.mozilla.org/mozilla-central/source/testing/mach_commands.py#44

FWIW, jmaher and I were discussing a possible generic way to implement this yesterday, by adding some sort of "subsuite" key to the manifest files, like:

[DEFAULT]
subsuite = devtools

and then requiring an invocation like "mach mochitest-browser --subsuite=devtools" (which you could obviously alias to "mach mochitest-devtools" or something).

Comment 10

[Joel Maher ( :jmaher ) (UTC -8)]

the work for this would include:
* annotate manifests with subsuite keyword (just add it to the default section of all browser.ini in devtools folder)
* add cli option for mochitest for --subsuite
* modify runtests.py mochitest harness to ignore all tests that have a subsuite defined unless one is passed in, then match on it (note: leave the flag to ignore subsuite stuff until the last push)
* modify the mach harness to support browser-dt, probably add in 'mach developer-tools' as an option
* modify buildbot to add a new test type with the additional commandline args: '--subsuite=devtools'
* modify trychooser to have the new M(dt) option to choose from
* modify tbpl/treeherder to support M(dt)

I see that as being mostly in order of operations.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

Just for kicks, here's a Try run with all devtools tests disabled to see what the affect on mochitest-bc would be. And as an aside, the moz.build files inside browser/devtools are a bit of rats nest. Would be nice if we could clean those up independent of the rest of this.

https://tbpl.mozilla.org/?tree=Try&rev=a7fe587af6f7

Comment 12

[Ryan VanderMeulen [:RyanVM]]

Moral of the story from the Try run:
* Runtimes on opt builds dropped by 50%, roughly in line with the reduction in number of tests run.
* Debug build runtimes dropped by closer to 60% (linux64 for example was 140+ -> ~60, linux32 170+ -> ~70)
* It wasn't a cure-all for our orange woes. browser_newtab_bug735987.js (bug 898317) continues to hurt Windows badly. Sessionstore tests continue to have trouble. And Linux debug is still bad. Really bad. "Why is this even visible?" bad.

I think this shows that mochitest-bc would benefit tremendously from mochitest-dt. We can also judge independently at that point whether it is a suite that meets TBPL visibility standards without impacting everything else that runs in mochitest-bc.

Comment 13

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Attachment: 1 Add mach mochitest-devtools

(In reply to Joel Maher (:jmaher) from comment #10)
> the work for this would include:
> * annotate manifests with subsuite keyword (just add it to the default
> section of all browser.ini in devtools folder)

Done

> * add cli option for mochitest for --subsuite

Done

> * modify runtests.py mochitest harness to ignore all tests that have a
> subsuite defined unless one is passed in, then match on it (note: leave the
> flag to ignore subsuite stuff until the last push)

Done

> * modify the mach harness to support browser-dt, probably add in 'mach
> developer-tools' as an option

Done

The following still remain as I don't believe I have access to the necessary repos so somebody else will need to take these on:
> * modify buildbot to add a new test type with the additional commandline
> args: '--subsuite=devtools'
> * modify trychooser to have the new M(dt) option to choose from
> * modify tbpl/treeherder to support M(dt)

Comment 14

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 8396301 [details] [diff] [review]
1 Add mach mochitest-devtools

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

Thanks for putting this together!

while I have no specific plan for other subsuites, this has a lot of value for other uses.  I am fine with a followup bug to fully support subsuite and make the changes outlined below.

Keep in mind that when we land this all devtools will stop working until the mozharness bits and buildbot changes are made.  We will need to land this on b2g-inbound, m-c, inbound, fx-team and any other trunk based tree.  Either land on all of them together or just let it get merged and live without coverage for ~6 hours.  We would just need to coordinate with the Sheriff's and releng on getting this out and then doing a buildbot config right afterwards.

Another option is to require another command-line option that doesn't run subsuites (--no-subsuites).  This would make it easier to coordinate with releng on deploying buildbot and mozharness changes.

r+ assuming we file a followup bug for manifestparser/runtests to support arbitrary subsuites and addressing the two nits below.

::: testing/mochitest/runtests.py
@@ +308,5 @@
>      # allow relative paths for logFile
>      if options.logFile:
>        options.logFile = self.getLogFilePath(options.logFile)
> +    if options.browserChrome or options.chrome or options.subsuite or \
> +       options.a11y or options.webapprtChrome:

this assumes subsuite is a chrome/browser-chrome style test.  If we made webgl a subsuite (which is run in mochitest-plain) this would fail.  I don't have a specific recommendation for this now other than to add a comment and file a followup bug.

::: testing/mozbase/manifestdestiny/manifestparser/manifestparser.py
@@ +1096,5 @@
> +        if options:
> +            if options.devtools:
> +                tests = [test for test in tests if test['subsuite'] == 'devtools']
> +            else:
> +                tests = [test for test in tests if test['subsuite'] != 'devtools']

again, this is hardcoded for this suite, I think a future proofing way would be to set tests initially like this:
tests = [test for test in tests if not test['subsuite']]

then:
if options and options.devtools:
 tests = [test for test in tests if test['subsuite'] == 'devtools']

Comment 15

[Joel Maher ( :jmaher ) (UTC -8)]

Armen, how much work is it to create a mochitest-dt run from buildbot?  I used to be familiar with the whole process, but since mozharness was rolled out I haven't fiddled much with it.  Could you outline the high level steps required from the mozharness/buildbot perspective to do this?

Comment 16

[Armen [:armenzg]]

Here are the different pieces involved.

1) Add it to MOCHITEST [1]
** That will add it across the board for all platforms for all branches
** If we need less branches or less platforms we can modify it with something like this [2]
2) Add it to mozharness [3][4][5]
3) Add it to trychooser [6]

Step #1 can vary wildly depending if a different configuration is needed. Enabling everywhere requires everything to be "green". I assume that will require some branch limitations until then [2].

[1] http://hg.mozilla.org/build/buildbot-configs/file/default/mozilla-tests/config.py#l376
[2] http://hg.mozilla.org/build/buildbot-configs/file/default/mozilla-tests/config.py#l1825
[3] http://hg.mozilla.org/build/mozharness/file/f0bb54c56271/configs/unittests/linux_unittest.py#l53
[4] http://hg.mozilla.org/build/mozharness/file/f0bb54c56271/configs/unittests/mac_unittest.py#l44
[5] http://hg.mozilla.org/build/mozharness/file/f0bb54c56271/configs/unittests/win_unittest.py#l53
[6] http://hg.mozilla.org/build/tools/file/4e132591752e/trychooser

Comment 17

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Attachment: 1 Add mach mochitest-devtools

(In reply to Joel Maher (:jmaher) from comment #14)
> Comment on attachment 8396301 [details] [diff] [review]
> 1 Add mach mochitest-devtools
> 
> Review of attachment 8396301 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Thanks for putting this together!
> 
> while I have no specific plan for other subsuites, this has a lot of value
> for other uses.  I am fine with a followup bug to fully support subsuite and
> make the changes outlined below.
> 
> Keep in mind that when we land this all devtools will stop working until the
> mozharness bits and buildbot changes are made.  We will need to land this on
> b2g-inbound, m-c, inbound, fx-team and any other trunk based tree.  Either
> land on all of them together or just let it get merged and live without
> coverage for ~6 hours.  We would just need to coordinate with the Sheriff's
> and releng on getting this out and then doing a buildbot config right
> afterwards.
> 
> Another option is to require another command-line option that doesn't run
> subsuites (--no-subsuites).  This would make it easier to coordinate with
> releng on deploying buildbot and mozharness changes.
> 
> r+ assuming we file a followup bug for manifestparser/runtests to support
> arbitrary subsuites and addressing the two nits below.
> 
> ::: testing/mochitest/runtests.py
> @@ +308,5 @@
> >      # allow relative paths for logFile
> >      if options.logFile:
> >        options.logFile = self.getLogFilePath(options.logFile)
> > +    if options.browserChrome or options.chrome or options.subsuite or \
> > +       options.a11y or options.webapprtChrome:
> 
> this assumes subsuite is a chrome/browser-chrome style test.  If we made
> webgl a subsuite (which is run in mochitest-plain) this would fail.  I don't
> have a specific recommendation for this now other than to add a comment and
> file a followup bug.
> 

Added comment, bug 987743 filed.

> ::: testing/mozbase/manifestdestiny/manifestparser/manifestparser.py
> @@ +1096,5 @@
> > +        if options:
> > +            if options.devtools:
> > +                tests = [test for test in tests if test['subsuite'] == 'devtools']
> > +            else:
> > +                tests = [test for test in tests if test['subsuite'] != 'devtools']
> 
> again, this is hardcoded for this suite, I think a future proofing way would
> be to set tests initially like this:
> tests = [test for test in tests if not test['subsuite']]
> 
> then:
> if options and options.devtools:
>  tests = [test for test in tests if test['subsuite'] == 'devtools']

Or even better:
Set options.subsuite to 'devtools' for devtools stuff

if options and options.subsuite:
    tests = [test for test in tests if options.subsuite == test['subsuite']]
else:
    tests = [test for test in tests if not test['subsuite']]

This way nobody needs to touch this section of code again.

Now adding a subsuite is simply a case of:
1. Add subsuite = devtools to [DEFAULT] section of browser.ini file in each subsuite folder.

2. Add subsuite in testing/mach_commands.py
+    'mochitest-devtools': {
+        'aliases': ('dt', 'DT', 'Dt'),
+        'mach_command': 'mochitest-browser --subsuite=devtools',
+        'kwargs': {'test_file': None},
+    },

3. Act on suite in testing/mochitest/mach_commands.py:
+        elif suite == 'devtools':
+            options.browserChrome = True
+            options.subsuite = 'devtools'
...
+    @Command('mochitest-devtools', category='testing',
+        conditions=[conditions.is_firefox],
+        description='Run a devtools mochitest with browser chrome.')
+    @MochitestCommand
+    def run_mochitest_devtools(self, test_file, **kwargs):
+        return self.run_mochitest(test_file, 'devtools', **kwargs)

4. Add the suite to testing/testsuite-targets.mk:
-MOCHITESTS := mochitest-plain mochitest-chrome mochitest-a11y mochitest-ipcplugins
+MOCHITESTS := mochitest-plain mochitest-chrome mochitest-devtools mochitest-a11y mochitest-ipcplugins
...
+mochitest-devtools:
+	$(RUN_MOCHITEST) --subsuite=devtools
+	$(CHECK_TEST_ERROR)
...
.PHONY: \
   mochitest \
   mochitest-plain \
   mochitest-chrome \
+  mochitest-devtools \
   mochitest-a11y \

Comment 18

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Attachment: 1 Add mach mochitest-devtools

subsuite = devtools was somehow dropped from markup view... added it again.

Comment 19

[Joel Maher ( :jmaher ) (UTC -8)]

Attachment: mozharness changes to support devtools (1.0)

Comment 20

[Joel Maher ( :jmaher ) (UTC -8)]

Attachment: buildbot custom changes to support mochitest-devtools (1.0)

Comment 21

[Ryan VanderMeulen [:RyanVM]]

Talking to jmaher on IRC, I think we should call this job mochitest-devtools-chrome to better match the convention of other tests running in this harness (like mochitest-metro-chrome). That's the job name I'm going to base the TBPL patch off.

Comment 22

[Joel Maher ( :jmaher ) (UTC -8)]

Attachment: [checked-in] mozharness changes to support devtools (1.1)

adjusting internal name to mochitest-devtools-chrome

Comment 23

[Joel Maher ( :jmaher ) (UTC -8)]

Attachment: buildbot custom changes to support mochitest-devtools (1.1)

adjusting internal name to mochitest-devtools-chrome

Comment 24

[Joel Maher ( :jmaher ) (UTC -8)]

mozharness bits landed:
https://hg.mozilla.org/build/mozharness/rev/7581de0051fa

Comment 25

[Armen [:armenzg]]

Comment on attachment 8397217 [details] [diff] [review]
buildbot custom changes to support mochitest-devtools (1.1)

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

I will attach the list of builders that get added with this patch.

In my opinion we should see this running first on one or two branches (e.g. mozilla-inbound, try) and then when ready enable across the board.

Otherwise, if it is not green we would have to visit every tbpl page to hide the builder.

::: mozilla-tests/config.py
@@ +414,4 @@
>          'script_path': 'scripts/desktop_unittest.py',
>          'extra_args': ['--mochitest-suite', 'browser-chrome'],
>          'blob_upload': True,
> +        'script_maxtime': 7200,

As long as the jobs are finishing these days withing that range.

@@ +455,4 @@
>          'script_path': 'scripts/desktop_unittest.py',
>          'extra_args': ['--mochitest-suite', 'browser-chrome-3'],
>          'blob_upload': True,
> +        'script_maxtime': 4200,

I assume not intentional.

@@ +1698,3 @@
>          'branches': ['fx-team'],
>          'platforms': {
>              'ubuntu64_vm': {'ext': 'linux-x86_64.tar.bz2', 'debug': True},

I assume not intentional.

Comment 26

[Armen [:armenzg]]

Attachment: builders that change

Comment 27

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 8397242 [details]
builders that change

that is a lot of branches, but the changes look right (platforms + pgo/opt/debug)

Comment 28

[Joel Maher ( :jmaher ) (UTC -8)]

Attachment: [checked-in] buildbot configs for devtools - cedar only (2.0)

updated to support cedar only for a trial run, r+ via irc

Comment 29

[Joel Maher ( :jmaher ) (UTC -8)]

buildbot configs for cedar:
https://hg.mozilla.org/build/buildbot-configs/rev/f2db0e466ef7

Comment 30

[Justin Wood (:Callek)]

Comment on attachment 8397216 [details] [diff] [review]
[checked-in] mozharness changes to support devtools (1.1)

this patch is Live in production

Comment 31

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 8396518 [details] [diff] [review]
1 Add mach mochitest-devtools

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

::: testing/mozbase/manifestdestiny/manifestparser/manifestparser.py
@@ +1095,5 @@
> +        # Filter on current subsuite
> +        if options and options.subsuite:
> +            tests = [test for test in tests if options.subsuite == test['subsuite']]
> +        else:
> +            tests = [test for test in tests if not test['subsuite']]

previously this had been:
if options:
    if options.subsuite:
        tests = <matching subsuite>
    else:
        tests = <not matching subsuite>

I tested locally before pushing to cedar, and switching back to this logic works.  The reason why is the build system was filtering the non subsuite tests out and they didn't exist- so since the build system doesn't pass in options, we work fine with the extra if nesting.

Comment 32

[Joel Maher ( :jmaher ) (UTC -8)]

I have pushed this up to cedar and most likely tomorrow we will have the buildbot-config changes for there.  We can see this live and next week we can roll out to all integration and project branches.

Comment 33

[Panos Astithas (he/him) [:past] (please ni?)]

(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #21)
> Talking to jmaher on IRC, I think we should call this job
> mochitest-devtools-chrome to better match the convention of other tests
> running in this harness (like mochitest-metro-chrome). That's the job name
> I'm going to base the TBPL patch off.

These are browser mochitests, but we also have devtools tests of the mochitest-chrome variant. Oh well.

Comment 34

[Ryan VanderMeulen [:RyanVM]]

Yeah, it's a bit unfortunate how things are named, but realistically, it's just a behind the scenes name anyway.

Comment 35

[John Hopkins (:jhopkins)]

in production

Comment 36

[Armen [:armenzg]]

Attachment: add which configs to use for each platform

Comment 37

[Armen [:armenzg]]

Comment on attachment 8397982 [details] [diff] [review]
add which configs to use for each platform

We have to rename.

Comment 38

[Armen [:armenzg]]

Attachment: [checked-in] add which configs to use for each platform

Comment 39

[Armen [:armenzg]]

Live in production.

Comment 40

[Ryan VanderMeulen [:RyanVM]]

https://tbpl.mozilla.org/?tree=Cedar&rev=a0682a0b637c&jobname=mochitest-devtools-chrome

Not as green as I would have hoped :(

Comment 41

[Joel Maher ( :jmaher ) (UTC -8)]

if we fix /browser/devtools/app-manager/test/browser_manifest_editor.js for linux opt (bug 989168), that would clean up about 1/3 of the failures, and almost all the rest are in styleinspector tests.  Personally we could probably fix one or two style inspector tests and call this 95% green.

Comment 42

[Joel Maher ( :jmaher ) (UTC -8)]

Attachment: update for bitrot and added fixes for tests (3.0)

Comment 43

[Joel Maher ( :jmaher ) (UTC -8)]

Attachment: [checked-in] support for linux64-asan and osx 10.9 (1.0)

this is not mission critical, but this should get us support for the remaining platforms.

Comment 44

[Joel Maher ( :jmaher ) (UTC -8)]

asan + 10.9 is landed:
https://hg.mozilla.org/build/buildbot-configs/rev/fc2f1e3b94f3

will be picked up in the next reconfig.

Comment 45

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8398472 [details] [diff] [review]
update for bitrot and added fixes for tests (3.0)

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

The "mach mochitest-devtools" part of this patch doesn't seem to work for me.

I get the following error:

TypeError: run_mochitest_devtools() takes exactly 2 arguments (1 given)
     
 File "/Users/jryans/projects/mozilla/gecko-dev-2/python/mach/mach/main.py", line 416, in _run
   result = fn(**vars(args.command_args))

Comment 46

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

jmaher sent me a small diff that fixes this error, and I can now run the new suite locally.

However, the new "mach mochitest-devtools" command appears to not be able to run just a single file or directory (instead it requires the full suite to be run) at the moment, so I think for now I will continue debugging without it.  We should definitely add this functionality now / very soon!

Comment 47

[John Hopkins (:jhopkins)]

in production.

Comment 48

[Joel Maher ( :jmaher ) (UTC -8)]

Attachment: [checked-in] mozharness changes for 2 chunk devtools (1.0)

Comment 49

[Joel Maher ( :jmaher ) (UTC -8)]

Attachment: buildbot-config changes for 2 chunks of devtools (1.0)

Comment 50

[Armen [:armenzg]]

I will look at the patch tomorrow. Please poke in #releng for a reviewer if you would prefer to speed this up.

Comment 51

[Joel Maher ( :jmaher ) (UTC -8)]

mozharness changes landed on default:
https://hg.mozilla.org/build/mozharness/rev/19d4c2bb576e

we need these to land prior to the buildbot changes, so we can wait until tomorrow for the buildbot-config changes.

Comment 52

[Armen [:armenzg]]

Comment on attachment 8397216 [details] [diff] [review]
[checked-in] mozharness changes to support devtools (1.1)

https://hg.mozilla.org/build/mozharness/rev/7581de0051fa

Do we need definitions for both m-d-c and chunked m-d-c?

Comment 53

[Armen [:armenzg]]

(In reply to Armen Zambrano [:armenzg] (Release Engineering) (EDT/UTC-4) from comment #52)
> Comment on attachment 8397216 [details] [diff] [review]
> [checked-in] mozharness changes to support devtools (1.1)
> 
> https://hg.mozilla.org/build/mozharness/rev/7581de0051fa
> 
> Do we need definitions for both m-d-c and chunked m-d-c?

NVM. I see what's going on.

Comment 54

[Joel Maher ( :jmaher ) (UTC -8)]

buildbot configs for 2 chunk debug devtools landed:
https://hg.mozilla.org/build/buildbot-configs/rev/b20a0773d06c

Comment 55

[Jordan Lund (:jlund)]

https://hg.mozilla.org/build/buildbot-configs/rev/b20a0773d06c -> live in production :)

Comment 56

[Jordan Lund (:jlund)]

https://bugzilla.mozilla.org/attachment.cgi?id=8400685 -> live in production :)

Comment 57

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Attachment: Switched back to test_paths and fixed running tests under a path.

Now tests run fine when a subpath is included.

Comment 58

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 8402605 [details] [diff] [review]
Switched back to test_paths and fixed running tests under a path.

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

this looks good.  Glad you figured out the test path stuff.  We have a few more subsuites to add:
browser/devtools/canvasdebugger
browser/devtools/webaudioeditor
toolkit/devtools/server/test/browser

I would really like to get this landed, but turn off the default filtering.  On cedar we are close, but have 5 or 6 bugs in the works (i.e. actively testing fixes).  This way we can avoid bitrot and keep any future patches much smaller.  Once we are ready to make the switch (a week or so), we can turn on the filtering by default.  I believe that will be just a small change to line 469 in runtests.py.

::: python/mozbuild/mozbuild/testing.py
@@ +91,5 @@
>          def fltr(tests):
>              for test in tests:
> +                if flavor and \
> +                   (flavor == 'devtools' and test.get('flavor') != 'browser-chrome') or \
> +                   flavor != 'devtools' and test.get('flavor') != flavor:

I am not an expert at python left to right parsing of expressions, but I would personally add a set of ()'s around the last line here to be more clear.

Comment 59

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Attachment: Added canvasdebugger, webaudioeditor and toolkit/devtools/server/test/browser

(In reply to Joel Maher (:jmaher) from comment #58)
> Comment on attachment 8402605 [details] [diff] [review]
> Switched back to test_paths and fixed running tests under a path.
> 
> Review of attachment 8402605 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> this looks good.  Glad you figured out the test path stuff.  We have a few
> more subsuites to add:
> browser/devtools/canvasdebugger
> browser/devtools/webaudioeditor
> toolkit/devtools/server/test/browser
> 

Added.

> I would really like to get this landed, but turn off the default filtering. 
> On cedar we are close, but have 5 or 6 bugs in the works (i.e. actively
> testing fixes).  This way we can avoid bitrot and keep any future patches
> much smaller.  Once we are ready to make the switch (a week or so), we can
> turn on the filtering by default.  I believe that will be just a small
> change to line 469 in runtests.py.
> 

To be honest I am not sure which filtering you are talking about.

> ::: python/mozbuild/mozbuild/testing.py
> @@ +91,5 @@
> >          def fltr(tests):
> >              for test in tests:
> > +                if flavor and \
> > +                   (flavor == 'devtools' and test.get('flavor') != 'browser-chrome') or \
> > +                   flavor != 'devtools' and test.get('flavor') != flavor:
> 
> I am not an expert at python left to right parsing of expressions, but I
> would personally add a set of ()'s around the last line here to be more
> clear.

Agreed, done.

Comment 60

[Joel Maher ( :jmaher ) (UTC -8)]

I was thinking a way to turn this off would be to not add the options into active_tests in runtests.py:
 tests = manifest.active_tests(disabled=True, options=options, **info)

Since we wouldn't be passing in options, then we wouldn't filter.  This leaves us with a one line change to alter everything.  I am open to other thoughts.

Thanks for updating the patch to include everything.  On Cedar we have a few fixes to tests landed, a dozen tests disabled on there, and a few harness tweaks.  With all of that we are just about as green as a leprechaun- quite possibly this week we can make all of it official and landed so we can make this change officially.  I view landing the bulk of these changes as part of the prep work, one less mystery.

Comment 61

[Joel Maher ( :jmaher ) (UTC -8)]

Attachment: turn debug mochitest-dt into 3 chunks. (1.0)

Comment 62

[Joel Maher ( :jmaher ) (UTC -8)]

Attachment: buildbot-config changes for 3 chunks of devtools (1.0)

Comment 63

[Joel Maher ( :jmaher ) (UTC -8)]

3 chunks for dt;

buildbot-configs:
https://hg.mozilla.org/build/buildbot-configs/rev/d8287cc60b51

mozharness changes (default branch):
https://hg.mozilla.org/build/mozharness/rev/2ebeb3843c6b

Comment 64

[Joel Maher ( :jmaher ) (UTC -8)]

Attachment: buildbot-config changes for 3 chunks of devtools (configs this time) (1.0)

Comment 65

[Jordan Lund (:jlund)]

https://bugzilla.mozilla.org/attachment.cgi?id=8402806&action=edit
https://bugzilla.mozilla.org/attachment.cgi?id=8402807&action=edit

are in production :)

Comment 66

[Joel Maher ( :jmaher ) (UTC -8)]

landed followup for 3rd dt chunk on buildbot-configs:
https://hg.mozilla.org/build/buildbot-configs/rev/c0ddd294a88e

Comment 67

[Jordan Lund (:jlund)]

https://bugzilla.mozilla.org/attachment.cgi?id=8403406&action=edit is in production :)

Comment 68

[Joel Maher ( :jmaher ) (UTC -8)]

Attachment: hard quit on --subsuite until we are ready to turn this on (1.0)

this is a temporary patch to be applied on top of the mochitest harness patch.  We are looking to enable this tomorrow if all goes well.  In order for that to happen, we need to have the mochitest test harness support --subsuite.

Comment 69

[Joel Maher ( :jmaher ) (UTC -8)]

landed:
https://hg.mozilla.org/integration/mozilla-inbound/rev/52da0fc935ce

and the temporary hack:
https://hg.mozilla.org/integration/mozilla-inbound/rev/df3c1971ecdc

the ball is rolling.

Comment 70

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/df3c1971ecdc

Comment 71

[Ed Morley [:emorley]]

Believe this was meant to be leave-open?

Comment 72

[Ed Morley [:emorley]]

(In reply to Joel Maher (:jmaher) from comment #69)
> landed:
> https://hg.mozilla.org/integration/mozilla-inbound/rev/52da0fc935ce

Landed with the wrong bug number (ended up in bug 983948); was merged at the same time as comment 70:
https://hg.mozilla.org/mozilla-central/rev/52da0fc935ce

Comment 73

[Joel Maher ( :jmaher ) (UTC -8)]

Attachment: remove cedar hack (1.0)

Comment 74

[Joel Maher ( :jmaher ) (UTC -8)]

Attachment: only add mavericks tests if we are on cedar (1.0)

Comment 75

[[e-mail] (:bhearsum)]

Attachment: [checked in] turn on the stuff we want

Ryan and Joel reviewed this over IRC over the course of many hours. I've checked it in, and I'm about to start a reconfig.

Comment 76

[Ryan VanderMeulen [:RyanVM]]

(In reply to Joel Maher (:jmaher) from comment #69)
> and the temporary hack:
> https://hg.mozilla.org/integration/mozilla-inbound/rev/df3c1971ecdc

Backed out.
https://hg.mozilla.org/mozilla-central/rev/591d708edaee

Go time.

Comment 77

[Pete Moore [:pmoore][:pete]]

Comment on attachment 8405388 [details] [diff] [review]
only add mavericks tests if we are on cedar (1.0)

Seems a reasonable change to me, but as discussed on IRC, we probably want to go with Ben's larger refactoring.

Comment 78

[Joe Walker [:jwalker]]

Since this landed: Bug 984930 - "mochitest-devtools mach" command doesn't allow to run single folders or files

Comment 79

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

(In reply to Joe Walker [:jwalker] from comment #78)
> Since this landed: Bug 984930 - "mochitest-devtools mach" command doesn't
> allow to run single folders or files

The fix is in bug 995972

Comment 80

[Brian Grinstead [:bgrins]]

I've updated the documentation at https://wiki.mozilla.org/DevTools/Hacking#Running_the_Developer_Tools_Tests to point to mochitest-devtools instead of mochitest-browser.  These were the only docs I could find referencing the test suites for devtools.

Comment 81

[Ryan VanderMeulen [:RyanVM]]

Bug 995972 still needs to land to fix running single directories with mach, but AFAIK, no other follow-up work is pending at this point. Bug 996240 is filed to track getting this enabled on Aurora30 as well.