Last Comment Bug 390916 - Make fxdbug-linux-tbox turn orange if RLk is nonzero
: Make fxdbug-linux-tbox turn orange if RLk is nonzero
Status: RESOLVED FIXED
:
Product: Release Engineering
Classification: Other
Component: Other (show other bugs)
: other
: All All
: P3 enhancement (vote)
: ---
Assigned To: Nobody; OK to take it and work on it
: build
Mentors:
Depends on: 397103
Blocks:
  Show dependency treegraph
 
Reported: 2007-08-04 03:32 PDT by Jesse Ruderman
Modified: 2013-08-12 21:54 PDT (History)
9 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
first shot, untested, etc. (1.36 KB, patch)
2007-10-04 14:59 PDT, Robert Helmer [:rhelmer]
coop: review+
Details | Diff | Review
print reason for test failure to log (1.49 KB, patch)
2007-10-05 00:42 PDT, Robert Helmer [:rhelmer]
dbaron: review+
Details | Diff | Review
set leak threshold on balsa (752 bytes, patch)
2007-10-24 19:31 PDT, Ted Mielczarek [:ted.mielczarek]
rhelmer: review+
Details | Diff | Review

Description Jesse Ruderman 2007-08-04 03:32:50 PDT
RLk is zero on fxdbug-linux-tbox for the first time ever (see http://dbaron.org/log/2007-08#e20070804a).  To help it stay that way, I think we should make the tinderbox turn orange if RLk is nonzero.
Comment 1 Jeff Walden [:Waldo] (remove +bmo to email) 2007-09-24 14:35:56 PDT
Given RLk's finickiness, perhaps we should make the exact amount of leakage a tinderbox preference?  We could set it to whatever the current value is on each of them, then, and not have to wait to drop the currently-8B RLk on fxdbug-linux-tbox back to 0B.  This would also allow us to set a limit on the Mac tinderbox, which recently had its RLk dropped to 12B.
Comment 2 Jesse Ruderman 2007-10-02 19:40:30 PDT
Mac is now at 0B :)
Comment 3 Robert Helmer [:rhelmer] 2007-10-04 14:59:01 PDT
Created attachment 283619 [details] [diff] [review]
first shot, untested, etc.

hey coop, what do you think of this approach for returning "testfailed" if RLk is above a certain threshold on leak tinderboxes?
Comment 4 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2007-10-04 15:22:31 PDT
You probably want the default to be something other than 0 (can it be infinite?), and then we can enable it for those tinderboxes where we want it.
Comment 5 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2007-10-04 15:43:20 PDT
On second thoughts, maybe a default of 0 is good... but you should probably make the case where it's failing print an explanatory message to the log so it's clear why the tinderbox is orange.
Comment 6 Chris Cooper [:coop] 2007-10-04 18:54:50 PDT
Comment on attachment 283619 [details] [diff] [review]
first shot, untested, etc.

Looks like it should do the trick.
Comment 7 Robert Helmer [:rhelmer] 2007-10-05 00:42:07 PDT
Created attachment 283683 [details] [diff] [review]
print reason for test failure to log

David, is this what you had in mind?
Comment 8 Jesse Ruderman 2007-10-05 00:50:38 PDT
"Number of leaks" should be something like "Number of bytes leaked".
Comment 9 Robert Helmer [:rhelmer] 2007-10-05 09:36:05 PDT
(In reply to comment #8)
> "Number of leaks" should be something like "Number of bytes leaked".
> 

Good point, maybe something more like:

print_log "Leak Test Failed: Number of bytes leaked ($leaks) is greater than LeakFailureThreshold ($Settings::LeakFailureThreshold)";

I'll do some testing on this patch and resubmit. If anyone wants to propose any changes in the meantime, please do and I'll include it in the next version.
Comment 10 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2007-10-05 09:49:38 PDT
Comment on attachment 283683 [details] [diff] [review]
print reason for test failure to log

Looks good to me, especially with the new wording from comment 9.  r=dbaron
Comment 11 Ted Mielczarek [:ted.mielczarek] 2007-10-05 09:59:02 PDT
FWIW, you'll get a little more use out of this if you stick an "Error: " in there, so the tinderbox error parser will show it in the brief log:
http://lxr.mozilla.org/mozilla/source/webtools/tinderbox/ep_unix.pl#47
Comment 12 Robert Helmer [:rhelmer] 2007-10-17 12:07:46 PDT
I don't have time to land this and watch for bustage right now, returning to the build pool in the meantime in case someone else does.
Comment 13 Ted Mielczarek [:ted.mielczarek] 2007-10-24 18:42:22 PDT
I checked this in for you, but I stuck an Error: at the beginning.  This might break balsa on the 1.8 branch, and it might break nye on the SeaMonkey tree, but we can probably fix them via tinder-config.  I'll watch and see.
Comment 14 Ted Mielczarek [:ted.mielczarek] 2007-10-24 19:23:54 PDT
FWIW, this works:
http://tinderbox.mozilla.org/showlog.cgi?log=Mozilla1.8/1193277180.1193278532.6501.gz
Comment 15 Ted Mielczarek [:ted.mielczarek] 2007-10-24 19:31:23 PDT
Created attachment 286109 [details] [diff] [review]
set leak threshold on balsa

I checked this in to MOZILLA_1_8_BRANCH_test_mem to fix balsa.  balsa leaks a lot.  It also seems reasonable that maybe the 1.8 branch tinderboxes shouldn't be running the trunk tinderbox code, but that's probably just more crap to deal with.
Comment 16 Ted Mielczarek [:ted.mielczarek] 2007-10-24 20:23:00 PDT
Conveniently balsa has gone AWOL, I filed bug 401051 on that.
Comment 17 Robert Helmer [:rhelmer] 2007-10-25 10:37:36 PDT
Comment on attachment 286109 [details] [diff] [review]
set leak threshold on balsa

r+ with trailing semicolon as per bug 401051#c2 :)

Note You need to log in before you can comment on or make changes to this bug.