It is common for my xpcshell, mochitest, etc tests to make use of shared testing artifacts, like mocks. e.g. I create a mock HTTP server so I can write tests that pretend to hit the real deal. This mock HTTP server is needed by tests in toolkit/ *and* services/. In the current world, we have to resort to: * Elevating the test-only code to "production" status and install and package it with the distribution (e.g. dist/bin/modules/). * Declaring "head" files in xpcshell.ini and using relative file imports. * Using load() to explicitly load some file (from a location not registered with the URL handling mechanism like resource://) head xpcshell.ini and load() are effectively equivalent. And, they are hard to use because you deal with cryptic paths like ../../../tests/xpcshell/head_foo.js. Shipping unnecessary test code is just silly for obvious reasons. I would like to propose the creation of a well-defined and easy-to-use shared repository of test-related artifacts/modules. How this is done exactly, I'm not sure. From the perspective of JS testing, I would like to support JS modules that can be imported via Components.utils.import() and not via load(). e.g. Cu.import("testing://services/storageserver.js"); What the exact registered URL should be and where that should point to, that's an open question. I think this bug should include: * Determining whether to put shared testing artifacts. Perhaps somewhere in objdir/testing (so they don't get included in the distribution). Although, I'm not sure what the requirements are. * Telling the test harnesses about this location (e.g. hooking up URL handlers) * Providing easy-to-use build system integration so definition of a variable or two magically results in files being installed to the proper location (e.g. like how files in modules/ magically get installed to the right place).

This patch is pretty basic. I'm guessing the contentious part would be the location of the installed testing files. If anyone has a better idea, I'm open to hearing it.

This registers resource://testing/ in xpcshell tests so it points to objdir/_tests/modules/ Try at http://tbpl.mozilla.org/?tree=Try&rev=557ed98f6419 Not sure how I can add test coverage for this. But, I've verified it works (on OS X at least).

Comment on attachment 618716 [details] [diff] [review] Add Makefile rules to install testing-only JS modules This all looks OK to me; I don't think I can provide any value beyond Ted's review.

This version makes the TESTING_MODULES_DIR argument to xpcshell optional. Before, if it wasn't defined, head.js would crash. All tests on the first try failed because the build infrastructure isn't adding the new runxpcshelltests.py argument. Once this is deployed, we'll have to modify the build infrastructure to add this new argument. Until that follow-up change is made, any tests that rely on this new functionality won't work. Fortunately, no tests yet rely on this change. New try at http://tbpl.mozilla.org/?tree=Try&rev=c99bd2a0687e

Try run for 557ed98f6419 is complete. Detailed breakdown of the results available here: https://tbpl.mozilla.org/?tree=Try&rev=557ed98f6419 Results (out of 16 total builds): success: 3 warnings: 13 Builds (or logs if builds failed) available at: http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/gszorc@mozilla.com-557ed98f6419

Try run for c99bd2a0687e is complete. Detailed breakdown of the results available here: https://tbpl.mozilla.org/?tree=Try&rev=c99bd2a0687e Results (out of 16 total builds): success: 9 warnings: 7 Builds (or logs if builds failed) available at: http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/gszorc@mozilla.com-c99bd2a0687e

I have high hopes this one will pass try (https://tbpl.mozilla.org/?tree=Try&rev=cea63c7c8bbb).

Try run for f99bb55423fe is complete. Detailed breakdown of the results available here: https://tbpl.mozilla.org/?tree=Try&rev=f99bb55423fe Results (out of 16 total builds): exception: 3 success: 6 warnings: 4 failure: 3 Builds (or logs if builds failed) available at: http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/gszorc@mozilla.com-f99bb55423fe

Try run for cea63c7c8bbb is complete. Detailed breakdown of the results available here: https://tbpl.mozilla.org/?tree=Try&rev=cea63c7c8bbb Results (out of 16 total builds): success: 15 warnings: 1 Builds (or logs if builds failed) available at: http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/gszorc@mozilla.com-cea63c7c8bbb

Comment on attachment 618716 [details] [diff] [review] Add Makefile rules to install testing-only JS modules Review of attachment 618716 [details] [diff] [review]: ----------------------------------------------------------------- ::: config/rules.mk @@ +1590,5 @@ > +ifdef TESTING_JS_MODULES > +testmodulesdir = $(DEPTH)/_tests/modules/$(TESTING_JS_MODULE_DIR) > + > +libs:: > + $(NSINSTALL) -D $(testmodulesdir) You should use: GENERATED_DIRS = $(testmodulesdir) http://mxr.mozilla.org/mozilla-central/source/config/makefiles/autotargets.mk#23 (that sets up a real dependency)

Comment on attachment 619804 [details] [diff] [review] Part 2: Integrate xpcshell with testing modules, v3 Review of attachment 619804 [details] [diff] [review]: ----------------------------------------------------------------- r+ with a couple of naming nits. ::: testing/testsuite-targets.mk @@ +244,5 @@ > --manifest=$(DEPTH)/_tests/xpcshell/xpcshell.ini \ > --build-info-json=$(DEPTH)/mozinfo.json \ > --no-logfiles \ > --tests-root-dir=$(call core_abspath,_tests/xpcshell) \ > + --tests-modules-dir=$(call core_abspath,_tests/modules) \ Hoo boy, every time I review something like this I really want bug 744540 fixed! ::: testing/xpcshell/head.js @@ +302,5 @@ > } > > function _execute_test() { > + // Map resource://test/ to current working directory and > + // resource://testing/ to the shared test modules directory. That seems confusingly similar. Can we call it resource://testing-common/ or resource://common/ or something like that? ::: testing/xpcshell/runxpcshelltests.py @@ +232,5 @@ > + '-n', > + '-s', > + '-e', 'const _HTTPD_JS_PATH = "%s";' % self.httpdJSPath, > + '-e', 'const _HEAD_JS_PATH = "%s";' % self.headJSPath > + ] Nice reformatting. :) @@ +234,5 @@ > + '-e', 'const _HTTPD_JS_PATH = "%s";' % self.httpdJSPath, > + '-e', 'const _HEAD_JS_PATH = "%s";' % self.headJSPath > + ] > + > + if self.testingModulesDir is not None: I generally just prefer "if self.testingModulesDir:", but maybe the surrounding code says otherwise here? @@ +900,5 @@ > self.add_option("--tests-root-dir", > type="string", dest="testsRootDir", default=None, > help="absolute path to directory where all tests are located. this is typically $(objdir)/_tests") > + self.add_option("--tests-modules-dir", > + dest="testingModulesDir", default=None, This option is a bit awkwardly named. Maybe --testing-modules-dir?

Switch to GENERATED_DIRS made and tested locally. I also pushed part 2 prematurely. Addressed review nits in new part 3. Oops. https://hg.mozilla.org/integration/mozilla-inbound/rev/137aabbc13a4 https://hg.mozilla.org/integration/mozilla-inbound/rev/b063ba6dd084 https://hg.mozilla.org/integration/mozilla-inbound/rev/d8e4e9519516

You rock, gps! We should get this documented on MDN so that developers will put their test helpers in reusable modules rather than head files... I also know a couple of places (*cough* no pun intended) that could benefit from a head -> jsm refactoring... but that's for another bug :)

Yeah, I need to throw something up on MDN. Not sure where though. Here are the choices: https://developer.mozilla.org/en/Mozilla_automated_testing https://developer.mozilla.org/en/Running_automated_tests https://developer.mozilla.org/en/Developing_Tests The testing documentation just needs a lot of love overall. FWIW, I threw up a blog post at http://gregoryszorc.com/blog/2012/05/10/better-sharing-of-test-code-in-mozilla-projects/. I'm being hypocritical of my own rule of not using personal blogs as a replacement for MDN :/

https://hg.mozilla.org/mozilla-central/rev/137aabbc13a4 https://hg.mozilla.org/mozilla-central/rev/b063ba6dd084 https://hg.mozilla.org/mozilla-central/rev/d8e4e9519516