For testing code in browser/ we need the ability to run tests on app-chrome windows, esp. browser.xul. Dietrich suggested using xul overlays, I think a simpler solution (at least for the browser window) would be to set up an extension on the tinderbox which would use the subscript loader to run tests-scripts.

This should be doable pretty easily, since the runtests.pl --chrome already creates a chrome directory and an extension-like environment (we don't start the EM because it makes it hard to kill processes). How would you like to identify the files to be executed this way? naming convention? make rule?

(In reply to comment #1) > How would you like to identify the files to be executed this way? naming > convention? make rule? OK, no answer, so files that start with the string "browser_" get this treatment. I don't really care how this gets done, so speak up if you have a better idea.

browser_ or browser_xul_ works for me, thanks.

Are there any platform-specific issues with this? I know, for example, that one cannot get references to menus from the DOM on a Mac. The nodes are just not there. Are there other problems also?

(In reply to comment #4) > Are there any platform-specific issues with this? Test files can be opted-out for a certain platform via their corresponding Makefile, I dunno whether we pre-process the test-files (which can rather check navigator.platform). > I know, for example, that one cannot get references to menus from the DOM > on a Mac. The nodes are just not there. Are there other problems also? So BTW, that's a false assertion - the nodes are there, and they're cpp/js-accessible (and useful, "label" and other attributes are dynamically updated).

I have this mostly-working, I just need to figure out how to get logging/visual output done. I was thinking maybe I could just re-use harness.xul, running in a separate window, but I think getting it to "run tests" in a different way (trigger subscript loads in another window rather than loading tests in a frame or content area) might be difficult.

excellent, thanks for doing this Gavin. this is a high priority item for places, as much of the A6 places work will be UI, and i'd like to have our interns writing tests against their UI changes from the start.

This is an example of the types of test that I can currently run with what I have (I have it in browser/base/test). The output/logging part still isn't done. I'm leaning towards just implementing basic logging/output on my own instead of trying to hack mochitest to support this. I can reuse some parts, like MozillaFileLogger.js, but most of the other stuff is pretty specific to running in content windows and loading documents in iframes, and I think trying to make the mochitest code generic enough to be usable here is more trouble than it's worth. That means I'll need to duplicate some of the functionality, but I have basic result display working already, and logging shouldn't be very hard to add.

(In reply to comment #8) > > The output/logging part still isn't done. I'm leaning towards just implementing > basic logging/output on my own instead of trying to hack mochitest to support > this. I can reuse some parts, like MozillaFileLogger.js, but most of the other > stuff is pretty specific to running in content windows and loading documents > Yeah, that sounds reasonable, but we want to keep the amount of test goop we have to a minimum. I gather you're using runtests.pl, which is good (and was the biggest pain to get right).

I think this is about ready to be landed, though I should probably look it over again, and test on Linux/Windows (an earlier version has already been tested on Windows). Supports async tests now, similar to the way mochitest does (waitForExplicitFinish(), finish(), etc). I'll attach some example tests next.

Just unzip this into the mozilla/ root (they end up in browser/base/test).

In patch v1, you move arrayOfTestFiles() out of jsonArrayOfTestFiles() so that browser-harness.xul's listTests() can call it directly. But while jsonArrayOfTestFiles() calls isTest() on the results of arrayOfTestFiles(), listTests() doesn't, so the harness tries to run every file in the test directory as a test, including supporting files. The isTest() check should probably be factored out into arrayOfTestFiles() so that both jsonArrayOfTestFiles() and listTests() get the same list of real tests. Also, given that browser chrome tests are isolated into their own subdirectory of the object dir (_tests/testing/mochitest/browser), and "browser" has multiple connotations, using that string as the prefix seems redundant and potentially confusing. I think it'd make sense to use the same "test_" prefix for browser chrome tests as for the other ones.

(In reply to comment #12) > The isTest() check should probably be factored out into arrayOfTestFiles() so > that both jsonArrayOfTestFiles() and listTests() get the same list of real > tests. > Also, given that browser chrome tests are isolated into their own subdirectory > of the object dir (_tests/testing/mochitest/browser), and "browser" has > multiple connotations, using that string as the prefix seems redundant and > potentially confusing. I think it'd make sense to use the same "test_" prefix > for browser chrome tests as for the other ones. I think you're right on both points. I'll fix both issues. Thanks for checking it out!

(In reply to comment #12) > Also, given that browser chrome tests are isolated into their own subdirectory > of the object dir (_tests/testing/mochitest/browser), and "browser" has > multiple connotations, using that string as the prefix seems redundant and > potentially confusing. I think it'd make sense to use the same "test_" prefix > for browser chrome tests as for the other ones. Actually, now that I think about it a bit more I think it makes sense to give browser tests their own prefix, so that isTest can distinguish supporting .js files for the normal "test_"-prefixed tests from the browser test files. isTest currently returns false for a filename that matches test_*.js, and this pattern seems to be used in mochitests for supporting files. browser_ might not be the best prefix, however. I'm open to suggestions :)

> Actually, now that I think about it a bit more I think it makes sense to give > browser tests their own prefix, so that isTest can distinguish supporting .js > files for the normal "test_"-prefixed tests from the browser test files. isTest > currently returns false for a filename that matches test_*.js, and this pattern > seems to be used in mochitests for supporting files. Seems like it might be better to just make isTest be aware of what kind of tests are being run. As a side effect, that'd also prevent isTest from returning true for a file resembling a content test when we're running browser tests, which happens with the current code.

(In reply to comment #15) > Seems like it might be better to just make isTest be aware of what kind of > tests are being run. Good idea, I've done that.

Comment on attachment 270204 [details] [diff] [review] patch v1.1 > >Index: testing/mochitest/browser-test.js >=================================================================== >+testScope.prototype = { >+ ok: function test_ok(condition, name, diag) { >+ this.tests.push(new testResult(condition, name, diag, "")); >+ }, >+ >+ is: function test_is(a, b, name) { >+ this.ok(a==b, name, "got " + a + ", expected " + b); >+ }, Should add todo() and is_not(). People like them. The rest of this looks very useful. r=sayrer

(In reply to comment #16) > Created an attachment (id=270204) [details] > patch v1.1 > > (In reply to comment #15) > > Seems like it might be better to just make isTest be aware of what kind of > > tests are being run. > > Good idea, I've done that. Hmm, but it looks like the latest patch continues to name the tests browser_*.js. What I was trying to say was that if isTest was aware of what kinds of tests are being run, then we could safely call browser tests test_*.js, since files with that naming scheme would correctly be considered browser tests in the browser tests directory and supporting files in the content and chrome tests directories.

(In reply to comment #18) > > Hmm, but it looks like the latest patch continues to name the tests > browser_*.js. What I was trying to say was that if isTest was aware of what > kinds of tests are being run, then we could safely call browser tests > test_*.js, since files with that naming scheme would correctly be considered > browser tests in the browser tests directory and supporting files in the > content and chrome tests directories. That doesn't seem like an advantage to me. I think browser_ is pretty clear.

(In reply to comment #19) > (In reply to comment #18) > > > > Hmm, but it looks like the latest patch continues to name the tests > > browser_*.js. What I was trying to say was that if isTest was aware of what > > kinds of tests are being run, then we could safely call browser tests > > test_*.js, since files with that naming scheme would correctly be considered > > browser tests in the browser tests directory and supporting files in the > > content and chrome tests directories. > > That doesn't seem like an advantage to me. I think browser_ is pretty clear. It's pretty clear once you know what it is. Before that, test_ is clearer. It's also less redundant (given that these tests live inside a "browser" directory) and consistent with the naming scheme for the other two kinds of MochiTests (content and chrome), both of which name their tests test_, which makes test_ for browser tests less likely to trip up folks who have written the other kinds of tests in the past.

(In reply to comment #20) > (In reply to comment #19) > > > > That doesn't seem like an advantage to me. I think browser_ is pretty clear. > > It's pretty clear once you know what it is. Before that, test_ is clearer. > It's also less redundant Yeah, I can buy that. I clearly don't spot redundancy in path names very well. :)

(In reply to comment #20) > It's also less redundant (given that these tests live inside a "browser" > directory) They live in a "browser" directory in the objdir when they're copied over, but in the source tree I figured it would make most sense to put them in the same directory as MochiTests, in which case the differentiation is useful. Perhaps this isn't a good idea, and they should just live in their own "browser" subdirectory in the source tree, too. > and consistent with the naming scheme for the other two kinds of MochiTests > (content and chrome), both of which name their tests test_, which > makes test_ for browser tests less likely to trip up folks who have written > the other kinds of tests in the past. I guess I don't really understand why consistency in naming between different types of tests is a good thing. It seems to me that it's easier on everyone if the type of test is easy to discern given the filename. Even if it makes sense for the two types of MochiTests (chrome and content), I'd argue that it doesn't for these tests, given that they're running in an entirely different test harness (the only thing they have in common is that they share runtests.pl, really).

A few minor fixes: - fix --close-when-done - add a title to the test window - add todo() and isnot() - don't start the server if we're running browser-chrome tests

mozilla/testing/mochitest/Makefile.in 1.7 mozilla/testing/mochitest/browser-harness.xul 1.1 mozilla/testing/mochitest/browser-test-overlay.xul 1.1 mozilla/testing/mochitest/browser-test.js 1.1 mozilla/testing/mochitest/runtests.pl.in 1.21 mozilla/testing/mochitest/server.js 1.12 Initial documentation for this is up at http://developer.mozilla.org/en/docs/Browser_chrome_tests . Filed bug 387415 to get this running on the buildbots.