Last Comment Bug 1066735 - Remove android and b2g specific xpcshell root manifests
: Remove android and b2g specific xpcshell root manifests
Status: RESOLVED FIXED
:
Product: Testing
Classification: Components
Component: XPCShell Harness (show other bugs)
: unspecified
: All All
-- normal (vote)
: mozilla35
Assigned To: Andrew Halberstadt [:ahal]
:
:
Mentors:
Depends on: 1067534 1073639 1170772
Blocks: 1073595 996183 1066279 1079278 1079651
  Show dependency treegraph
 
Reported: 2014-09-12 10:33 PDT by Andrew Halberstadt [:ahal]
Modified: 2015-06-02 14:34 PDT (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
fixed
fixed
fixed
fixed


Attachments
remove_android_b2g_manifests (61.28 KB, patch)
2014-09-12 13:47 PDT, Andrew Halberstadt [:ahal]
no flags Details | Diff | Splinter Review
Remove android/b2g specific xpcshell manifests (74.38 KB, patch)
2014-10-01 09:06 PDT, Andrew Halberstadt [:ahal]
no flags Details | Diff | Splinter Review
Remove android/b2g specific xpcshell manifests (75.89 KB, patch)
2014-10-07 07:33 PDT, Andrew Halberstadt [:ahal]
no flags Details | Diff | Splinter Review
Remove android/b2g specific xpcshell manifests (76.53 KB, patch)
2014-10-07 11:29 PDT, Andrew Halberstadt [:ahal]
cmanchester: review+
Details | Diff | Splinter Review
Update for bitrot (76.38 KB, patch)
2014-10-07 20:49 PDT, Andrew Halberstadt [:ahal]
ahalberstadt: review+
Details | Diff | Splinter Review

Description User image Andrew Halberstadt [:ahal] 2014-09-12 10:33:35 PDT
E.g, search for 'does not exist':
https://tbpl.mozilla.org/php/getParsedLog.php?id=47976612&tree=Mozilla-Central&full=1
https://tbpl.mozilla.org/php/getParsedLog.php?id=47973609&full=1&branch=mozilla-central

This isn't being flagged as an error because the xpcshell test harness uses strict=False when parsing the manifests [1]. We should fix these missing manifest errors and switch to strict=True.

[1] http://dxr.mozilla.org/mozilla-central/source/testing/xpcshell/runxpcshelltests.py#792
Comment 1 User image Andrew Halberstadt [:ahal] 2014-09-12 10:38:03 PDT
I'm not really sure how the xpcshell manifest generation works, but I'm guessing this issue has something to do with the fact that android and b2g have different root manifests, e.g:

http://dxr.mozilla.org/mozilla-central/source/testing/xpcshell/xpcshell_android.ini

Ideally we would get rid of those two root manifests and just skip android/b2g in the default section of the manifests they aren't supposed to run with.
Comment 2 User image Ted Mielczarek [:ted.mielczarek] 2014-09-12 10:46:35 PDT
xpcshell manifests predate the "specify manifests in moz.build" time, so yeah, we should be auto-generating the master manifests nowadays. We should fix whatever's broken and skip whatever we need to get things working with that and then ditch those other manifests.
Comment 3 User image Andrew Halberstadt [:ahal] 2014-09-12 11:00:16 PDT
I'll tackle this as it's kind of pseudo-blocking the manifest reporting stuff I'm working on.
Comment 4 User image Andrew Halberstadt [:ahal] 2014-09-12 13:47:07 PDT
Created attachment 8488832 [details] [diff] [review]
remove_android_b2g_manifests

This patch should skip all the manifests not listed in xpcshell_android.ini/xpcshell_b2g.ini. But it requires a mozharness change to get the automation to stop using them, so I'll test it out on ash.
Comment 5 User image Andrew Halberstadt [:ahal] 2014-10-01 09:06:33 PDT
Created attachment 8498238 [details] [diff] [review]
Remove android/b2g specific xpcshell manifests

Unbitrotted patch.

Also as of bug 1067534, this no longer requires a mozharness change, here's a try run:
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=d7f3dae68ac1
Comment 6 User image Andrew Halberstadt [:ahal] 2014-10-03 10:57:15 PDT
This should get fixed ASAP. It's causing us to silently lose coverage anytime a manifest is renamed/moved. E.g the indexed db manifest got renamed to xpcshell-shared.ini and we haven't been running those tests on b2g or android since.

This bug will aim to run the exact same set of tests that are currently running. I'll file followups to investigate turning the accidentally disabled tests back on later.
Comment 7 User image Andrew Halberstadt [:ahal] 2014-10-06 11:23:22 PDT
Sigh, this is almost there, but the changes re-organized the chunks a bit which apparently caused a perma-fail in services/fxaccounts/tests/xpcshell/test_client.js:
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=e233c266863a

Also 4 b2g tests seem to have been disabled as a result of my patch.. I'll need to do some log parsing to figure out which ones.
Comment 8 User image Ryan VanderMeulen [:RyanVM] 2014-10-07 07:29:48 PDT
Bug 1073639 covers the fxaccounts slowness, fwiw.
Comment 9 User image Andrew Halberstadt [:ahal] 2014-10-07 07:33:56 PDT
Created attachment 8501077 [details] [diff] [review]
Remove android/b2g specific xpcshell manifests

I found the b2g manifest that was accidentally skip-if'ed, so now both b2g and android are running the exact same set of tests as before. I think the test_client.js timeout is because I removed the 'requesttimeoutfactor = 2' that was set globally in the b2g manifest here:
http://dxr.mozilla.org/mozilla-central/source/testing/xpcshell/xpcshell_b2g.ini#9

This patch adds it back in specifically for that test. Try run:
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=b41ec898a444

I'll do a bunch of retriggers to see if there are other tests that intermittently timeout and add the timeoutfactor there too. In the meantime, let's get this reviewed!
Comment 10 User image Chris Manchester (:chmanchester) 2014-10-07 10:13:21 PDT
services/fxaccounts/tests/xpcshell/test_client.js is coming back with a so far consistent timeout.
Comment 11 User image Chris Manchester (:chmanchester) 2014-10-07 10:28:13 PDT
I don't really know how device manager works, but it looks like a mozprocess timeout, not an xpcshell timeout.
Comment 12 User image Andrew Halberstadt [:ahal] 2014-10-07 10:52:33 PDT
Yeah, you're right. Requesttimeoutfactor is definitely working because without it I see:
test_client.js | test passed (time: 311407.929ms)

and with it I see:
test_client.js | test passed (time: 611198.152ms)

So something is hanging, it's not simply being slow. I have no idea what my patch could have changed to cause this. The only thing I can think of is if it depended on some specific ordering. I may need to disable it to carry on with this patch as I really don't think we should block this on a single test failure.
Comment 13 User image Andrew Halberstadt [:ahal] 2014-10-07 10:58:15 PDT
(In reply to Chris Manchester [:chmanchester] from comment #11)
> I don't really know how device manager works, but it looks like a mozprocess
> timeout, not an xpcshell timeout.

Oh, and ps that is an xpcshell timeout. On b2g the per test timeout is used in mozdevice (which in turn uses mozprocess):
http://dxr.mozilla.org/mozilla-central/source/testing/xpcshell/remotexpcshelltests.py#135
Comment 14 User image Chris Manchester (:chmanchester) 2014-10-07 11:00:31 PDT
(In reply to Andrew Halberstadt [:ahal] from comment #13)
> (In reply to Chris Manchester [:chmanchester] from comment #11)
> > I don't really know how device manager works, but it looks like a mozprocess
> > timeout, not an xpcshell timeout.
> 
> Oh, and ps that is an xpcshell timeout. On b2g the per test timeout is used
> in mozdevice (which in turn uses mozprocess):
> http://dxr.mozilla.org/mozilla-central/source/testing/xpcshell/
> remotexpcshelltests.py#135

Gotcha, thanks. Maybe we should do a push with verbose output on and ni? the test authors if anything looks fishy in the timeouts?
Comment 15 User image Andrew Halberstadt [:ahal] 2014-10-07 11:29:50 PDT
Created attachment 8501237 [details] [diff] [review]
Remove android/b2g specific xpcshell manifests

This incarnation of the patch skips test_client.js on debug emulators. It also adds requesttimeoutfactor = 2 at the manifest level to the toolkit/components/osfile tests which RyanVM said were especially prone to intermittent timeouts.

Here's a new try run:
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=1db5e6a27d72
Comment 16 User image Ryan VanderMeulen [:RyanVM] 2014-10-07 11:33:08 PDT
(In reply to Andrew Halberstadt [:ahal] from comment #15)
> This incarnation of the patch skips test_client.js on debug emulators. It
> also adds requesttimeoutfactor = 2 at the manifest level to the
> toolkit/components/osfile tests which RyanVM said were especially prone to
> intermittent timeouts.

CC Yoric, FYI. We should get a bug on file for figuring out what if anything can be done about the runtime of the OS.File tests on B2G debug. Especially since it seems to be intermittent.
Comment 17 User image Andrew Halberstadt [:ahal] 2014-10-07 11:35:27 PDT
(In reply to Chris Manchester [:chmanchester] from comment #14)
> Gotcha, thanks. Maybe we should do a push with verbose output on and ni? the
> test authors if anything looks fishy in the timeouts?

Yeah, there's already a bug on file and I added instructions for doing that there:
https://bugzilla.mozilla.org/show_bug.cgi?id=1073639#c7
Comment 18 User image Chris Manchester (:chmanchester) 2014-10-07 12:09:00 PDT
Comment on attachment 8501237 [details] [diff] [review]
Remove android/b2g specific xpcshell manifests

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

Can we make the strict=True change in runxpcshelltests.py along with this patch? Either way this is a huge improvement to our manifest story for xpcshell.

I didn't take the step of confirming for myself we're preserving the set of running tests, but I could for a sanity check this afternoon if you prefer.

::: security/manager/ssl/tests/unit/xpcshell.ini
@@ +24,5 @@
>  [test_hmac.js]
>  
>  [test_sts_preloadlist_perwindowpb.js]
>  # Bug 978426: Test fails consistently only on B2G ARM
> +skip-if = buildapp == "b2g" && processor == "arm"

I don't see why this line shows up in the diff.

::: testing/xpcshell/example/unit/xpcshell.ini
@@ +4,5 @@
>  
>  [DEFAULT]
>  head =
>  tail =
> +skip-if = toolkit == 'gonk'

I know it was probably like this before, but I find skipping the harness checks wholesale fairly disturbing.

::: toolkit/components/osfile/tests/xpcshell/xpcshell.ini
@@ +1,5 @@
>  [DEFAULT]
>  head = head.js
>  tail =
> +# Intermittently times out on debug b2g emulators
> +requesttimeoutfactor = 2

It would require extending our manifest language, but timeoutfactor-if would be a good thing to do.
Comment 19 User image Andrew Halberstadt [:ahal] 2014-10-07 12:18:44 PDT
(In reply to Chris Manchester [:chmanchester] from comment #18)
> >  [test_sts_preloadlist_perwindowpb.js]
> >  # Bug 978426: Test fails consistently only on B2G ARM
> > +skip-if = buildapp == "b2g" && processor == "arm"
> 
> I don't see why this line shows up in the diff.

Because it used to be "processor = arm" which is a syntax error. Until now this manifest was never parsed with a b2g platform, so the right side of the && was never evaluated.
Comment 20 User image David Teller [:Yoric] (please use "needinfo") 2014-10-07 16:04:51 PDT
(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #16)
> CC Yoric, FYI. We should get a bug on file for figuring out what if anything
> can be done about the runtime of the OS.File tests on B2G debug. Especially
> since it seems to be intermittent.

Filed bug 1079454.
Comment 21 User image Andrew Halberstadt [:ahal] 2014-10-07 20:49:48 PDT
Created attachment 8501501 [details] [diff] [review]
Update for bitrot

Try run looks good. There was some minor bitrot, but nothing that should cause problems. Carry r+ forward.
Comment 22 User image Andrew Halberstadt [:ahal] 2014-10-07 20:51:06 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/3d1a89fbcaa1
Comment 23 User image Carsten Book [:Tomcat] 2014-10-08 06:58:34 PDT
https://hg.mozilla.org/mozilla-central/rev/3d1a89fbcaa1
Comment 24 User image Ryan VanderMeulen [:RyanVM] 2014-10-08 18:35:22 PDT
https://hg.mozilla.org/releases/mozilla-aurora/rev/ed870ea3cdb8

Note You need to log in before you can comment on or make changes to this bug.