This covers a good chunk of what's left, I was working from the list in bug 920185 comment 1. I need to run this by try after I sort out an issue with one of the other patches in my queue.

Comment on attachment 8371516 [details] [diff] [review] Move more Mochitests into manifests Review of attachment 8371516 [details] [diff] [review]: ----------------------------------------------------------------- did a drive by here- this is looking pretty good in general- thanks for the work on this ted!

A few additions.

Okay, this covers just about everything. I need to run this past the tryserver still. I compared the contents of $objdir/_tests/testing/mochitest before and after this patch, and on my Linux build the only differences were the added manifests + some support-files that I put into DEFAULT and so got copied even if the tests in that directory were disabled. jmaher asked (on IRC) about the removal of browser/base/content/test/general/Makefile.in in this patch. It turns out that this Makefile had been previously removed in bug 932147, it must have been reverted in a bad merge: http://hg.mozilla.org/mozilla-central/rev/dfe65b31de6a There are two additional patches I'll upload shortly that I wanted separated out for clarity, but they can be folded back in for landing.

This is toolkit/components/places/tests/browser, which has a bunch of files that get installed from subdirectories. It turns out that the MOCHITEST*_FILES rules and the rules in manifests are not compatible in this regard--the former simply copies the leaf filename to the $relativesrcdir target directory, whereas the latter preserves the relative path. I chose to simply hg mv the files from subdirs up a level to avoid having to change test files to match.

toolkit/component/places/tests/mochitest had the same problem, applied the same solution.

Comment on attachment 8373441 [details] [diff] [review] places/tests/browser oh cool!

Comment on attachment 8373438 [details] [diff] [review] Move more Mochitests into manifests Review of attachment 8373438 [details] [diff] [review]: ----------------------------------------------------------------- patch in general is great, a few questions ::: content/canvas/test/mochitest.ini @@ +153,5 @@ > +[test_2d.gradient.radial.outside3.html] > +skip-if = toolkit != 'cocoa' > +# These tests only pass on Mac OS X >= 10.5; see bug 450114 > +[test_2d.gradient.radial.touch1.html] > +disabled = bug 450114 is bug 450114 a comment? I am unfamiliar with the disabled syntax here. @@ +179,5 @@ > +skip-if = toolkit != 'cocoa' > +# This test is bogus according to the spec; see bug 407107 > +[test_2d.path.rect.zero.6.html] > +disabled = bug 407107 > +skip-if = toolkit != 'cocoa' this appears to be disabled and skipped on cocoa? that is confusing ::: docshell/test/navigation/mochitest.ini @@ +20,5 @@ > parent.html > > [test_bug13871.html] > +[test_bug270414.html] > +skip-if = buildapp == "mobile/android" could we just use toolkit=="android"? ::: toolkit/components/places/tests/Makefile.in @@ -12,3 @@ > # Simple MochiTests > MOCHITEST_FILES = \ > mochitest/bug94514-postpage.html \ we still have mochitest and chrome files in here ::: toolkit/components/places/tests/mochitest/prompt_common.js @@ -1,1 @@ > - why is this file removed? I believe it is still used!

(In reply to Joel Maher (:jmaher) from comment #8) > Comment on attachment 8373438 [details] [diff] [review] > Move more Mochitests into manifests > > Review of attachment 8373438 [details] [diff] [review]: > ----------------------------------------------------------------- > > patch in general is great, a few questions > > ::: content/canvas/test/mochitest.ini > @@ +153,5 @@ > > +[test_2d.gradient.radial.outside3.html] > > +skip-if = toolkit != 'cocoa' > > +# These tests only pass on Mac OS X >= 10.5; see bug 450114 > > +[test_2d.gradient.radial.touch1.html] > > +disabled = bug 450114 > > is bug 450114 a comment? I am unfamiliar with the disabled syntax here. The value of the disabled key is ignored, so I just used it as a convenient place to put the reason. There are a few instances of this in the tree already, although it's not widespread: http://mxr.mozilla.org/mozilla-central/search?string=disabled+%3D&find=\.ini&findi=&filter=^[^\0]*%24&hitlimit=&tree=mozilla-central I like it better than comments, since it's actually data in the manifest, but we don't have anything comparable for skip-if disablement, which is a bummer. > > @@ +179,5 @@ > > +skip-if = toolkit != 'cocoa' > > +# This test is bogus according to the spec; see bug 407107 > > +[test_2d.path.rect.zero.6.html] > > +disabled = bug 407107 > > +skip-if = toolkit != 'cocoa' > > this appears to be disabled and skipped on cocoa? that is confusing Hm, I thought this was inside the OSX block in the Makefile and disabled in there, but apparently not. I'll remove the skip-if. I've been trying to port the conditionals from Makefiles as closely as possible, but I must have confused myself. > > ::: docshell/test/navigation/mochitest.ini > @@ +20,5 @@ > > parent.html > > > > [test_bug13871.html] > > +[test_bug270414.html] > > +skip-if = buildapp == "mobile/android" > > could we just use toolkit=="android"? We could, I've just been porting exactly what the Makefiles say right now. (Even if they're doing silly things.) > > ::: toolkit/components/places/tests/Makefile.in > @@ -12,3 @@ > > # Simple MochiTests > > MOCHITEST_FILES = \ > > mochitest/bug94514-postpage.html \ > > we still have mochitest and chrome files in here Those changes are in attachment 8373442 [details] [diff] [review]. > > ::: toolkit/components/places/tests/mochitest/prompt_common.js > @@ -1,1 @@ > > - > > why is this file removed? I believe it is still used! Not AFAICT: http://mxr.mozilla.org/mozilla-central/search?string=prompt_common.js There are other files in the tree named prompt_common.js, and those are used: http://mxr.mozilla.org/mozilla-central/find?text=&kind=text&string=prompt_common.js

I pushed these to try a few days ago, the results look good: https://tbpl.mozilla.org/?tree=Try&rev=4ded4d2dc418

Followup bustage fix, wound up conflicting with bug 969021 in a way that didn't break my local Linux build: https://hg.mozilla.org/integration/mozilla-inbound/rev/b2dd850b0ca3

https://hg.mozilla.org/mozilla-central/rev/acefe67f49db https://hg.mozilla.org/mozilla-central/rev/b2dd850b0ca3