Closed
Bug 420084
Opened 17 years ago
Closed 16 years ago
reftest should use nsIXULRuntime.widgetToolkit instead of autoconf.js
Categories
(Testing :: Reftest, defect)
Testing
Reftest
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla1.9.2a1
People
(Reporter: ted, Assigned: ted)
References
Details
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
dbaron
:
review+
|
Details | Diff | Splinter Review |
I don't think reftest needs anything other than the OS for skipping tests, am I right? It should just use nsIXULRuntime.OS instead, then we could get rid of autoconf.js. I'd like to see reftest packaged as an extension at some point, and this would be a necessary step.
xr = Components.classes["@mozilla.org/xre/app-info;1"].getService(Components.interfaces.nsIXULRuntime);
xr.OS
Assignee | ||
Comment 1•17 years ago
|
||
This isn't fantastic, since the values don't match, so you have to map them, but it works. Using this patch, I was able to manually package a reftest extension:
http://ted.mielczarek.org/code/mozilla/reftest.xpi
Assignee: nobody → ted.mielczarek
Status: NEW → ASSIGNED
Comment 2•16 years ago
|
||
At some point there's a pretty good chance we'll have a qt port, so we may well have tests that need to be marked failing on one of qt or gtk2. Likewise for conditioning failures on things like some of the new accelerated gfx backends. So we probably ought to have a way to query for a few of those things at runtime.
See also the patch in bug 448121, where I added the HTTP handler's variables.
Comment 3•16 years ago
|
||
The patch I mentioned in the previous comment is attachment 332597 [details] [diff] [review].
Updated•16 years ago
|
Component: Layout: Misc Code → Reftest
Product: Core → Testing
Version: Trunk → unspecified
Comment 4•16 years ago
|
||
So I think we probably ought to add xr.OS and xr.XPCOMABI like in attachment 332597 [details] [diff] [review], but also find a way to add the correct widget toolkit.
Then I think we should mass-change the manifests to switch to the new way, and then remove the autoconf.js stuff.
Assignee | ||
Comment 5•16 years ago
|
||
We could add a cut-down autoconf.js.in that just copies the values we need from autoconf.mk, and either have configure generate it at configure time, or the preprocessor at build time.
Assignee | ||
Updated•16 years ago
|
QA Contact: layout.misc-code → reftest
Assignee | ||
Comment 7•16 years ago
|
||
I'm adding nsIXULRuntime.widgetToolkit in bug 475811, so we can use that and not have to change any test manifests.
Summary: reftest should use nsIXULRuntime.OS instead of autoconf.js → reftest should use nsIXULRuntime.widgetToolkit instead of autoconf.js
Comment 8•16 years ago
|
||
We should probably expose OS, widgetToolkit, and XPCOMABI, since we might have things that depend on them (e.g., tests that fail in the x86-assembly version only). And I think we probably want to do it in a way such that we do change manifiests, since MOZ_WIDGET_TOOLKIT is really the Makefile-ish way of looking at what we want. So maybe just add widgetToolkit to what I said in comment 4?
Assignee | ||
Comment 9•16 years ago
|
||
Ok, this gets rid of autoconf.js completely, and uses the widgetToolkit property I added to nsIXULRuntime to populate MOZ_WIDGET_TOOLKIT in the sandbox. It also adds (as dbaron suggested), a xulRuntime object in the sandbox, with OS, XPCOMABI, and widgetToolkit populated directly from the xul runtime object. I've added sanity tests for xulRuntime.OS. They're not super pretty, but they were the simplest thing I could think of.
Attachment #306286 -
Attachment is obsolete: true
Attachment #359524 -
Flags: review?(dbaron)
Comment 10•16 years ago
|
||
Comment on attachment 359524 [details] [diff] [review]
ditch autoconf.js, use nsIXULRuntime.widgetToolkit
r=dbaron, although I think we need to come up with something better than fails-if() for tests like this (i.e., something that doesn't count as failing for the other platforms).
Attachment #359524 -
Flags: review?(dbaron) → review+
Assignee | ||
Comment 11•16 years ago
|
||
What's the impetus?
Comment 12•16 years ago
|
||
Not adding to the "known fails" total when it's actually the right result.
Assignee | ||
Comment 13•16 years ago
|
||
Ah. Should I be using skip-if instead, or do you really want something semantically better?
Comment 14•16 years ago
|
||
I want something semantically better, and where we still run the test when it's supposed to fail.
Assignee | ||
Comment 15•16 years ago
|
||
Filed bug 476642 on sorting that out.
Assignee | ||
Comment 16•16 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Updated•16 years ago
|
Whiteboard: [needs 1.9.1 landing]
Target Milestone: --- → mozilla1.9.2a1
Version: unspecified → Trunk
Assignee | ||
Comment 17•16 years ago
|
||
This isn't going to land on 1.9.1, as it depends on the nsIXULRuntime patch, which isn't going to land on 1.9.1. It's not really necessary for the runreftest.py patch, it's just necessary for enabling tests on the build machines.
Updated•16 years ago
|
Whiteboard: [needs 1.9.1 landing]
You need to log in
before you can comment on or make changes to this bug.
Description
•