Note: There are a few cases of duplicates in user autocompletion which are being worked on.

Remove android and b2g specific xpcshell root manifests

RESOLVED FIXED in Firefox 34, Firefox OS v2.1

Status

Testing
XPCShell Harness
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: ahal, Assigned: ahal)

Tracking

(Blocks: 1 bug)

unspecified
mozilla35
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox34 fixed, firefox35 fixed, b2g-v2.1 fixed, b2g-v2.2 fixed)

Details

Attachments

(2 attachments, 3 obsolete attachments)

(Assignee)

Description

3 years ago
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
(Assignee)

Comment 1

3 years ago
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.
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.
(Assignee)

Comment 3

3 years ago
I'll tackle this as it's kind of pseudo-blocking the manifest reporting stuff I'm working on.
Assignee: nobody → ahalberstadt
Status: NEW → ASSIGNED
(Assignee)

Comment 4

3 years ago
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.
(Assignee)

Updated

3 years ago
Depends on: 1067534
(Assignee)

Updated

3 years ago
Blocks: 996183
(Assignee)

Comment 5

3 years ago
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
Attachment #8488832 - Attachment is obsolete: true
(Assignee)

Comment 6

3 years ago
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.
Summary: Several android/b2g xpcshell tests not running because their manifests aren't in the test package → Remove android and b2g specific xpcshell root manifests
(Assignee)

Comment 7

3 years ago
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.
Bug 1073639 covers the fxaccounts slowness, fwiw.
Blocks: 1073595
(Assignee)

Comment 9

3 years ago
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!
Attachment #8498238 - Attachment is obsolete: true
Attachment #8501077 - Flags: review?(cmanchester)
(Assignee)

Updated

3 years ago
Blocks: 1079278
services/fxaccounts/tests/xpcshell/test_client.js is coming back with a so far consistent timeout.
I don't really know how device manager works, but it looks like a mozprocess timeout, not an xpcshell timeout.
(Assignee)

Comment 12

3 years ago
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.
(Assignee)

Comment 13

3 years ago
(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
(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?
(Assignee)

Updated

3 years ago
Depends on: 1079405
(Assignee)

Updated

3 years ago
Depends on: 1073639
No longer depends on: 1079405
(Assignee)

Comment 15

3 years ago
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
Attachment #8501077 - Attachment is obsolete: true
Attachment #8501077 - Flags: review?(cmanchester)
Attachment #8501237 - Flags: review?(cmanchester)
(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.
(Assignee)

Comment 17

3 years ago
(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 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.
Attachment #8501237 - Flags: review?(cmanchester) → review+
(Assignee)

Comment 19

3 years ago
(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.
(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.
(Assignee)

Comment 21

3 years ago
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.
Attachment #8501501 - Flags: review+
(Assignee)

Comment 22

3 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/3d1a89fbcaa1
(Assignee)

Updated

3 years ago
Blocks: 1079651
https://hg.mozilla.org/mozilla-central/rev/3d1a89fbcaa1
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
https://hg.mozilla.org/releases/mozilla-aurora/rev/ed870ea3cdb8
status-b2g-v2.1: --- → fixed
status-b2g-v2.2: --- → fixed
status-firefox34: --- → fixed
status-firefox35: --- → fixed
Depends on: 1170772
You need to log in before you can comment on or make changes to this bug.