Filing this as a new bug to use what bug 448416 implemented on the SeaMonkey tester buildbots, as this doesn't actually fix bug 448125 - it only hides the leaks but doesn't actually fix them.

This might seem over-engineered but it makes the task easy to later reduce the thresholds until we come down to zero. It also codifies the currently seems numbers of leaks as thresholds for the moment. We surely want to bring those down, that's what bug 448125 is still here for.

Comment on attachment 334205 [details] [diff] [review] make thresholds configurable in config.py [Checkin: Comment 4] My only nit is I think it would be easier to visualize and edit if you group the platform leak Values together in the config.py rather than by test type.

My review did not include actually verifying the leak values being entered. I will double check soon anyway, and I do feel that tweaking these by you can have an rs+ from me for initial green; and rs+ for tuning them down to 0.

OK, checked in the current state, and did pull + reconfig on the master. The next cycle should pick this up, let's see how green it turns.

I'll at least reopen this as the values don't get through to the actual mochitest runs. It may be that it's actually bug 448416 that wasn't fixed correctly though.

From some investigation Bug 448416 was fixed correctly, but sadly it does not really fix us, as we currently need to set command manually for all these ourselves, (to pass in correct --appname) The easy fix is to manually append each command in our buildbot config with a --leakThreshold option for now. The slightly harder and probably more correct fix is to extend the buildbotcustom code to allow specifying an appname as well. I'll eventually get this done but I suspect at least a week before I can develop the patch, not counting review. my suggestion is to manually set/hack in the leakThreshold option to these and once I get the appname patch done/approved/etc we can revert that hack and do it the right way.

I don't think passing in appName should be necessary at all, as automation.py already knows this information. I think its presence in the unittest configs is an accident. http://mxr.mozilla.org/mozilla-central/source/build/pgo/automation.py.in#80

I actually think we should change the implementation of all this over to the new make targets and pass the leakThreshold as a make variable.

(In reply to comment #7) > I don't think passing in appName should be necessary at all, as automation.py > already knows this information. I think its presence in the unittest configs is > an accident. > http://mxr.mozilla.org/mozilla-central/source/build/pgo/automation.py.in#80 > We don't run pgo and I don't see how those are easily available to other portions of the tree, perhaps they should be? (In reply to comment #8) > I actually think we should change the implementation of all this over to the > new make targets and pass the leakThreshold as a make variable. > I could be wrong as I haven't studied those build targets heavily yet, but I don't remember leakThreshold being an option using that method. Either way, I still feel that my hackier way is best for the moment (we really need to catch for increases in leaks sooner than later)

(In reply to comment #9) > (In reply to comment #7) > > I don't think passing in appName should be necessary at all, as automation.py > > already knows this information. I think its presence in the unittest configs is > > an accident. > > http://mxr.mozilla.org/mozilla-central/source/build/pgo/automation.py.in#80 > > > > We don't run pgo and I don't see how those are easily available to other > portions of the tree, perhaps they should be? automation.py is used by runtests.py, it's just a library. It happens to live in build/pgo because it's also used by the pgo profiler script.

After Bug 451812 is fixed, this should turn our buildbots green.

(In reply to comment #11) > Created an attachment (id=335147) [details] > After Bug 451812 is fixed, this should turn our buildbots green. (Dumb question: exact same patch file as in that other bug !?) (I'm wondering further as it looks like a Hg patch to a cvs repository: http://mxr.mozilla.org/mozilla/source/tools/buildbotcustom/unittest/steps.py)

...this should be the right patch, thanks Gerv.

> ...this should be the right patch, thanks Gerv. err, Serg

Comment on attachment 335169 [details] [diff] [review] real patch for followup [Checkin: See comment 42] looks good to me - if bug 451812 gets r+ Have you verified that automation.py looks for the right out of "seamonkey-bin", "seamonkey.exe" and "seamonkey" (Mac/Win/unix)? I'm mostly concerned about the Mac case as FF may actually ship and possibly use a "firefox" script there even if it's unneeded.

Comment on attachment 335169 [details] [diff] [review] real patch for followup [Checkin: See comment 42] Hmm, I actually have not tested mac case... stefanh can you please test that |python runtests.py --autorun --console-level=INFO --close-when-done| works from |build/objdir/mozilla/_tests/testing/mochitest| on Mac for me and if so, grant review here.

For the record, before this bug is fixed, there are already some leaks reduced to warning only: Linux comm-central dep unit test on 2008/08/23 23:31:34 mochitest TEST-PASS | runtests-leaks | WARNING leaked 143568 bytes during test execution Linux / MacOSX / Windows browser TEST-PASS | runtests-leaks | WARNING leaked 8... bytes during test execution NB: Though I don't know why. I guess there already some settings somewhere !?

Comment on attachment 335169 [details] [diff] [review] real patch for followup [Checkin: See comment 42] rbgray over IRC verified that what I asked stefanh to do works, great!

Comment on attachment 335169 [details] [diff] [review] real patch for followup [Checkin: See comment 42] $ hg push pushing to ssh://hg.mozilla.org/build/buildbot-configs searching for changes remote: adding changesets remote: adding manifests remote: adding file changes remote: added 1 changesets with 1 changes to 1 files $ hg tip -v changeset: 246:7682755245d4 tag: tip user: Justin Wood <Callek@gmail.com> date: Mon Aug 25 18:53:31 2008 -0400 files: seamonkey-unittest/master.cfg description: Bug 450983, followup -- do not pass command to buildsteps for mochikit steps r=KaiRo

(leaving bug open until KaiRo pushes these changes to buildbots themselves)

the change did not work, we ended up with strange commandlines like in http://cn-sea-qm-centos5-01.nl.mozilla.org:8010/builders/MacOSX%2010.4%20comm-central%20dep%20unit%20test/builds/590/steps/mochitest/logs/stdio so I went back to the changeset before this was checked in to get the buildbots running again. Could you back out your patch and try to figure out the problem?

Comment on attachment 335169 [details] [diff] [review] real patch for followup [Checkin: See comment 42] backed out per buildbots failing oddly, KaiRo has had them running without this changeset for a bit now.

I checked in http://hg.mozilla.org/build/buildbot-configs/rev/e2205fdd23c7 for now, setting leakThreshold directly in the commands we're calling. I hope it works, but it's only a temporary fix until we get a real one. Maybe the buildbotcustom instance should always set --leaThreshold and just default the actual value to 0 unless something else is handed to the buildstep?

We gained 240 bytes on mac and 232 bytes on Windows in mochitest leaks since comment #1 (Linux has some much higher value indicating a larger additional problem there). On mochichrome we gained 140 bytes on Linux, 132 on Windows, and also 140 on Mac. mochibrowser has a real test failure, so leak reports might not be accurate. I'm upping the thresholds for the mochitest/mochichrome test, but the difference numbers reported here might give a pointer to what actually got added, so I wanted to note them here.

(In reply to comment #24) NB: "gain" meaning "more" leaks newly happening (or simply detected). > a pointer to what actually got added I think unless we would have a graphic (or even text) report [hint !] like nye has, chasing in (tinderbox waterfall, 3 weeks) history would be too painful. As these leaks are not even reported on the waterfall itself [hint !], I think this will help for now on (only). Yet, that's just what I've been hoping for all along ;-)

Just as an uncompleted example of what happened.

Apart from the 2 usual MacOSX failing tests, we have 1 new failing test (bug 454513) and 1 new leak (bug 454524), which keep the boxes Orange even after comment 23 checkin; I would suggest to not workaround them immediately, until we see if we can get them fixed.

I added the current amount of mochibrowser leaks to the config.py and will reconfig soon to make it live, as it's better to actually see boxes going from green to orange when something new is introduced as to stay in indefinite orange.

(In reply to comment #25) > I think unless we would have a graphic (or even text) report [hint !] As Callek asked me, I filed Tb bug 456274, Tb bug 456275 and SM bug 456279.

Thanks to fixing bug 433132, we can reduce the threshold on the Linux test box: { mochitest http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1222418644.1222423001.26272.gz&fulltext=1 Linux comm-central dep unit test on 2008/09/26 01:44:04 TEST-PASS | runtests-leaks | WARNING leaked 143116 bytes during test execution http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1222426721.1222431311.32673.gz&fulltext=1 Linux comm-central dep unit test on 2008/09/26 03:58:41 TEST-PASS | runtests-leaks | WARNING leaked 21416 bytes during test execution } Now, we have (what seems to be) the same leak reports on all 3 platforms :-) *** The MacOSX and Windows boxes didn't report bug 433122 :-/ (Which I could reproduce on my local Windows.) Do they have a (dumb) printer set up ? ...

They probably have no printer set up - none of our boxes is actually connected to one. The reason why Linux catches the bug might be that Linux always has the "print to PDF" stuff that GNOME gives us. :) I'll do the threshold change when I'm back on my normal machine, probably Monday.

(In reply to comment #30) > The MacOSX and Windows boxes didn't report bug 433122 :-/ Bug 433132 ! (In reply to comment #31) > The reason why Linux catches the bug might be that Linux always has the > "print to PDF" stuff that GNOME gives us. :) Actually, that's the reason why I couldn't work on that bug until recently, when I installed the (Windows) doPDF utility ;-)

(In reply to comment #31) > I'll do the threshold change when I'm back on my normal machine, probably > Monday. Done by bug 457440 comment 1.

I noticed that we have a small but consistent improvement on mochi+browser+chrome mochitests: Linux -80, Mac and Win -76.

(In reply to comment #34) > I noticed that we have a small but consistent improvement on > mochi+browser+chrome mochitests: > Linux -80, Mac and Win -76. Robert, Justin, could you update these chrome and browser thresholds ? mochi has regressed in the meantime: see bug 463355.

I adjusted our thresholds for this now, see http://hg.mozilla.org/build/buildbot-configs/rev/58af505b0e9e

(In reply to comment #21) > http://cn-sea-qm-centos5-01.nl.mozilla.org:8010/builders/MacOSX%2010.4%20comm-central%20dep%20unit%20test/builds/590/steps/mochitest/logs/stdio This link is HTTP 404 now :-( > try to figure out the problem? Guessing: maybe related to bug 456281 comment 10 ??

See bug 391318 comment 45 for a brief explanation of what got fixed. KaiRo, can you update our thresholds per "Current state" ? Thanks.

http://hg.mozilla.org/build/buildbot-configs/rev/88ee32f431b0 adjusts thresholds to new levels after fixes for both bug 465556 and bug 391318. OS X leaks 1112+8 on mochitest and 1112+0 on mochichrome, and even still leaks slightly more than the others on browser chrome tests - the 1112 seems to be some magtc number across the machines ;-)

(In reply to comment #39) I filed bug 470709 about the remaining BrowserChrome leak. > OS X leaks [...] more [...] I filed bug 470710 about the MacOSX case.

We're using leak thresholds correctly with current configs.

Comment on attachment 335169 [details] [diff] [review] real patch for followup [Checkin: See comment 42] Ftr, checkin: comment 19, backout: comment 22; eventually done by http://hg.mozilla.org/build/buildbot-configs/rev/86561671f4cc and http://hg.mozilla.org/build/buildbotcustom/rev/4a337a624db2

V.Fixed