add sandbox.Android, and skip a few crashtests so we can run green on Android

RESOLVED FIXED

Status

RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: jmaher, Assigned: jmaher)

Tracking

(Depends on: 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [mobile_unittests])

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

8 years ago
Created attachment 515913 [details] [diff] [review]
skip-if(xulRuntime.OS=="Android") for crashtests.list that hang/fail (1.0)

this patch ends up doing a skip-if(xulRuntime.OS=="Android") on all hangs and failures.  There are 5 failures that don't need a skip-if, but a "fails-if(..) load filename.html" results in a manifest parsing error.
(Assignee)

Updated

8 years ago
OS: Linux → Android
Hardware: x86 → ARM
Whiteboard: [mobile_unittests]
(Assignee)

Comment 1

8 years ago
Comment on attachment 515913 [details] [diff] [review]
skip-if(xulRuntime.OS=="Android") for crashtests.list that hang/fail (1.0)

for remote testing, we don't really support .xul (it doesn't load over http).  That is the majority of the changes here for crashtests.
Attachment #515913 - Flags: review?(mark.finkle)
Attachment #515913 - Flags: feedback?(dbaron)
For pedantry's sake, you should use skip-if for tests that hang or crash, and fails-if for tests that fail. (skipped tests don't get run at all, fail tests will still run, which is nice).
Assignee: nobody → jmaher
(Assignee)

Comment 3

8 years ago
yes, I use skip-if for all the .xul files which hang and tried to use fails-if, but I ran into manifest errors when using a 'fails-if(...) load filename'.  So there are some comments in those instances indicating it should be a fails-if instead of a skip-if.
Attachment #515913 - Flags: review?(mark.finkle) → review+
Here:
http://hg.mozilla.org/mozilla-central/file/372eca6704c2/layout/tools/reftest/reftest.js#l404
you could add a line saying:
  sandbox.Android = xr.OS == "Android";

and then you could just write all of this as skip-if(Android), fails-if(Android), etc., which would be preferable.


Don't we flip the remote XUL pref while running reftest and crashtest?  Is that insufficient for loading over HTTP?  Or is the mobile harness just not doing that correctly?
(Assignee)

Comment 5

8 years ago
Created attachment 516131 [details] [diff] [review]
skip-if(Android) for tests that hang (2.0)

updated to include a sandbox.Android as suggested in previous comment.  Also updated patch to register a remoteXUL instance for a webserver when we are accessing a manifest via http[s].  This works great and only 2 crashtests need to be skip-if to run clean on android tegra.
Attachment #515913 - Attachment is obsolete: true
Attachment #515913 - Flags: feedback?(dbaron)
Attachment #516131 - Flags: review?(dbaron)
Comment on attachment 516131 [details] [diff] [review]
skip-if(Android) for tests that hang (2.0)

I'd somewhat prefer not to have the permission manager stuff in reftest.js, which I'd prefer not to mess with profiles or mess with them as little as possible.

We already have stuff to set up the appropriate permissions in automation.py and runreftest.py:
http://mxr.mozilla.org/mozilla-central/search?string=setuppermissions
Maybe remotereftest.py needs similar code?
(Assignee)

Comment 7

8 years ago
Created attachment 516283 [details] [diff] [review]
skip-if(Android) for tests that hang (3.0)

thanks for the pointer to how this is done in automation.py.  I wasn't aware we had added support for permissions.sqlite entries.  I was able to do this and have a working patch that does the trick.
Attachment #516131 - Attachment is obsolete: true
Attachment #516283 - Flags: review?(dbaron)
(Assignee)

Updated

8 years ago
Summary: fix crashtests.list files to ignore hangs and failures on Android → add sandbox.Android, and skip a few crashtests so we can run green on Android
(Assignee)

Comment 8

8 years ago
this patch is still blocking us from running crashtests on android.
Comment on attachment 516283 [details] [diff] [review]
skip-if(Android) for tests that hang (3.0)

r=dbaron.  Sorry for the delay.
Attachment #516283 - Flags: review?(dbaron) → review+
(Assignee)

Comment 10

8 years ago
http://hg.mozilla.org/mozilla-central/pushloghtml?changeset=2269cb1136f9
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
Have bugs been filed to re-enable these test? If not, please do. In any case, the manifests should be annotated.

Updated

8 years ago
Depends on: 644697

Updated

8 years ago
Depends on: 647990
You need to log in before you can comment on or make changes to this bug.