479998 - move (objdir) _tests/testing/mochitest to _tests/mochitest

Status: RESOLVED WONTFIX

(Testing :: Mochitest, defect)

Description

[Serge Gautherie (:sgautherie)]

Bug 446300 comment 20:
{
From  Ted Mielczarek (:luser)   2009-01-29 05:05:01 PST

Yeah, I'd prefer to get rid of the "testing". I think mochitest is in there
just to parallel the source directory structure, but I don't think that's
really necessary. "_tests/xpcshell" seems fine, and we could move mochitest to
"_tests/mochitest", although you'll have to let people know, because people
tend to run runtests.py directly.
}

***

What do you actually mean by "let people know" ?

Comment 1

[(not currently active) Ted Mielczarek]

Post about it on dev.platform or something.

Comment 2

[Serge Gautherie (:sgautherie)]

As a reminder, we use 4 subdirectories:
mochitest  : mochitest/tests
mochichrome: mochitest/chrome
browser    : mochitest/browser
a11y       : mochitest/a11y


The mozilla-central part, to start with:

http://mxr.mozilla.org/mozilla-central/search?string=_tests%2Ftesting%2Fmochitest&case=on
{
Found 159 matching lines in 122 files

$(INSTALL) $^ $(DEPTH)/_tests/testing/mochitest/chrome/$(relativesrcdir)

$(INSTALL) $^ $(DEPTH)/_tests/testing/mochitest/tests/$(relativesrcdir)

$(INSTALL) $^ $(DEPTH)/_tests/testing/mochitest/tests/toolkit/components/microformats/tests

$(INSTALL) $(foreach f,$^,"$f") $(DEPTH)/_tests/testing/mochitest/a11y/$(relativesrcdir)

$(INSTALL) $(foreach f,$^,"$f") $(DEPTH)/_tests/testing/mochitest/browser/$(relativesrcdir)

$(INSTALL) $(foreach f,$^,"$f") $(DEPTH)/_tests/testing/mochitest/chrome/$(relativesrcdir)

$(INSTALL) $(foreach f,$^,"$f") $(DEPTH)/_tests/testing/mochitest/tests/$(relativesrcdir)

$(INSTALL) $(foreach f,$^,"$f") $(DEPTH)/_tests/testing/mochitest/tests/$(relativesrcdir)/applets/org/w3c/domts
}
(I hope I didn't miss some, but anyway:)
Would there be a mean to share/reuse (and possibly unify; even if keeping some exceptions) these rules from a common place ?  Instead of having them copied all everywhere ?
If so, I could do this in 2 steps: 1- merge, 2- update the path.


As a reminder that there is some more places to fix:
/testing/mochitest/runtests.py.in
/testing/testsuite-targets.mk
http://mxr.mozilla.org/mozilla-central/search?string=mochitest&case=on&find=%2FMakefile%5C.in%24
(and the comm-central part...)

Comment 3

[Serge Gautherie (:sgautherie)]

(In reply to comment #2)
> (I hope I didn't miss some, but anyway:)

Missed one
http://mxr.mozilla.org/mozilla-central/search?string=%2Ftesting%2Fmochitest%2F.*%2F%5B%5E%5C%24%5D&regexp=on&case=on&find=%2FMakefile%5C.in%24
{
$(INSTALL) $(foreach f,$^,"$f") $(DEPTH)/_tests/testing/mochitest/tests/$(relativesrcdir)/subdir
}

Comment 4

[(not currently active) Ted Mielczarek]

Yeah, I bet we already have a bug filed on that. I'd like to see something like (in Makefiles):

MOCHITEST_TESTS := \
  test_foo.html \
  $(NULL)

MOCHITEST_CHROME_TESTS := \
  test_chrome_foo.xul \
  $(NULL)

MOCHITEST_BROWSER_TESTS := \
  browser_foo.js \
  $(NULL)

MOCHITEST_A11Y_TESTS := \
  test_a11y_foo.html \
  $(NULL)

and then code to handle each of these cases in rules.mk.

Comment 5

[Serge Gautherie (:sgautherie)]

(In reply to comment #4)
> Yeah, I bet we already have a bug filed on that.

Thre's
Bug 370750 - Add MOCHITESTS_DIR magic to config/rules.mk
but doesn't look like to be the exact same issue.

Comment 6

[Serge Gautherie (:sgautherie)]

Attachment: (Av1-MC) Create a rule for MOCHITEST_A11Y_TESTS

[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.2a1pre) Gecko/20090228 Minefield/3.2a1pre] (home, optim default) (W2Ksp4)
(http://hg.mozilla.org/mozilla-central/rev/f7f62131998d)

Like this ?

Comment 7

[(not currently active) Ted Mielczarek]

This would be nicer with bug 395019 in place, too.

Comment 8

[(not currently active) Ted Mielczarek]

Comment on attachment 366102 [details] [diff] [review]
(Av1-MC) Create a rule for MOCHITEST_A11Y_TESTS

I assume you started with a11y since it's the smallest of the mochitest suites? Hopefully you'll expand this to all mochitest types. I hate all the makefile gunk we've grown just for installing tests.

+libs:: $(MOCHITEST_A11Y_TESTS)
+	$(INSTALL) $(foreach f,$^,"$f") $(DEPTH)/_tests/testing/mochitest/a11y/$(relativesrcdir)

Listing $(MOCHITEST_A11Y_TESTS) as a dependency doesn't really do anything useful here, since libs is explicitly built every time. Just ditch that.

$(foreach f,$^,"$f")

Do we really have any test files with quotes in their names? I'd just replace this foreach with $(MOCHITEST_A11Y_TESTS)  directly. Doesn't seem worth worrying about.

r=me with those changes.

Comment 9

[Serge Gautherie (:sgautherie)]

(In reply to comment #7)
> This would be nicer with bug 395019 in place, too.

Indeed! And given both bugs "need" to update the same ("150") Makefile.in files,
let's (wait here then) update them in one pass.


(In reply to comment #8)
> (From update of attachment 366102 [details] [diff] [review])
> I assume you started with a11y since it's the smallest of the mochitest suites?

Exactly, I wanted your review on how I understood it first. ;-)

> r=me with those changes.

I'll do. (I'm not too familiar with this so I just wanted to start safely.)

Comment 10

[Jeff Walden [:Waldo]]

(In reply to comment #8)
> $(foreach f,$^,"$f")
> 
> Do we really have any test files with quotes in their names? I'd just replace
> this foreach with $(MOCHITEST_A11Y_TESTS)  directly. Doesn't seem worth
> worrying about.

This wasn't for quotes; see bug 389793.  We should indeed keep worrying about it, but an explanatory comment that this makes special characters, particularly ^, unspecial would be good.

Comment 11

[Jeff Walden [:Waldo]]

(There previously was no explanatory comment because I thought it smart to keep the necessary hunk to copy-paste as small as possible.)

Comment 12

[(not currently active) Ted Mielczarek]

Ok, makes sense. If we're centralizing it then a comment is quite fine.

Comment 13

[Andrew Halberstadt [:ahal]]

Mass closing mochitest bugs that haven't had activity in the past 5 years. Please re-open or file a new bug with modern context if this is still relevant.