(In reply to bug 484298 comment 3) > Does it slow the tests down a lot? Do they take up a lot of disk space? Actually, it does, especially (now) after bug 362433. (In reply to bug 484298 comment 4) > I don't think we need a special argument, we should just always write the log > or never write the log. I'd like to either: *Not log by default and add an option to enable logging (for developers). *Log by default and add an option to disable logging (for test boxes and testers). Ted, what do you think?

Ted, ping for answer.

I don't know. I just want it to "do the right thing" and make people not think about it. I guess that means logging by default. Maybe we can make the top-level xpcshell-tests target disable logging, but the per-directory ones enable it?

(In reply to comment #2) > Maybe we can make the top-level xpcshell-tests target disable logging, > but the per-directory ones enable it? I like that! ;-)

Av1, with dir/file handling improvement too.

Comment on attachment 399310 [details] [diff] [review] (Av2) Support --no-logfiles in xpcst suite, improve dir/file handling in 3 suites Succeeded as try-2a5369c1706f.

Comment on attachment 399310 [details] [diff] [review] (Av2) Support --no-logfiles in xpcst suite, improve dir/file handling in 3 suites - # Start with a clean slate. - shutil.rmtree(profileDir, True) - os.mkdir(profileDir) - # reftest should only need the dump pref set + # Set preferences. What's the point of these changes? -def runTests(xpcshell, testdirs=[], xrePath=None, testPath=None, - manifest=None, interactive=False, symbolsPath=None): - """Run the tests in |testdirs| using the |xpcshell| executable. +def runTests(xpcshell, xrePath, symbolsPath, + manifest, testdirs, testPath, + interactive, logfiles): Don't change the default values here. + if profileDir: + # shutil.rmtree(profileDir, True) + shutil.rmtree(profileDir) I don't think it matters whether we pass "ignoreErrors=True" or not, but get rid of the commented version. - if not runTests(args[0], testdirs=args[1:], - xrePath=options.xrePath, - testPath=options.testPath, - interactive=options.interactive, - manifest=options.manifest, - symbolsPath=options.symbolsPath): + if not runTests(args[0], options.xrePath, options.symbolsPath, + options.manifest, args[1:], options.testPath, + options.interactive, options.logfiles): Don't change this. I like the keyword argument style.

Av2, with comment 6 suggestion(s). (In reply to comment #6) > (From update of attachment 399310 [details] [diff] [review]) > - # Start with a clean slate. > - shutil.rmtree(profileDir, True) > - os.mkdir(profileDir) > - # reftest should only need the dump pref set > + # Set preferences. > > What's the point of these changes? Calling code is { profileDir = mkdtemp() createReftestProfile(options, profileDir) } mkdtemp() is supposed to return an already clean dir, so it seems rmtree+mkdir is useless and asking for trouble. (And it helps keep all "same" code the same...) If you do want to keep them, I would suggest to add a comment to explain why they're needed. (And the code is now setting up 2 prefs, not just 1.)

Comment on attachment 401333 [details] [diff] [review] (Av3) Support --no-logfiles in xpcst suite, improve dir/file handling in 3 suites [Checkin: See comment 9 & 10 & 11] interactive=options.interactive, - manifest=options.manifest, - symbolsPath=options.symbolsPath): + options.logfiles): Pass logfiles as a keyword argument, logfiles=options.logfiles r=me with that change. Thanks for the explanation, I guess either that code has always been stupid, or it became stupid during a refactoring.

Comment on attachment 401333 [details] [diff] [review] (Av3) Support --no-logfiles in xpcst suite, improve dir/file handling in 3 suites [Checkin: See comment 9 & 10 & 11] http://hg.mozilla.org/mozilla-central/rev/72ae5f35e8fa Av3, with comment 8 suggestion(s).

Comment on attachment 401333 [details] [diff] [review] (Av3) Support --no-logfiles in xpcst suite, improve dir/file handling in 3 suites [Checkin: See comment 9 & 10 & 11] http://hg.mozilla.org/releases/mozilla-1.9.2/rev/1c148973c1fb

Comment on attachment 401333 [details] [diff] [review] (Av3) Support --no-logfiles in xpcst suite, improve dir/file handling in 3 suites [Checkin: See comment 9 & 10 & 11] http://hg.mozilla.org/releases/mozilla-1.9.1/rev/6b19fc023243