Closed Bug 448416 Opened 12 years ago Closed 12 years ago

Buildbotcustom should have leakThreshold option for Mochitests

Categories

(Release Engineering :: General, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: Callek, Assigned: Callek)

References

Details

Attachments

(2 files, 1 obsolete file)

Any project running buildbot should be able to specify explicit leakThresholds for the Mochi tests.

runtests.py currently accepts this as --leakThreshold=num
from irc discussion:

The buildbotcustom classes that run these tests should accept a parameter in their constructor for this. The mozilla-central unittest machines will need a master.cfg change to cope with this.
Attached patch Patch for buildbotcustom (obsolete) — Splinter Review
I am unable to test this easily. From inspection on some other ShellCommand in buildbot configs the win32 changes should be safe (if not, I'll need a slightly different approach)
Assignee: nobody → bugspam.Callek
Status: NEW → ASSIGNED
Attachment #331641 - Flags: review?(bhearsum)
Comment on attachment 331641 [details] [diff] [review]
Patch for buildbotcustom

>--- a/tools/buildbotcustom/unittest/steps.py
>+    def __init__(self, **kwargs):
>+        if 'leakThreshold' in kwargs:
>+            command.append("--leak-threshold=" + kwargs["leakThreshold"])

leakThreshold doesn't need to be passed to parent classes, just use a regular argument. eg:
def __init__(self, leakThreshold)


>@@ -355,6 +365,11 @@
>               "--autorun",
>               "--console-level=INFO",
>               "--close-when-done"]
>+    
>+    def __init__(self, **kwargs):

Same thing here.



Before this is checked in I'd like lsblakk to weigh in - do you think this will break anything?
Attachment #331641 - Flags: review?(lukasblakk)
Attachment #331641 - Flags: review?(bhearsum)
Attachment #331641 - Flags: review+
This is the only occurance of this class being used, so the only place I actually need to patch this.
Attachment #331840 - Flags: review?(bhearsum)
Comment on attachment 331840 [details] [diff] [review]
make buildbot config use the new option [checked-in]

looks fine to me...
Attachment #331840 - Flags: review?(lukasblakk)
Attachment #331840 - Flags: review?(bhearsum)
Attachment #331840 - Flags: review+
Attachment #331840 - Flags: review?(lukasblakk) → review+
Attachment #331641 - Flags: review?(lukasblakk) → review+
Comment on attachment 331840 [details] [diff] [review]
make buildbot config use the new option [checked-in]

checked in on changeset 	7c5cf375160a
Attachment #331840 - Attachment description: make buildbot config use the new option → make buildbot config use the new option [checked-in]
Addresses bhearsum's nits and also fixes one minor error I had in last patch (self.command not command by itself)
Attachment #331641 - Attachment is obsolete: true
Comment on attachment 331934 [details] [diff] [review]
Patch for buildbotcustom [checked in]

Checking in tools/buildbotcustom/unittest/steps.py;
/cvsroot/mozilla/tools/buildbotcustom/unittest/steps.py,v  <--  steps.py
new revision: 1.11; previous revision: 1.10
done
Attachment #331934 - Attachment description: Patch for buildbotcustom, for checkin → Patch for buildbotcustom [checked in]
Fixed, just needs a push to the buildbot machines.
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
(In reply to comment #9)

Has it been pushed yet ?
To which? It hasn't been pushed to Firefox machines yet, but that's probably not urgent.
It's been pushed to moz2 unittest, but I haven't had downtime to restart the master yet.
(In reply to comment #10)
> Has it been pushed yet ?

Serge, if you mean SeaMonkey, that needs me logging into the master and pulling/reconfiguring, which I only can do when I'm back from vacation, i.e. not before Monday.
Blocks: 450983
Yes, I was thinking about SeaMonkey.
See bug 450983 now ;-)
No longer blocks: SmTestLeak
Blocks: 451814
Product: mozilla.org → Release Engineering
You need to log in before you can comment on or make changes to this bug.