It sure would be nice for people to not need to use the boilerplate that is found everywhere for ProfD to work. Additionally, we should make sure it's a clean ProfD for every test run so bad tests don't cause more failures that are difficult to debug.

Having issues with the password manager tests here. dolske is currently looking into it, but I have other things that I need to work on.

More work - forgot to qrefresh. Basically, if we delete key3.db from dist/bin, and run the password manager tests everything works out fine. If we run make check toplevel, things fail in the password manager. Not really sure why either.

I want this functionality for a test I want to write, but I'd rather this be opt-in, since I'm not sure all tests will want it, and there might be tests that explicitly don't want a profile. I think we can just expose do_setup_profile_directory() and make it return the profile directory. That should work just fine. Might need a smarter way to cleanup at the end, as well.

Ted, let me bring this to your attention as it might help. In netwerk/test/unit/test_bug248970_cache.js, I setup a profile directory manually, but forgot to clean it up at the end of the test. Bug 489585 was filed to clean up that directory, but my relatively simple patch there caused a crash at xpcshell cleanup on Mac (and only Mac). You may want to look for similar problems in the cleanup code for this bug...

Ehsan: thanks for pointing that out. Because of that I decided to implement this with cooperation between the Python harness and the JS harness. The Python harness creates a temp dir prior to each test being run and sticks the path into an environment variable. The JS harness provides a do_get_profile() function that sets up the directory provider to point to that directory and also returns it as an nsILocalFile. The Python harness then removes the directory after xpcshell exits.

Sounds like a very reasonable plan. Thanks for driving this forward!

Ok, this implements what I just said I was implementing, and fixes all the tests I could find in the tree that were currently rolling their own dir provider for a profile dir to use it. r?sdwilsh because I was a little more invasive on the DM tests--while testing the patch I noticed that since I made the changes to make tests not reference files from the srcdir, all the DM tests have been requesting a file that's 404 to the test webserver. I fixed that.

Comment on attachment 377454 [details] [diff] [review] v0.3 [Checkin: See comment 18+19+20 & 28] You should be able to drop the directory provider in the download manager tests for DLoads. We've dropped support for downloads.rdf migration on mozilla-central. r=sdwilsh for download manager and places changes.

Comment on attachment 377454 [details] [diff] [review] v0.3 [Checkin: See comment 18+19+20 & 28] >diff --git a/toolkit/mozapps/extensions/test/unit/head_extensionmanager.js b/toolkit/mozapps/extensions/test/unit/head_extensionmanager.js >--- a/toolkit/mozapps/extensions/test/unit/head_extensionmanager.js >+++ b/toolkit/mozapps/extensions/test/unit/head_extensionmanager.js >@@ -234,8 +230,6 @@ > */ > function shutdownEM() > { >- // xpcshell calls xpcom-shutdown so we don't actually do anything here. >- gDirSvc.unregisterProvider(dirProvider); > gEM = null; > } Please leave the comment in the function. You can probably also remove the code from tail_extensionmanager.js that tries to delete the profile folder.

Waldo, when you get time to review this, you only really need to look at: testing/xpcshell/head.js testing/xpcshell/example/unit/test_profile.js testing/xpcshell/runxpcshelltests.py The rest of the patch is just changing existing tests to use the new helper function.

Comment on attachment 377454 [details] [diff] [review] v0.3 [Checkin: See comment 18+19+20 & 28] Re-assigning review of the harness bits to sdwilsh, since he agreed to do them.

Comment on attachment 377454 [details] [diff] [review] v0.3 [Checkin: See comment 18+19+20 & 28] I'd prefer if you used a real java-doc style comment for do_get_profile(). r=sdwilsh with that change.

Pushed to m-c: http://hg.mozilla.org/mozilla-central/rev/9ddc25fb2246

I just tried var gProfD = do_get_profile(); dump("profile path: " + gProfD + "\n"); in my head file and it failed on windows. It appears that XPCSHELL_TEST_PROFILE_DIR wasn't set. Perhaps a unit test to test this functionality would be a good thing?

(In reply to comment #15) > I just tried > > var gProfD = do_get_profile(); > dump("profile path: " + gProfD + "\n"); was actually dump("profile path: " + gProfD.path + "\n");

Oops, forgot to reopen. I just backed this out because it was failing tests. It's possible rebasing it broke it along the way. Backout: http://hg.mozilla.org/mozilla-central/rev/c83b6352eb12 (merge) http://hg.mozilla.org/mozilla-central/rev/c29765db7521

It turned out to just be a bad merge from rebasing. I fixed it up, verified that it passed tests, and re-pushed: http://hg.mozilla.org/mozilla-central/rev/e570c006d264

I made a bad change to one test, pushed a bustage fix: http://hg.mozilla.org/mozilla-central/rev/eaaff4b3401c

*sigh* Fixed that test for real (and tested it on Windows): http://hg.mozilla.org/mozilla-central/rev/884483e28aae Turns out putting the profile directory in $TEMP broke that test, because it was saving the file to the profile directory by default, and I guess Windows doesn't like to put files from the temp directory in the recent files list.

V.Fixed, per bug 489585 comment 24.

Any reason not to land this bug on m-1.9.1?

You're welcome to land it there if you want, it's strictly test-harness+test fixes.

This landed before we branched for 1.9.2

(In reply to comment #24) > This landed before we branched for 1.9.2 Indeed: not 1.9.3a1, not 1.9.2a1, but 1.9.2b1 ;->

(In reply to comment #25) > Indeed: not 1.9.3a1, not 1.9.2a1, but 1.9.2b1 ;-> No, we had a1 before we branched: at Thu Aug 13 14:08:32 2009 +0100 GECKO_1_9_2_BASE at Thu Aug 06 15:38:47 2009 -0700 FIREFOX_3_6a1_RELEASE

Plz stop quibbling about the target milestone of a patch to our test harnesses. It doesn't really matter.

Comment on attachment 377454 [details] [diff] [review] v0.3 [Checkin: See comment 18+19+20 & 28] http://hg.mozilla.org/releases/mozilla-1.9.1/rev/b8f01b7aadc3 merged patch and 2 bustage fixes

(In reply to comment #26) > (In reply to comment #25) > > Indeed: not 1.9.3a1, not 1.9.2a1, but 1.9.2b1 ;-> > No, we had a1 before we branched: > at Thu Aug 13 14:08:32 2009 +0100 GECKO_1_9_2_BASE > at Thu Aug 06 15:38:47 2009 -0700 FIREFOX_3_6a1_RELEASE Fyi, changeset dates are irrelevant, as they can be arbitrarily set. (Only push dates are relevant.) In my m-c repo: FIREFOX_3_6a1_RELEASE = changeset: 31431 : da7fbe8a24dd GECKO_1_9_2_BASE = changeset: 31410 : 376b78fc7223 Main patch push = changeset: 31160 : e570c006d264 FFv3.6a1 'default' r. = changeset: 31152 : 5c913c4662d8 In other words: branching is irrelevant and this bug T.M. _is_ 1.9.2b1. I would guess you looked at http://hg.mozilla.org/mozilla-central/summary, but that reports the "last" push (only) of the branch, not its 'default' base :-/ (At least, I did make that mistake (too) previously :-<)