920569 - Add support for webapprt-test-chrome test jobs & enable them per push on Cedar

Status: RESOLVED FIXED

(Release Engineering :: General, defect)

Description

[Ed Morley [:emorley]]

Broken out from bug 899707.

The Firefox Webapp runtime team would like to start running the webapprt test suite per push in automation. Initially these should just be scheduled on Cedar (to green them up), but eventually they will need to be enabled on all trunk trees.

Platforms: Windows, Linux, OS X
Builds types: Opt + debug

The test suites are currently run using:
|mach webapprt-test-chrome| and |mach webapprt-test-content|

Comment 1

[Ed Morley [:emorley]]

Marco, should both the chrome and content suites be run inside the same job run (ie similar to how mochitest-other currently runs several sub-suites) or as completely separate jobs/symbols on TBPL?

Comment 2

[Marco Castelluccio [:marco]]

I think we should start with webapprt-test-chrome as webapprt-test-content will likely change. When we'll want to enable content tests, I think they will be separate.

Comment 3

[Chris AtLee [:catlee]]

Do these run as part of the build, or as a separate test job? If the latter, then we can't use mach and will need to know the underlying commands that mach is executing. We'll also have to make sure all the tests are packaged up in the tests archive.

Comment 4

[Marco Castelluccio [:marco]]

(In reply to Chris AtLee [:catlee] from comment #3)
> Do these run as part of the build, or as a separate test job? If the latter,
> then we can't use mach and will need to know the underlying commands that
> mach is executing. We'll also have to make sure all the tests are packaged
> up in the tests archive.

Yes, they run as part of the build.
The webapprt chrome tests are packaged in obj-*/_tests/testing/mochitest/webapprtChrome.

Comment 5

[cmtalbert]

Catlee, this doesn't actually have to be run by mach on the builders. You can run these using the standard packaged tests using slight tweaks to the existing mochitest command lines. We need to ensure that the webapprt-stub application is bundled with our builds (which I believe it should be, since it's in the dist/bin directory of our builds. Here are the command lines required (you'd still need to supply xre-path, certificate-path, and utility-path but they are the standard values that other mochitests use so I didn't include them here.

webapprt-content tests: (important ones here are --webapprt-content, and --app)
python runtests.py --webapprt-content --app=../../../dist/bin/webapprt-stub --autorun --close-when-done --console-level=INFO
Currently on my m-c build this runs 6 tests all of which pass.

webapprt-chrome tests: (important ones here are --webapprt-chrome, --app, --browser-arg and --testing-modules-dir)
python runtests.py --webapprt-chrome --app=../../../dist/bin/webapprt-stub --autorun --close-when-done --console-level=INFO --browser-arg=-test-mode --extra-profile-file=/home/ctalbert/projects/m-c/obj-dbg-linux/dist/plugins --testing-modules-dir=/home/ctalbert/projects/m-c/obj-dbg-linux/_tests/modules/
Currently on my m-c build: this runs 220 tests which pass and 1 which fails

So, we should be able to tweak the existing mozharness scripts for this support and we don't need to run them on builders.

Comment 6

[Myk Melez [:myk] [@mykmelez]]

(In reply to Clint Talbert ( :ctalbert ) from comment #5)
> webapprt-content tests: (important ones here are --webapprt-content, and
> --app)

Note that this bug is just about the webapprt-chrome tests, which is the first test suite that we'd like to start running per-commit.


> webapprt-chrome tests: (important ones here are --webapprt-chrome, --app,
> --browser-arg and --testing-modules-dir)
...
> Currently on my m-c build: this runs 220 tests which pass and 1 which fails

I filed the failure as bug 976300.

Comment 7

[Myk Melez [:myk] [@mykmelez]]

We're back to green!  catlee: What's the next step?

Comment 8

[Marco Castelluccio [:marco]]

Per comment 7, what's the next step? Can we help in some way to make this happen?

Comment 9

[Ed Morley [:emorley]]

(In reply to Marco Castelluccio [:marco] from comment #8)
> Per comment 7, what's the next step? Can we help in some way to make this
> happen?

The next step will need to come from releng (I'm A-Team).

Comment 10

[Chris AtLee [:catlee]]

We could really use some help writing the patches. Does ateam have anybody who can help?

Comment 11

[Jonathan Griffin (:jgriffin)]

We're going to need three patches:  one to in-tree configs, one to mozharness, and one to buildbot-configs.  I'll start the ball rolling, but won't be able to finish before I PTO.

Comment 12

[Jonathan Griffin (:jgriffin)]

Attachment: Add webapprt to in-tree configs,

This adds a webapprt section to the in-tree configs, which includes all of the common options for chrome/content webapprt tests.  The exceptional options will be specified in buildbot-configs.

Comment 13

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

Comment on attachment 8412763 [details] [diff] [review]
Add webapprt to in-tree configs,

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

the fun begins

Comment 14

[Jonathan Griffin (:jgriffin)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d4684d3c0340

Comment 15

[Jonathan Griffin (:jgriffin)]

Attachment: Mozharness support for webapprt tests,

I'm going to land this on ash to make sure it's generally harmless.

Comment 16

[Jonathan Griffin (:jgriffin)]

pushed to ash-mozharness:  http://hg.mozilla.org/users/asasaki_mozilla.com/ash-mozharness/rev/ecc677987e7f

"try" run on ash:  https://tbpl.mozilla.org/?tree=Ash&rev=779568f0fd70

Comment 17

[Jonathan Griffin (:jgriffin)]

Attachment: Schedule webapprt-chrome on cedar,

Schedule the webapprt-chrome tests on all desktop platforms on cedar.  Shouldn't land until after the mozharness patch does.

Comment 18

[Aki Sasaki (not active)]

Comment on attachment 8412857 [details] [diff] [review]
Schedule webapprt-chrome on cedar,

lgtm, you're a pro at this!

Comment 19

[Armen [:armenzg]]

Comment on attachment 8412801 [details] [diff] [review]
Mozharness support for webapprt tests,

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

The patch looks good to me, however, I would like to get jlund's blessings since it is a non-standard new-{mochitest,reftest,known_category} type of patch.

Comment 20

[Jonathan Griffin (:jgriffin)]

New try run on ash: https://tbpl.mozilla.org/?tree=Ash&rev=31322f05bd0b (looks good so far)

Comment 21

[Jonathan Griffin (:jgriffin)]

ash looks good; I think this can be landed on mozharness after r+

Comment 22

[Jordan Lund (:jlund)]

Comment on attachment 8412801 [details] [diff] [review]
Mozharness support for webapprt tests,

lgtm! awesome to see more suites being added. :)

I noticed that by supplying --webapprt-content, --browser-arg will be extended with '-test-mode' directly inside mochitest's runtests.py: http://mxr.mozilla.org/mozilla-central/source/testing/mochitest/runtests.py#1250

is there a reason why we explicitly pass --browser-arg=-test-mode for webapprt-chrome? Doesn't really matter j/w.

also, we can't land this patch, even on default, until https://hg.mozilla.org/integration/mozilla-inbound/rev/d4684d3c0340 merges with m-c (as of now it hasn't) otherwise everything will burn like it did in ash: https://tbpl.mozilla.org/?tree=Ash&rev=779568f0fd70

which makes me wonder if we have implemented self.tree_config correctly. As in, even if we are trying to run a mochitest or reftest suite, by just having 'self._run_category_suites('webapprt')' in desktop_unittest.py, we will fail  due to http://mxr.mozilla.org/build/source/mozharness/scripts/desktop_unittest.py#251

maybe the answer is to not bother looking up abs_base_cmd unless we are actually running that suite: eg: http://mxr.mozilla.org/build/source/mozharness/scripts/desktop_unittest.py#381 after http://mxr.mozilla.org/build/source/mozharness/scripts/desktop_unittest.py#387

IIUC we will need a TBPL patch for the new suite.

Comment 23

[Ryan VanderMeulen [:RyanVM]]

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

Comment 24

[Armen [:armenzg]]

Comment on attachment 8412763 [details] [diff] [review]
Add webapprt to in-tree configs,

https://hg.mozilla.org/integration/mozilla-inbound/rev/d4684d3c0340

Comment 25

[Jonathan Griffin (:jgriffin)]

(In reply to Jordan Lund (:jlund) from comment #22)
> Comment on attachment 8412801 [details] [diff] [review]
> which makes me wonder if we have implemented self.tree_config correctly. As
> in, even if we are trying to run a mochitest or reftest suite, by just
> having 'self._run_category_suites('webapprt')' in desktop_unittest.py, we
> will fail  due to
> http://mxr.mozilla.org/build/source/mozharness/scripts/desktop_unittest.
> py#251
> 
> maybe the answer is to not bother looking up abs_base_cmd unless we are
> actually running that suite: eg:
> http://mxr.mozilla.org/build/source/mozharness/scripts/desktop_unittest.
> py#381 after
> http://mxr.mozilla.org/build/source/mozharness/scripts/desktop_unittest.
> py#387
> 
> IIUC we will need a TBPL patch for the new suite.

Yes, this was very surprising when this failed on ash, even though we weren't running the tests yet there.  I think we should make the change you suggest.  We'll need to do this before running this on trunk branches, so we don't have to check in the in-tree configs for this in all upstream branches.

Comment 26

[Armen [:armenzg]]

Return this bug to jgriffin to follow up on comment 25.
Let me know if you would prefer me to take care of it next week.

Comment 27

[Jonathan Griffin (:jgriffin)]

Attachment: Mozharness support for webapprt tests,

This includes the change suggested in bug 920569 comment #25

Comment 28

[Jordan Lund (:jlund)]

Comment on attachment 8422762 [details] [diff] [review]
Mozharness support for webapprt tests,

lgtm! :)

Comment 29

[Jonathan Griffin (:jgriffin)]

Comment on attachment 8422762 [details] [diff] [review]
Mozharness support for webapprt tests,

https://hg.mozilla.org/build/mozharness/rev/5f1962b1e8a4

Comment 30

[Jonathan Griffin (:jgriffin)]

Comment on attachment 8422762 [details] [diff] [review]
Mozharness support for webapprt tests,

in production

Comment 31

[Jonathan Griffin (:jgriffin)]

Comment on attachment 8412857 [details] [diff] [review]
Schedule webapprt-chrome on cedar,

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

Comment 32

[Justin Wood (:Callek)]

Something here went live today

Comment 33

[Myk Melez [:myk] [@mykmelez]]

And promptly started failing on some machines with:

========= Started '/tools/buildbot/bin/python scripts/scripts/desktop_unittest.py ...' failed (results: 2, elapsed: 0 secs) (at 2014-05-23 07:23:24.672020) =========
/tools/buildbot/bin/python scripts/scripts/desktop_unittest.py --webapprt-suite chrome --blob-upload-branch cedar --download-symbols ondemand
…
Required config file not set! (use --config-file option)
program finished with exit code 255
elapsedTime=0.288343
========= Finished '/tools/buildbot/bin/python scripts/scripts/desktop_unittest.py ...' failed (results: 2, elapsed: 0 secs) (at 2014-05-23 07:23:24.980298) =========


And other machines with:

07:44:11  WARNING -  TEST-UNEXPECTED-FAIL | (browser-test.js) | Found an unexpected webapprt:mochitest
…
07:44:57  WARNING -  TEST-UNEXPECTED-FAIL | chrome://mochitests/content/webapprtChrome/webapprt/test/chrome/browser_alarm.js | Test timed out
07:44:57  WARNING -  TEST-UNEXPECTED-FAIL | chrome://mochitests/content/webapprtChrome/webapprt/test/chrome/browser_alarm.js | Cleanup function threw an exception at chrome://mochitests/content/webapprtChrome/webapprt/test/chrome/browser_alarm.js:35 - TypeError: mutObserverAlarmSet is null
etc.

Comment 34

[Marco Castelluccio [:marco]]

(In reply to Myk Melez [:myk] [@mykmelez] from comment #33)
> 07:44:11  WARNING -  TEST-UNEXPECTED-FAIL | (browser-test.js) | Found an
> unexpected webapprt:mochitest
> …
> 07:44:57  WARNING -  TEST-UNEXPECTED-FAIL |
> chrome://mochitests/content/webapprtChrome/webapprt/test/chrome/
> browser_alarm.js | Test timed out
> 07:44:57  WARNING -  TEST-UNEXPECTED-FAIL |
> chrome://mochitests/content/webapprtChrome/webapprt/test/chrome/
> browser_alarm.js | Cleanup function threw an exception at
> chrome://mochitests/content/webapprtChrome/webapprt/test/chrome/
> browser_alarm.js:35 - TypeError: mutObserverAlarmSet is null
> etc.

I've seen the mutObserver error happening intermittently, but bug 1007402 should fix this.

The failure in the debugger test is due to bug 859372 (filed bug 1014141 to fix the regression).

Comment 35

[Jonathan Griffin (:jgriffin)]

I'll look at the 'config file not set' problem.

Comment 36

[Jonathan Griffin (:jgriffin)]

Attachment: Fix 'config file not found' errors,

This fixes the 'config file not found' errors.  It looks like hg's fuzzing failed on the earlier patch, due to all the similar chunks of code in this file.

Comment 37

[Jordan Lund (:jlund)]

Comment on attachment 8429692 [details] [diff] [review]
Fix 'config file not found' errors,

lgtm :)

Comment 38

[Jonathan Griffin (:jgriffin)]

Comment on attachment 8429692 [details] [diff] [review]
Fix 'config file not found' errors,

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

Comment 39

[Chris Cooper [:coop] (he/him)]

Merged to production and deployed.

Comment 40

[Myk Melez [:myk] [@mykmelez]]

The most recent builds are pretty busted in general, but this test suite is still showing a config file issue, "Required config file not set! (use --config-file option)":

https://tbpl.mozilla.org/php/getParsedLog.php?id=40662469&tree=Cedar&full=1

Comment 41

[Jonathan Griffin (:jgriffin)]

(In reply to Myk Melez [:myk] [@mykmelez] from comment #40)
> The most recent builds are pretty busted in general, but this test suite is
> still showing a config file issue, "Required config file not set! (use
> --config-file option)":
> 
> https://tbpl.mozilla.org/php/getParsedLog.php?id=40662469&tree=Cedar&full=1

That log is from yesterday, and the change that fixes that was just merged to production this morning.  I've just kicked off a new build on cedar, so we can see the results soon.

Comment 42

[Jonathan Griffin (:jgriffin)]

Linux and OSX builds are going on cedar; there's a problem with Windows builds that's likely the fault of the mozharness script; I'll take a look.

Comment 43

[Jonathan Griffin (:jgriffin)]

Attachment: Add missing . in EXE_SUFFIX,

This fixes the 'Error: Path C:\slave\test\build\application\firefox\webapprt-stubexe doesn't exist.' error on Windowds.

Comment 44

[Armen [:armenzg]]

Comment on attachment 8431912 [details] [diff] [review]
Add missing . in EXE_SUFFIX,

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

Fun!

Comment 45

[Jonathan Griffin (:jgriffin)]

Comment on attachment 8431912 [details] [diff] [review]
Add missing . in EXE_SUFFIX,

https://hg.mozilla.org/build/mozharness/rev/636bceab7560

Comment 46

[Jonathan Griffin (:jgriffin)]

These are running on cedar now.   They will show up as U until bug 1017254 is fixed, but otherwise there aren't any infrastructure issues remaining.

Comment 47

[Simone Bruno [:simone]]

In production with reconfig on 2014-06-03 00:53 PT