Closed Bug 637660 Opened 9 years ago Closed 9 years ago

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

Categories

(Testing :: Reftest, defect)

ARM
Android
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: jmaher, Assigned: jmaher)

References

(Depends on 1 open bug)

Details

(Whiteboard: [mobile_unittests])

Attachments

(1 file, 2 obsolete files)

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.
OS: Linux → Android
Hardware: x86 → ARM
Whiteboard: [mobile_unittests]
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
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?
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?
Attachment #516131 - Flags: review?(dbaron) → review-
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)
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
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+
http://hg.mozilla.org/mozilla-central/pushloghtml?changeset=2269cb1136f9
Status: NEW → RESOLVED
Closed: 9 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.
Depends on: 647990
You need to log in before you can comment on or make changes to this bug.