898140 - Move some mochitests that use XUL/XBL to their specific XUL/XBL test subdirectories

Status: RESOLVED INVALID

(Core :: DOM: Core & HTML, defect)

Description

[Martijn Wargers (dead)]

There are a bunch of mochitests that use XUL/XBL that don't reside in content/xul/ or layout/xul/ or content/xbl/ subdirectories.
It seems to me that it would be best if those tests would move to those directories.

The reason is that XUL and XBL are Mozilla only technologies (which are going away, fwiu).  

Some files where this is the case (probably not all of them):
layout/style/test/media_queries_dynamic_xbl_iframe.html
layout/style/test/test_media_queries_dynamic_xbl.html 
content/base/test/test_xbl_userdata.xhtml 
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
dom/tests/mochitest/general/test_focusrings.xul
layout/base/tests/test_bug465448.xul

Agree/disagree?

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

There's an advantage to keeping the tests near the code most likely to break the tests -- developers often run the "local" tests locally (e.g., I run layout/style/ mochitests frequently when changing any code in layout/style) for quick feedback before submitting patches to the try server.

Comment 2

[Martijn Wargers (dead)]

Ok, fair enough.

I mainly thought about this in order to remove those entries from the b2g.json file (b2g mochitest currently don't support XUL/XBL).

Could I add some code to tests that use XBL that checks whether XBL is enabled/supported? 
I know to check for this, using SpecialPowers.hasPermission('allowXULXBL', document);
But is there perhaps a regular dom method that can check if XBL is working?

That would leave the 2 .xul files. I think those could be moved to chrome tests.
layout/base/tests/test_bug465448.xul could be moved to layout/base/tests/chrome/test_bug465448.xul
dom/tests/mochitest/general/test_focusrings.xul could be moved to dom/tests/mochitest/chrome/test_focusrings.xul

Is that ok?

Comment 3

[Geo Mealer [:geo] -- This account is inactive after 2015-07-07]

Worth noting that when you have *some* tests in separate subdirs according to criteria like "uses XUL", it implies an organization and leads you to believe that tests that aren't there don't meet the criteria. Rogues become more confusing.

I get David's point, but would be nice if at least the naming of the tests made that dependency more transparent. I'd have never assumed "content/base/test/test_bug372086.html " used XUL or XBL when there directories right there too.

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

The organization of the tests is more about their main focus.  A majority of our tests test the HTML parser in some way, but there's only a small set that focus on the HTML parser.

Comment 5

[Martijn Wargers (dead)]

Attachment: test_xbl_userdata.diff

Would something like this be acceptable for the tests that have xbl in them, that don't reside in content/xbl?

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

That doesn't seem great; if that test fails, we'll just stop running any tests.

What's the rationale for this?  In other words, why is it worth spending time on rather than other things?

I think if we want to write cross-browser tests, they should be using testharness.js, not mochitest.

Comment 7

[Martijn Wargers (dead)]

To support gecko browsers that don't support XBL, I guess (which is actually the case for Firefox itself, except in mochitest it's enabled globally).

Comment 8

[Martijn Wargers (dead)]

Attachment: move_xul_to_chrome.diff

Ok, I'll leave the xbl tests as they are.

But is changing the xul tests to mochitest-chrome ok? Fennec and b2g don't support xul and these seem one of the few (only?) mochitests that are still xul based.
I also changed test_selection_expanding.html to mochi-test chrome, because this is also working for desktop only.

Comment 9

[Martijn Wargers (dead)]

Attachment: move_xul_to_chrome.diff

With removal in b2g.json, android.json.