The default bug view has changed. See this FAQ.

Move editor-related tests to editor/

RESOLVED FIXED in mozilla15

Status

()

Core
Editor
--
enhancement
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: ayg, Assigned: ayg)

Tracking

Trunk
mozilla15
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

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.
Created attachment 616495 [details] [diff] [review]
Patch part 1, v1 -- Move editing-related tests to editor/ directories
Attachment #616495 - Flags: review?(ehsan)
Created attachment 616496 [details] [diff] [review]
Patch part 2, v1 -- Move layout/reftests/editor/ to editor/reftests/
Attachment #616496 - Flags: review?(ehsan)
Whiteboard: [autoland-try:-u all]

Updated

5 years ago
Whiteboard: [autoland-try:-u all] → [autoland-in-queue]

Comment 3

5 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

5 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

5 years ago
Whiteboard: [autoland-in-queue]
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 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)
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+
Sorry -- which three lines?
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+
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).
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
https://hg.mozilla.org/integration/mozilla-inbound/rev/4bce31c59259
https://hg.mozilla.org/integration/mozilla-inbound/rev/42c372c2f749
Flags: in-testsuite-
Target Milestone: --- → mozilla15
https://hg.mozilla.org/mozilla-central/rev/4bce31c59259
https://hg.mozilla.org/mozilla-central/rev/42c372c2f749
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.