Make fxdbug-linux-tbox turn orange if RLk is nonzero

RESOLVED FIXED

Status

Release Engineering
Other
P3
enhancement
RESOLVED FIXED
10 years ago
4 years ago

People

(Reporter: Jesse Ruderman, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

10 years ago
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.
Assignee: nobody → build
Component: Testing → Build & Release
Product: Core → mozilla.org
QA Contact: testing → preed
Version: Trunk → other
(Reporter)

Updated

10 years ago
Depends on: 397103
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.
Priority: -- → P3
(Reporter)

Comment 2

10 years ago
Mac is now at 0B :)
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?
Attachment #283619 - Flags: review?
Attachment #283619 - Flags: review? → review?(ccooper)
Assignee: build → rhelmer
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.
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

10 years ago
Comment on attachment 283619 [details] [diff] [review]
first shot, untested, etc.

Looks like it should do the trick.
Attachment #283619 - Flags: review?(ccooper) → review+
Status: NEW → ASSIGNED
OS: Mac OS X → All
Hardware: PC → All
Created attachment 283683 [details] [diff] [review]
print reason for test failure to log

David, is this what you had in mind?
Attachment #283619 - Attachment is obsolete: true
Attachment #283683 - Flags: review?
Attachment #283683 - Flags: review? → review?(dbaron)
(Reporter)

Comment 8

10 years ago
"Number of leaks" should be something like "Number of bytes leaked".
(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 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
Attachment #283683 - Attachment description: print reason for test failure to lo → print reason for test failure to log
Attachment #283683 - Flags: review?(dbaron) → review+
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
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.
Assignee: rhelmer → build
Status: ASSIGNED → NEW
Assignee: build → nobody
QA Contact: mozpreed → build
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.
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
FWIW, this works:
http://tinderbox.mozilla.org/showlog.cgi?log=Mozilla1.8/1193277180.1193278532.6501.gz
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.
Attachment #286109 - Flags: review?(rhelmer)
Conveniently balsa has gone AWOL, I filed bug 401051 on that.
Comment on attachment 286109 [details] [diff] [review]
set leak threshold on balsa

r+ with trailing semicolon as per bug 401051#c2 :)
Attachment #286109 - Flags: review?(rhelmer) → review+
(Assignee)

Updated

4 years ago
Product: mozilla.org → Release Engineering
You need to log in before you can comment on or make changes to this bug.