Closed
Bug 746915
Opened 12 years ago
Closed 12 years ago
Move editor-related tests to editor/
Categories
(Core :: DOM: Editor, enhancement)
Core
DOM: Editor
Tracking
()
RESOLVED
FIXED
mozilla15
People
(Reporter: ayg, Assigned: ayg)
Details
Attachments
(2 files)
6.60 KB,
patch
|
ehsan.akhgari
:
review+
|
Details | Diff | Splinter Review |
34.18 KB,
patch
|
dbaron
:
review+
|
Details | Diff | Splinter Review |
We have some editing-related mochitests in content/html/document/test/, and the reftests are mostly under layout/ for some reason (see bug 279330 comment 26). At the very least, we should move all the editing-related mochitests to editor/. I don't see why we shouldn't move layout/reftests/editor/ to editor/reftests/ too -- that way to run all editor-related tests is just TEST_PATH=editor make -C $(OBJDIR) mochitest-plain TEST_PATH=editor make -C $(OBJDIR) crashtest TEST_PATH=editor make -C $(OBJDIR) reftest But I'll write that as a second, separate patch in case you'd prefer to keep them under layout/ for some reason.
Assignee | ||
Comment 1•12 years ago
|
||
Attachment #616495 -
Flags: review?(ehsan)
Assignee | ||
Comment 2•12 years ago
|
||
Attachment #616496 -
Flags: review?(ehsan)
Assignee | ||
Updated•12 years ago
|
Whiteboard: [autoland-try:-u all]
Updated•12 years ago
|
Whiteboard: [autoland-try:-u all] → [autoland-in-queue]
Comment 3•12 years ago
|
||
Autoland Patchset: Patches: 616495, 616496 Branch: mozilla-central => try Destination: http://hg.mozilla.org/try/pushloghtml?changeset=8b75c3514ea3 Try run started, revision 8b75c3514ea3. To cancel or monitor the job, see: https://tbpl.mozilla.org/?tree=Try&rev=8b75c3514ea3
Comment 4•12 years ago
|
||
Try run for 8b75c3514ea3 is complete. Detailed breakdown of the results available here: https://tbpl.mozilla.org/?tree=Try&rev=8b75c3514ea3 Results (out of 220 total builds): success: 182 warnings: 38 Builds (or logs if builds failed) available at: http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/autolanduser@mozilla.com-8b75c3514ea3
Updated•12 years ago
|
Whiteboard: [autoland-in-queue]
Comment 5•12 years ago
|
||
Comment on attachment 616495 [details] [diff] [review] Patch part 1, v1 -- Move editing-related tests to editor/ directories This is awesome!
Attachment #616495 -
Flags: review?(ehsan) → review+
Comment 6•12 years ago
|
||
Comment on attachment 616496 [details] [diff] [review] Patch part 2, v1 -- Move layout/reftests/editor/ to editor/reftests/ I'm not sure how much we care about putting most reftests under layout. There are components which do this, but I always thought that the idea is to keep reftests close to each other. If dbaron is fine with this, I'm fine as well.
Attachment #616496 -
Flags: review?(ehsan) → review?(dbaron)
Assignee | ||
Comment 7•12 years ago
|
||
I can find content/html/content/reftests content/html/document/reftests content/test/reftest dom/plugins/test/reftest gfx/tests/reftest image/test/reftest netwerk/test/reftest parser/htmlparser/tests/reftest toolkit/content/tests/reftests toolkit/themes/pinstripe/reftests widget/reftests Seems pretty popular. I think it's quite surprising to have tests unrelated to layout under layout/.
Comment on attachment 616496 [details] [diff] [review] Patch part 2, v1 -- Move layout/reftests/editor/ to editor/reftests/ >diff --git a/layout/reftests/reftest.list b/layout/reftests/reftest.list >--- a/layout/reftests/reftest.list >+++ b/layout/reftests/reftest.list >@@ -127,17 +127,17 @@ include counters/reftest.list > > # datalist > include datalist/reftest.list > > # dom/ > include dom/reftest.list > > # editor/ >-include editor/reftest.list >+include ../../editor/reftests/reftest.list > > # generated-content/ > include generated-content/reftest.list > > # first-letter/ > skip-if(Android) include first-letter/reftest.list > > # first-line/ Please move these 3 lines lower, someplace appropriate near the bottom of the list so it stays sorted reasonably. r=dbaron with that
Attachment #616496 -
Flags: review?(dbaron) → review+
Assignee | ||
Comment 9•12 years ago
|
||
Sorry -- which three lines?
Assignee | ||
Comment 10•12 years ago
|
||
Comment on attachment 616496 [details] [diff] [review] Patch part 2, v1 -- Move layout/reftests/editor/ to editor/reftests/ Sorry, I didn't understand your review comment -- which three lines?
Attachment #616496 -
Flags: review+ → review?(dbaron)
>
> # editor/
>-include editor/reftest.list
>+include ../../editor/reftests/reftest.list
Attachment #616496 -
Flags: review?(dbaron) → review+
Assignee | ||
Comment 12•12 years ago
|
||
Okay, so I still don't understand -- how is it not sorted reasonably? editor/ comes after dom/ and before generated-content/, which is roughly where it should go. If anything, generated-content/ should be moved down between forms and gfx. Where do you want me to move the lines? Sorry for being slow!
Oh, the stuff that begins with ".." is down at the bottom, separate from the subdirectories of layout/reftests/ which are at the top (and where it's currently listed).
Assignee | ||
Comment 14•12 years ago
|
||
I don't see them at the bottom. I see them sorted alphabetically. Like: # columns/ include columns/reftest.list # content/ include ../../content/test/reftest/reftest.list # counters/ include counters/reftest.list ... # forms skip-if(Android) include forms/reftest.list # gfx include ../../gfx/tests/reftest/reftest.list # block-inside-inline splits include ib-split/reftest.list ... # native-theme/ skip-if(Android) include native-theme/reftest.list # netwerk/ include ../../netwerk/test/reftest/reftest.list # object/ include object/reftest.list # ogg-video/ include ogg-video/reftest.list # webm-video/ include webm-video/reftest.list # parser/ include ../../parser/htmlparser/tests/reftest/reftest.list # percent-overflow-sizing/ include percent-overflow-sizing/reftest.list # pixel-rounding/ include pixel-rounding/reftest.list # plugin/ include ../../dom/plugins/test/reftest/reftest.list # printing include printing/reftest.list include pagination/reftest.list ... # text-shadow/ skip-if(Android) include text-shadow/reftest.list # theme (pinstripe) include ../../toolkit/themes/pinstripe/reftests/reftest.list # text-transform/ include text-transform/reftest.list ... # unicode/ (verify that we don't do expend effort doing unicode-aware case checks) include unicode/reftest.list # widget/ include ../../widget/reftests/reftest.list # xml-stylesheet/ include ../../content/test/reftest/xml-stylesheet/reftest.list # xul-document-load/ include xul-document-load/reftest.list # xul/ include xul/reftest.list # xul include ../xul/base/reftest/reftest.list # xul grid include ../xul/base/src/grid/reftests/reftest.list # z-index/ include z-index/reftest.list The only paths starting ../ that aren't in some type of alphabetical order are ../../image/test/reftest/reftest.list at the very top, and ../../content/html/{document,content}/reftests/reftests.list at the very bottom. All the others are alphabetical, as in this patch.
ok, either way, but I'd prefer it at the bottom
Assignee | ||
Comment 16•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/4bce31c59259 https://hg.mozilla.org/integration/mozilla-inbound/rev/42c372c2f749
Flags: in-testsuite-
Target Milestone: --- → mozilla15
Comment 17•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/4bce31c59259 https://hg.mozilla.org/mozilla-central/rev/42c372c2f749
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•