Last Comment Bug 746915 - Move editor-related tests to editor/
: Move editor-related tests to editor/
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Editor (show other bugs)
: Trunk
: All All
: -- enhancement (vote)
: mozilla15
Assigned To: Aryeh Gregor (:ayg) (away until October 25)
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-04-19 01:34 PDT by Aryeh Gregor (:ayg) (away until October 25)
Modified: 2012-05-04 03:16 PDT (History)
2 users (show)
ayg: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch part 1, v1 -- Move editing-related tests to editor/ directories (6.60 KB, patch)
2012-04-19 01:51 PDT, Aryeh Gregor (:ayg) (away until October 25)
ehsan: review+
Details | Diff | Splinter Review
Patch part 2, v1 -- Move layout/reftests/editor/ to editor/reftests/ (34.18 KB, patch)
2012-04-19 01:52 PDT, Aryeh Gregor (:ayg) (away until October 25)
dbaron: review+
Details | Diff | Splinter Review

Description Aryeh Gregor (:ayg) (away until October 25) 2012-04-19 01:34:53 PDT
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) (away until October 25) 2012-04-19 01:51:46 PDT
Created attachment 616495 [details] [diff] [review]
Patch part 1, v1 -- Move editing-related tests to editor/ directories
Comment 2 Aryeh Gregor (:ayg) (away until October 25) 2012-04-19 01:52:18 PDT
Created attachment 616496 [details] [diff] [review]
Patch part 2, v1 -- Move layout/reftests/editor/ to editor/reftests/
Comment 3 Mozilla RelEng Bot 2012-04-19 02:12:36 PDT
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 2012-04-19 07:16:51 PDT
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
Comment 5 :Ehsan Akhgari 2012-04-19 12:15:11 PDT
Comment on attachment 616495 [details] [diff] [review]
Patch part 1, v1 -- Move editing-related tests to editor/ directories

This is awesome!
Comment 6 :Ehsan Akhgari 2012-04-19 12:17:06 PDT
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) (away until October 25) 2012-04-19 23:24:00 PDT
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-7 (busy September 14-25) 2012-04-22 15:49:15 PDT
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) (away until October 25) 2012-04-23 02:14:31 PDT
Sorry -- which three lines?
Comment 10 Aryeh Gregor (:ayg) (away until October 25) 2012-04-29 07:40:02 PDT
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-7 (busy September 14-25) 2012-04-29 07:46:39 PDT
> 
> # editor/
>-include editor/reftest.list
>+include ../../editor/reftests/reftest.list
Comment 12 Aryeh Gregor (:ayg) (away until October 25) 2012-04-29 22:47:51 PDT
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-7 (busy September 14-25) 2012-04-30 10:38:51 PDT
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) (away until October 25) 2012-04-30 22:51:53 PDT
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-7 (busy September 14-25) 2012-05-01 08:42:04 PDT
ok, either way, but I'd prefer it at the bottom

Note You need to log in before you can comment on or make changes to this bug.