746915 - Move editor-related tests to editor/

Status: RESOLVED FIXED

(Core :: DOM: Editor, enhancement)

Description

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

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.

Comment 1

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

Attachment: Patch part 1, v1 -- Move editing-related tests to editor/ directories

Comment 2

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

Attachment: Patch part 2, v1 -- Move layout/reftests/editor/ to editor/reftests/

Comment 3

[Mozilla RelEng Bot]

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

[Mozilla RelEng Bot]

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/[e-mail]-8b75c3514ea3

Comment 5

[(no longer active)]

Comment on attachment 616495 [details] [diff] [review]
Patch part 1, v1 -- Move editing-related tests to editor/ directories

This is awesome!

Comment 6

[(no longer active)]

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.

Comment 7

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

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 8

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

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

Comment 9

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

Sorry -- which three lines?

Comment 10

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

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?

Comment 11

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

> 
> # editor/
>-include editor/reftest.list
>+include ../../editor/reftests/reftest.list

Comment 12

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

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!

Comment 13

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

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).

Comment 14

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

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.

Comment 15

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

ok, either way, but I'd prefer it at the bottom

Comment 16

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4bce31c59259
https://hg.mozilla.org/integration/mozilla-inbound/rev/42c372c2f749

Comment 17

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/4bce31c59259
https://hg.mozilla.org/mozilla-central/rev/42c372c2f749