456275 - In (Brief/Full) log of (test) builds, remind leak threshold on known leak too

Status: RESOLVED FIXED

(Release Engineering :: General, defect)

Description

[Serge Gautherie (:sgautherie)]

In the full log, I'd like the threshold to be reminded too on

TEST-PASS | runtests-leaks | WARNING leaked 8620 bytes during test execution

as it is on

TEST-UNEXPECTED-FAIL | runtests-leaks | leaked 8688 bytes during test execution
(should have leaked no more than 8620 bytes)


Add something like " (threshold set at 8620 bytes)".

Would help to notice when leak reduces, etc.

Comment 1

[Nick Thomas [:nthomas] (UTC+12)]

Code is at:
http://mxr.mozilla.org/mozilla-central/source/testing/mochitest/runtests.py.in#391

Patches welcome but no time on our side at the moment.

Comment 2

[Serge Gautherie (:sgautherie)]

Attachment: (Av1) sync and extend

*"Unify" report format.
*Add more detailed cases, to help manage threshold (toward its goal of "0").

Comment 3

[Serge Gautherie (:sgautherie)]

Attachment: (Av1a) sync and extend

[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.1b1pre) Gecko/20081003 SeaMonkey/2.0a2pre] (home, optim default) (W2Ksp4)

Fixed and tested.

Comment 4

[Serge Gautherie (:sgautherie)]

Comment on attachment 341573 [details] [diff] [review]
(Av1a) sync and extend

ted, I saw you reviewed bug 456894 ... and I'm looking for an (active) reviewer :-|

Comment 5

[Serge Gautherie (:sgautherie)]

Comment on attachment 341573 [details] [diff] [review]
(Av1a) sync and extend

sayer, ted: ping ?

Comment 6

[(not currently active) Ted Mielczarek]

Comment on attachment 341573 [details] [diff] [review]
(Av1a) sync and extend

You've really complicated the code here for minimal gain. I'd suggest just adding a line or two of code under the |elif bytesLeaked > 0:| where if the threshold != INFINITY, then you can stick your extra text on the end of the message. You've over doubled the number of lines of code here just to get that extra warning.

Comment 7

[Serge Gautherie (:sgautherie)]

Attachment: (Av2) sync and extend [Checkin: Comment 9]

Av1a, with comment 6 suggestion(s):
less code, less details :-|

While there,
*Remove (2) leftover from bug 423235.
*Add (2) missed followups to bug 456894.

Comment 8

[(not currently active) Ted Mielczarek]

Comment on attachment 343996 [details] [diff] [review]
(Av2) sync and extend
[Checkin: Comment 9]

-          log.info("TEST-UNEXPECTED-FAIL | runtests-leaks | leaked %d bytes during test execution (should "
-                   "have leaked no more than %d bytes)",
-                   bytesLeaked, threshold)
+          leakLog = "TEST-UNEXPECTED-FAIL | runtests-leaks | leaked " + \
+                     str(bytesLeaked) + " bytes during test execution"

I'd prefer to keep the string substitution with "%d " % bytesLeaked here (and elsewhere), it's more Python-like.

Other than that, looks fine.

Comment 9

[Serge Gautherie (:sgautherie)]

Comment on attachment 343996 [details] [diff] [review]
(Av2) sync and extend
[Checkin: Comment 9]

http://hg.mozilla.org/mozilla-central/rev/1b5146eb5738
+ comment 8 suggestion(s) =
http://hg.mozilla.org/mozilla-central/rev/699c40b616e1

Sorry: I didn't mean to push my "history" :-(