448416 - Buildbotcustom should have leakThreshold option for Mochitests

Status: RESOLVED FIXED

(Release Engineering :: General, defect)

Description

[Justin Wood (:Callek)]

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

runtests.py currently accepts this as --leakThreshold=num

Comment 1

[[e-mail] (:bhearsum)]

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.

Comment 2

[Justin Wood (:Callek)]

Attachment: Patch for buildbotcustom

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)

Comment 3

[[e-mail] (: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?

Comment 4

[Justin Wood (:Callek)]

Attachment: make buildbot config use the new option [checked-in]

This is the only occurance of this class being used, so the only place I actually need to patch this.

Comment 5

[[e-mail] (:bhearsum)]

Comment on attachment 331840 [details] [diff] [review]
make buildbot config use the new option [checked-in]

looks fine to me...

Comment 6

[Justin Wood (:Callek)]

Comment on attachment 331840 [details] [diff] [review]
make buildbot config use the new option [checked-in]

checked in on changeset 	7c5cf375160a

Comment 7

[Justin Wood (:Callek)]

Attachment: Patch for buildbotcustom [checked in]

Addresses bhearsum's nits and also fixes one minor error I had in last patch (self.command not command by itself)

Comment 8

[Justin Wood (:Callek)]

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

Comment 9

[Justin Wood (:Callek)]

Fixed, just needs a push to the buildbot machines.

Comment 10

[Serge Gautherie (:sgautherie)]

(In reply to comment #9)

Has it been pushed yet ?

Comment 11

[[e-mail] (:bhearsum)]

To which? It hasn't been pushed to Firefox machines yet, but that's probably not urgent.

Comment 12

[Lukas Blakk [:lsblakk] use ?needinfo]

It's been pushed to moz2 unittest, but I haven't had downtime to restart the master yet.

Comment 13

[Robert Kaiser]

(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.

Comment 14

[Serge Gautherie (:sgautherie)]

Yes, I was thinking about SeaMonkey.
See bug 450983 now ;-)