Currently mochitest-browser-chrome logs some things a little differently than other mochitest types, which is a pain for log parsers. E.g. Browser Chrome Test Summary Passed: 9848 Failed: 0 Todo: 7 instead of INFO TEST-START | Shutdown INFO Passed: 486 INFO Failed: 0 INFO Todo: 0 and TEST-INFO | chrome://mochikit/content/browser/toolkit/mozapps/plugins/tests/browser_bug435788.js | Test took 5.171s to complete instead of INFO SimpleTest finished /tests/modules/plugin/test/test_twostreams.html in 180ms These should be made to be consistent.

This patches browser-chrome logging to more closely conform to regular mochitest standards, so that the logparser in bug 601216 doesn't have to special-case browser-chrome logs.

Comment on attachment 480316 [details] [diff] [review] mochitest patch Looks good. I am concerned about downstream fallout of this patch. So, I propose we do the following: 1. Run it on try server (with only a mochitest-other test specified) 2. File a follow-on RelEng bug with a simple patch to fix the buildbot log parsing code: http://mxr.mozilla.org/build/source/buildbotcustom/steps/unittest.py#119 3. Add a maemkit patch to ensure that picks up the necessary changes as well (see jmaher for help with hunting down the maemkit code). 4. Flag this bug to land on all active branches (1.9.2, 1.9.1). Once we have the buildbot changes landed (that will require a downtime), we can take this patch on m-c, 1.9.2, 1.9.1. You'll probably want to make sure the patch will apply cleanly to the 1.9.2 and 1.9.1 trees and attach specific versions of the patch for those branches if it doesn't apply. Thanks so much for helping make our logs sane.

Christian, Due to the fact that buildbot does not branch per tree, we will need to land this change on all active security branches. Jgriffin will create patches for those branches, but can I get approval for him to land there since this is in no way a part of the build on those branches? Or is approval even needed for this since it doesn't affect the products being built from those branches? Thanks

Historically we have not required approval for test/test harness changes.

Is it actually possible to guarantee that a change lands in the buildbot parser, + m-c, + 1.9.1 + 1.9.2, at the same time? It seems that if these land separately, they'll break tinderbox log parsing. To avoid this potential problem, perhaps we should land a patch on all the branches which adds new logging in addition to the old logging, then patch the buildbot parser, then a followup patch to the branches which removes the obsolete logging.

(In reply to comment #5) > Is it actually possible to guarantee that a change lands in the buildbot > parser, + m-c, + 1.9.1 + 1.9.2, at the same time? It seems that if these land > separately, they'll break tinderbox log parsing. > > To avoid this potential problem, perhaps we should land a patch on all the > branches which adds new logging in addition to the old logging, then patch the > buildbot parser, then a followup patch to the branches which removes the > obsolete logging. Sounds like a much better way forward. Let's do that.

This is a better patch, which I don't think will require any buildbot changes. Baking on tryserver now to verify.

Comment on attachment 482727 [details] [diff] [review] mochitest logging patch v0.2 This patch passes on tryserver, and doesn't require any buildbot changes. It modifies both 'regular' mochitest and browser-chrome, introducing a TEST-END 'status' type that contains the duration of a test.

Comment on attachment 482727 [details] [diff] [review] mochitest logging patch v0.2 This looks good. I'd still like to change the results output of browser-chrome one of these days to match the other test frameworks, but let's just file a follow-on bug for that.

Pushed to m-c as http://hg.mozilla.org/mozilla-central/rev/b1d1e0873373

This broke manually running browser-chrome tests, this seems to be the fix

Landed the fix: http://hg.mozilla.org/mozilla-central/rev/e4e3f54c851a

Here's a patch for 1.9.2. It does something similar, but the patch itself is different, as there are quite a few differences in browser-test between m-c and 1.9.2.

Can we just merge all of the harness files from trunk to 1.9.2? Hopefully none of the changes since then depend on other mozilla-central changes...

It can be tricky, getting all the dependencies right is hard. There can be platform dependencies, build system dependencies, etc.

I think the browser-chrome harness itself hasn't changed much. I'll give it a shot and see what happens!

Comment on attachment 485445 [details] [diff] [review] 1.9.2 mochitest patch Johnathan, do we still want to do this? Not sure that updating 1.9.2 is worth the effort at this point. Ridiculous of me to keep this in my queue for so long - sorry about that.

Comment on attachment 485445 [details] [diff] [review] 1.9.2 mochitest patch No, I don't think we need this patch any longer.

Is this still tracking something useful?

I'd say it's been resolved as much as we need it to be.