Closed Bug 846090 Opened 7 years ago Closed 6 years ago

XBL mochitests timing out/failing on b2g

Categories

(Testing :: General, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
mozilla26

People

(Reporter: martijn.martijn, Assigned: martijn.martijn)

References

()

Details

Attachments

(1 file, 8 obsolete files)

I noticed that a lot (all?) of the mochitests involving xbl are failing.
So I tested one xbl mochitest, added this line:
SpecialPowers.addPermission("allowXULXBL", true, document);
And then the test would not time out anymore.

So for some reason, the allowXULXBL permission isn't set.
The locations that are being set is done here:
http://mxr.mozilla.org/mozilla-central/source/build/automation.py.in#434
Those locations are being read from here:
http://mxr.mozilla.org/mozilla-central/source/build/automation.py.in#216

server-locations.txt can be found here:
http://mxr.mozilla.org/mozilla-central/source/build/pgo/server-locations.txt

It looks like runtestsb2g.py doesn't call this code at all:
http://mxr.mozilla.org/mozilla-central/source/testing/mochitest/runtestsb2g.py#431
While it does so on B2GDesktop:
http://mxr.mozilla.org/mozilla-central/source/testing/mochitest/runtestsb2g.py#546
Attached patch tried this (obsolete) — Splinter Review
I tried something like this, but it didn't seem to solve the problem.
Martijn, could you possibly be hitting the issue in bug https://bugzilla.mozilla.org/show_bug.cgi?id=685652?
Mochitest is run inside a Gaia app with these permissions:  https://github.com/mozilla-b2g/gaia/blob/master/test_apps/test-container/manifest.webapp

If the container app lacks a permission, there is no way to add it from within mochitest.  We could try adding allowXULXBL to the app's manifest and see if that helps things.
Isn't that part of class B2GDesktopMochitest?
http://mxr.mozilla.org/mozilla-central/source/testing/mochitest/runtestsb2g.py#508

Isn't this code called instead?
http://mxr.mozilla.org/mozilla-central/source/testing/mochitest/runtestsb2g.py#431

(In reply to Jonathan Griffin (:jgriffin) from comment #4)
> We do call initalizeProfile for B2G mochitests, here:
> 
> http://mxr.mozilla.org/mozilla-central/source/build/mobile/b2gautomation.
> py#116
> 
> called from 
> 
> http://mxr.mozilla.org/mozilla-central/source/testing/mochitest/runtestsb2g.
> py#547
Yes, you're right.  For emulator tests we actually call buildProfile that's in runtests.py here:

http://mxr.mozilla.org/mozilla-central/source/testing/mochitest/runtestsb2g.py#434

and that calls initializeProfile.
I should also add that for B2G we don't care about XUL tests, since there are no XUL widgets in B2G.  I think we can safely exclude these tests.
Depends on: 848411
(In reply to Jonathan Griffin (:jgriffin) from comment #3)
> If the container app lacks a permission, there is no way to add it from
> within mochitest.  We could try adding allowXULXBL to the app's manifest and
> see if that helps things.

I tried that, that didn't seem to work.
From irc:
[22:10]	<gwagner>	mw22: the appId has to match. otherwise we don't allow any permission
[22:11]	<gwagner>	mw22: I see that you always use appId 0

my question, how can you know the appId?
[23:00]	<fabrice>	mw22: it can ba anything, but it has to match what's in the webapps.json
The appId is here inserted in permissions.sqlite database:
http://mxr.mozilla.org/mozilla-central/source/build/automation.py.in#292
It's always 0, which isn't correct, but I have no idea how to get the appId of the mochitest app.
I did this code while running a mochitest:
let appsSvc = SpecialPowers.Cc["@mozilla.org/AppsService;1"].getService(SpecialPowers.Ci.nsIAppsService);
let appId = appsSvc.getAppLocalIdByManifestURL('http://mochi.test:8888');

This returns '0', but that can't be the appId of the mochitest app, since '0' doesn't work.
Ah! Never mind, I should have used:
let appId = appsSvc.getAppLocalIdByManifestURL('http://mochi.test:8888/manifest.webapp');

This gives me '39'.

So I guess I could use that magic number, but that is probably not the best thing to do.
B2G mochitests are being updated to use mozbase instead of automation.py.in, so the code that handles profile creation for them is currently at https://github.com/mozilla/mozbase/blob/master/mozprofile/mozprofile/profile.py#L77.

If you moved the webapp manifest updating so that it occurred before the permissions writing, you could then use self.webapps to find the id for the mochitest app.

Unfortunately, we are not using in-tree mozprofile for running B2G mochitests yet, so you wouldn't be able to validate this with a try run.  However, if it worked locally, you could submit a patch to mozbase and we could push that to our internal pypi server that is used to retrieve mozprofile during B2G mochitest runs.

Or, we could wait until we use in-tree mozprofile for B2G mochitests.
I guess that's bug 688667?
Is there a (easy) way to use mozbase instead of automation.py.in, locally?
(In reply to Martijn Wargers [:mw22] (QA - IRC nick: mw22) from comment #15)
> I guess that's bug 688667?
> Is there a (easy) way to use mozbase instead of automation.py.in, locally?

If you are using mozilla-central, you are using mozbase instead of automation.py. In that case the allowXULXBL permission should be getting set here: https://github.com/mozilla/mozbase/blob/master/mozprofile/mozprofile/permissions.py#L249

To make sure you have the required packages you can just cd testing/marionette/client && python setup.py install
If you're wanting to play with mozprofile and see the effects in B2G mochitest, you should instead set up a virtualenv, and then run 'python setup.py develop' from testing/mozbase/mozprofile, before you run setup.py develop from testing/marionette.

Then, you can modify the contents of testing/mozbase/mozprofile, and they will automatically get used when you run mochitests.
This would allow removing the following entries in the exclude list of b2g.json:
content/base/test/test_base.xhtml
content/base/test/test_bug330925.xhtml
content/base/test/test_bug372086.html
content/base/test/test_bug419527.xhtml
content/base/test/test_bug444030.xhtml
content/events/test/test_bug391568.xhtml
layout/inspector/tests/test_bug522601.xhtml
layout/style/test/test_selectors_on_anonymous_content.html
Attached patch workaround patch (obsolete) — Splinter Review
Sorry, I don't have the knowledge currently to carry out what is suggested in comment 16 and comment 17.
Is this acceptable for now as a workaround?

Please ignore the file permission changes in this patch, there is something weird going on on my Ubuntu vmware environment with random files getting readonly.
Attachment #719255 - Attachment is obsolete: true
Attachment #722804 - Attachment is obsolete: true
Attachment #780625 - Flags: review?(jgriffin)
Comment on attachment 780625 [details] [diff] [review]
workaround patch

I'd rather not put "random" settings in harness files, as it defeats the purpose of what we're trying to do by consolidating the settings.

Ahal, where would be the right place to put this?  In https://github.com/mozilla/mozbase/blob/master/mozprofile/mozprofile/permissions.py#L249, what is 'locations' for mochitests?  Is it some server-locations.txt file, or something else?
Attachment #780625 - Flags: review?(jgriffin)
Attachment #780625 - Flags: review-
Attachment #780625 - Flags: feedback?(ahalberstadt)
Ok, fair enough, it would be a workaround anyway.
Actually, I think these tests could be adjusted to check if the getAnonymousNodes method exists and then skip that xbl related part of the test if xbl is turned off.
I think the mochitest suite in itself should not expect that a browser supports (mozilla's version of) xbl (unless when testing the content/xbl subdiretory).
Yes, that would be reasonable as well.  I don't think XBL tests are particularly relevant for B2G anyway.
Just filed bug 898140 for moving xbl/xul mochitests to content/xul content/xbl directories.
Comment on attachment 780625 [details] [diff] [review]
workaround patch

For reftests we do it here: http://mxr.mozilla.org/mozilla-central/source/layout/tools/reftest/b2g_start_script.js#33

Still kind of hacky I guess. The mochitest version of that script is inside runtestsb2g.py, but if you wait a day or two, it'll be moved to a separate file.
Attachment #780625 - Flags: feedback?(ahalberstadt)
Also yes, a location is a single line in a server-locations.txt file.
Assignee: nobody → martijn.martijn
Attached patch Failed attempt (obsolete) — Splinter Review
This is a failed attempt at doing as what was suggested.
Obviously, it isn't working, because I don't really know what I'm doing, but perhaps there is something in there that is in the right direction.
Attached patch Failed attempt nr. 2 (obsolete) — Splinter Review
With some logging here, When I run this, I don't see any mentioning of 'mochi.test' in the webapps list. The 'mochi.test' app is what I need the appId for, to write to the permissions database.
Attachment #789947 - Attachment is obsolete: true
I was under the impression that XBL tests are not relevant for B2G; is that not the case?  If they aren't, we shouldn't spend any time trying to fix them.
That might be, I don't know.
But there are some test files that just depend for a small part on xbl.
I'll try to find time to run this patch and see what's happening, but I'm not sure it will be this week; the remainder of the week is very busy!
Attached patch b2g_start_script.diff (obsolete) — Splinter Review
Another failed attempt, I tried this in b2g_start_script.js, which seems to work for reftest. But for some reason, this hasn't any effect for mochitest.
Ok, reftest doesn't run oop, so perhaps that's the reason it's working there, but not for mochitest.
Attached patch b2g_start_script.diff (obsolete) — Splinter Review
Ok, this is something that seems to be working. This could be an acceptable workaround, right? The code inside the loadFrameScript call is:
addEventListener("DOMWindowCreated", function(e) {
  removeEventListener("DOMWindowCreated", arguments.callee, false);
  var window = e.target.defaultView;
  var msg = {
      'op': 'add',
      'type': 'allowXULXBL',
      'permission': 1,
      'url': 'http://mochi.test:8888',
      'appId': window.document.nodePrincipal.appId,
      'isInBrowserElement': false
  };
  sendSyncMessage('SPPermissionManager', msg);
}, false);
Attachment #792417 - Flags: review?
Attached patch b2g_start_script.diff (obsolete) — Splinter Review
Ok, this is working too, thanks for Olli Pettay that explained to me to use wrappedJSObject for this (in fact, iirc, I have to use this in one of my other patches, perhaps).
This is the source of the loadFrameScript workaround:
addEventListener("DOMWindowCreated", function(e) {
  removeEventListener("DOMWindowCreated", arguments.callee, false);
  var window = e.target.defaultView;
  window.wrappedJSObject.SpecialPowers.addPermission("allowXULXBL", true, window.document);
}
Attachment #792810 - Flags: review?(jgriffin)
Comment on attachment 792810 [details] [diff] [review]
b2g_start_script.diff

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

This looks fine if a try run (for all B2G tests) is green
Attachment #792810 - Flags: review?(jgriffin) → review+
I talked to jgriffin on irc and he agreed on adding the comment that I added in the patch.
It basically explains this is a workaround for bug 848411.

Tryserver was all green, so this can be checked in.
Attachment #780625 - Attachment is obsolete: true
Attachment #790182 - Attachment is obsolete: true
Attachment #790687 - Attachment is obsolete: true
Attachment #792417 - Attachment is obsolete: true
Attachment #792810 - Attachment is obsolete: true
Attachment #792417 - Flags: review?
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/2c5c0e94bf7c
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
Depends on: 908436
You need to log in before you can comment on or make changes to this bug.