At the moment we just try to open the specified path, but this means that if you want log files to be placed in a subdirectory you first need to create the directory structure. It would be nice if mozlog could create the directories as needed. See: http://hg.mozilla.org/mozilla-central/file/3094601af679/testing/mozbase/mozlog/mozlog/structured/commandline.py#l60 Something like: os.makedirs(os.path.dirname(name))

So I ran the tests locally with success. Also the following command: python runtests.py --binary /home/jp/dev/mozilla/obj-x86_64-unknown-linux-gnu/dist/bin/firefox-bin tests/unit/test_click.py --log-html=toto/f1.html --log-xunit=titi/f2.xunit work as expected.

Comment on attachment 8563372 [details] [diff] [review] Create directories for log specified on the command line if not present Review of attachment 8563372 [details] [diff] [review]: ----------------------------------------------------------------- ::: testing/mozbase/mozlog/tests/test_structured.py @@ +799,5 @@ > + try: > + # ensure the directory does not yet exists > + self.assertFalse(os.path.exists(os.path.dirname(tempfiletbpl))) > + > + args = parser.parse_args(["--log-tbpl=%s" % tempfiletbpl, This is OK, I guess, but in general it's preferable not to rely on formatters other than the raw formatter in tests because the output format could change.

push to try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=c6b2c9523003

So, try fails on windows (32/64) due to my new unit test: WindowsError: [Error 32] The process cannot access the file because it is being used by another process Rhh. Windows just don't know how to delete a file. :) (still, I used mozfile.remove!). I'm trying with addCleanup instead of the try/finally: https://treeherder.mozilla.org/#/jobs?repo=try&revision=98f13f4f168b

Still no luck. I see some choices: 1. someone have a better way to do it 2. deactive this test on windows 3. remove completely this test James, what do you think ?

I think it ought to work if you make sure that the file handle you open in python is closed before the finally block i.e. something like with open(tempfiletbpl) as out_f: result_lines = .read().splitlines() self.assertEqual(["INFO message", "DEBUG message", "ERROR message"], result_lines) finally: mozfile.remove(tempdir) Sorry, I should have noticed that during review.

(In reply to James Graham [:jgraham] from comment #6) > I think it ought to work if you make sure that the file handle you open in > python is closed before the finally block Oh, thanks, good to know ! I pushed a new try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=5b22cbb3b6a4 I removed the try/finally logic and used addCleanup instead - I think now everything is python 2.7 so it's ok. I ask for another review in case it's not. :)

Comment on attachment 8565075 [details] Create directories for log specified on the command line if not present Well, that did not worked (push is still red on Windows). I suspect this is because the file is opened in write mode in the mozlog code directly (this is the purpose of the patch after all). I propose that we do not active this test on windows. Jgraham, could you have a look at my last patch (to ensure I did not make any mistake) and tell me what you think ?

This seems like a fairly fundamental issue; we totaly rely on gc to free up StreamHandler's file handles, which seems rather suboptimal. I think we should file a followup bug on this. In the meantime I think it's OK for this to land without a test; the patch is rather straightforward (significantly shorter than the test and not doing anything other than calling library functions).

Ok, I removed the test. Carrying myself the r+ from the previous patch reviewed by jgraham.

does this fix also the orange build warning from the try run ?

Yes, I should have mentioned that, sorry. That fixes the warning because it was the new test causing it, and I removed it.

Carsten: Does comment 12 answer your question? Can this be checked in?

(In reply to Dave Hunt (:davehunt) from comment #13) > Carsten: Does comment 12 answer your question? Can this be checked in? Hi Dave, yes the requirements are fullfilled now, thanks! also this doesn't apply cleanly it seems: \adding 1132409 to series file renamed 1132409 -> file_1132409.txt applying file_1132409.txt patching file testing/mozbase/mozlog/mozlog/structured/commandline.py Hunk #2 FAILED at 51 1 out of 2 hunks FAILED -- saving rejects to file testing/mozbase/mozlog/mozlog/structured/commandline.py.rej patch failed, unable to continue (try -v) could someone take a look ?

Ok, the context around was changed in the mean time. Fixed now.