Closed
Bug 977699
Opened 11 years ago
Closed 11 years ago
Move the few remaining mochitests to manifests
Categories
(Firefox Build System :: General, defect)
Firefox Build System
General
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla30
People
(Reporter: ted, Assigned: ted)
References
Details
Attachments
(1 file)
28.47 KB,
patch
|
jmaher
:
review+
|
Details | Diff | Splinter Review |
This gets everything but one chrome mochitest moved into a manifest. This patch has a few quirks, notably I decided to just use the existing make conditionals to conditionally use manifests instead of adding entries to mozinfo. This seemed reasonable given that the conditionals here cover one or two tests each.
A few of the things under testing/mochitest will wind up in slightly different places, I don't know if this will cause issues. I've pushed this patch (+ my other outstanding patches) to try:
https://tbpl.mozilla.org/?tree=Try&rev=ba2a7fc59025
Assignee | ||
Comment 1•11 years ago
|
||
Attachment #8383170 -
Flags: review?(jmaher)
Comment 2•11 years ago
|
||
Comment on attachment 8383170 [details] [diff] [review]
Move the few remaining mochitests to manifests
Review of attachment 8383170 [details] [diff] [review]:
-----------------------------------------------------------------
a few comments/questions. I really don't like the idea of using conditions in moz.build to filter out manifests, likewise the trend of creating >1 manifest/directory. I would like to have a followup bug to investigate a better solution to this via mozinfo + manifestdestiny. I know we are making 10 steps forward and I don't want to block on it.
::: content/base/test/moz.build
@@ +29,5 @@
> +# (see Bug 774939). App permission checks are also disabled on
> +# anything but B2G (Bug 900707).
> +if CONFIG['MOZ_CHILD_PERMISSIONS']:
> + MOCHITEST_MANIFESTS += [
> + 'mochitest-child-permissions.ini',
this doesn't do much to aid in our quest to see what tests are enabled/disabled.
::: testing/mochitest/moz.build
@@ +20,5 @@
> + 'dynamic/mochitest.ini',
> + 'MochiKit/mochitest.ini',
> + 'static/mochitest.ini',
> + 'tests/MochiKit-1.4.2/MochiKit/mochitest.ini',
> + 'tests/MochiKit-1.4.2/tests/mochitest.ini',
do these get installed to the proper location?
::: testing/mochitest/static/Makefile.in
@@ -4,5 @@
> -# file, You can obtain one at http://mozilla.org/MPL/2.0/.
> -
> -STATIC_FILES = test.template.txt \
> - xhtml.template.txt \
> - xul.template.txt \
I don't see the *.template.txt added to a .ini file
Attachment #8383170 -
Flags: review?(jmaher) → review+
Assignee | ||
Comment 3•11 years ago
|
||
(In reply to Joel Maher (:jmaher) from comment #2)
> a few comments/questions. I really don't like the idea of using conditions
> in moz.build to filter out manifests, likewise the trend of creating >1
> manifest/directory. I would like to have a followup bug to investigate a
> better solution to this via mozinfo + manifestdestiny. I know we are making
> 10 steps forward and I don't want to block on it.
I agree, I'm not wild about the way I implemented that. I don't think it's completely terrible though, since this is disabling tests by *feature*, not by platform. That is, these are tests for features that only make sense if the feature is enabled. I'm open to discussion about it though. The alternative is to stick every one of those conditionals into mozinfo, which is not super appealing, but not technically difficult.
> ::: testing/mochitest/moz.build
> @@ +20,5 @@
> > + 'dynamic/mochitest.ini',
> > + 'MochiKit/mochitest.ini',
> > + 'static/mochitest.ini',
> > + 'tests/MochiKit-1.4.2/MochiKit/mochitest.ini',
> > + 'tests/MochiKit-1.4.2/tests/mochitest.ini',
>
> do these get installed to the proper location?
The first three do (you'll note the manifests use absolute paths in support-files, which puts files in the right places). The last two go to a slightly different location, but I don't think that matters since they're not referenced from anywhere else. (I'm not even sure why those tests are in this directory, honestly.)
> ::: testing/mochitest/static/Makefile.in
> @@ -4,5 @@
> > -# file, You can obtain one at http://mozilla.org/MPL/2.0/.
> > -
> > -STATIC_FILES = test.template.txt \
> > - xhtml.template.txt \
> > - xul.template.txt \
>
> I don't see the *.template.txt added to a .ini file
Yeah, they're not actually used by anything, so I left them out.
Comment 4•11 years ago
|
||
if they are not used by anything, will they just be in the source tree and not the tests.zip?
Assignee | ||
Comment 5•11 years ago
|
||
Yes. I think that's fine. Their only use is with the gen_template.pl script, and they should work fine from there. There's no point in copying them around and into the test package.
Assignee | ||
Comment 6•11 years ago
|
||
These are looking good on try:
https://tbpl.mozilla.org/?tree=Try&rev=ba2a7fc59025 (the orange is from bug 948801).
Comment 7•11 years ago
|
||
could we land this patch? we are so close!
Assignee | ||
Comment 8•11 years ago
|
||
Comment 9•11 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
Updated•7 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•