1066735 - Remove android and b2g specific xpcshell root manifests

Status: RESOLVED FIXED

(Testing :: XPCShell Harness, defect)

Description

[Andrew Halberstadt [:ahal]]

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

[Andrew Halberstadt [:ahal]]

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

[(not currently active) Ted Mielczarek]

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

[Andrew Halberstadt [:ahal]]

I'll tackle this as it's kind of pseudo-blocking the manifest reporting stuff I'm working on.

Comment 4

[Andrew Halberstadt [:ahal]]

Attachment: 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

[Andrew Halberstadt [:ahal]]

Attachment: 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

[Andrew Halberstadt [:ahal]]

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

[Andrew Halberstadt [:ahal]]

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

[Ryan VanderMeulen [:RyanVM]]

Bug 1073639 covers the fxaccounts slowness, fwiw.

Comment 9

[Andrew Halberstadt [:ahal]]

Attachment: 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

[Chris Manchester (limited bugmail, email directly)]

services/fxaccounts/tests/xpcshell/test_client.js is coming back with a so far consistent timeout.

Comment 11

[Chris Manchester (limited bugmail, email directly)]

I don't really know how device manager works, but it looks like a mozprocess timeout, not an xpcshell timeout.

Comment 12

[Andrew Halberstadt [:ahal]]

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

[Andrew Halberstadt [:ahal]]

(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

[Chris Manchester (limited bugmail, email directly)]

(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

[Andrew Halberstadt [:ahal]]

Attachment: 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

[Ryan VanderMeulen [:RyanVM]]

(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

[Andrew Halberstadt [:ahal]]

(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

[Chris Manchester (limited bugmail, email directly)]

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

[Andrew Halberstadt [:ahal]]

(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

[David Teller [:Yoric] - still alive but not very active]

(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

[Andrew Halberstadt [:ahal]]

Attachment: Update for bitrot

Try run looks good. There was some minor bitrot, but nothing that should cause problems. Carry r+ forward.

Comment 22

[Andrew Halberstadt [:ahal]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3d1a89fbcaa1

Comment 23

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/3d1a89fbcaa1

Comment 24

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/ed870ea3cdb8