mochitest leak test failures should TinderboxPrint a line for the failure

VERIFIED FIXED in mozilla1.9.1b3

Status

VERIFIED FIXED
10 years ago
10 years ago

People

(Reporter: dbaron, Assigned: Waldo)

Tracking

Trunk
mozilla1.9.1b3
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

When the mochitest (or chrome or browser) tests fail due to going over the leak threshold, they should TinderboxPrint a line like 

TinderboxPrint: mochitest <em class="testfail">leaks</em>

so that it's clear that the reason for the orange was leaks in the mochitest run.


I'm not sure whether this code should live on the buildbot side or in runtests.py.in, although it looks from existing precedent that it should probably be the former.
(Assignee)

Comment 1

10 years ago
Created attachment 352073 [details] [diff] [review]
Patch

Output if Mochitests crash:
  FAIL

Output if Mochitests have unexpected failures/passes:
  x/y/z

Output if Mochitests have unexpected failures/passes and leak:
  x/y/z LEAK

Untested, but not difficult to read and execute manually.

Note that output if the leak threshold isn't exceeded is like so and won't trigger the matching:

TEST-PASS | runtests-leaks | WARNING leaked 76 bytes during test execution (threshold set at 75000 bytes)
Assignee: nobody → jwalden+bmo
Status: NEW → ASSIGNED
Attachment #352073 - Flags: review?(bhearsum)
Comment on attachment 352073 [details] [diff] [review]
Patch

>Index: mozilla/tools/buildbotcustom/unittest/steps.py
>===================================================================

>+            if "during test execution" in line and \
>+               "runtests-leaks" in line and \
>+               "TEST-UNEXPECTED-FAIL" in line:
>+               match = re.search(r"leaked (\d+) bytes during test execution", line)

Style nit: Don't indent continuation of the conditional the same amount as the following block, it makes it harder to read. Use a two space indent for continuation. Same thing applies to the ones below.


This looks fine to me. Coop knows more about this code than I though, so I'd like him to review it as well.
Attachment #352073 - Flags: review?(ccooper)
Attachment #352073 - Flags: review?(bhearsum)
Attachment #352073 - Flags: review+
Comment on attachment 352073 [details] [diff] [review]
Patch

No issues here.
Attachment #352073 - Flags: review?(ccooper) → review+
(Assignee)

Comment 4

10 years ago
Created attachment 352168 [details] [diff] [review]
With two-space indents

It doesn't seem to me that this makes it any easier to read, but I didn't mind the original anyway.
(Assignee)

Comment 5

10 years ago
Created attachment 352172 [details] [diff] [review]
[checked in] Er, this is what was really meant
Attachment #352168 - Attachment is obsolete: true
Comment on attachment 352172 [details] [diff] [review]
[checked in] Er, this is what was really meant

Going to test this on staging today.
This hasn't caused any problems in staging - I think we can deploy this. Coop, any chance I can convince you to land this during the downtime on Friday? If not, I'll deploy it next week.
Comment on attachment 352172 [details] [diff] [review]
[checked in] Er, this is what was really meant

Checking in steps.py;
/cvsroot/mozilla/tools/buildbotcustom/unittest/steps.py,v  <--  steps.py
new revision: 1.16; previous revision: 1.15
done
Attachment #352172 - Attachment description: Er, this is what was really meant → [checked in] Er, this is what was really meant
Alright, the m-c/m-1.9.1 unittest master has this now - once I see a clean run I'll close this bug.
Argh, I was wrong. This can't be deployed without a master restart - which takes down existing builds, or until we fix bug 469125..
What is the current status ?

For an example I'm interested in:
{
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1229323080.1229327018.18968.gz
WINNT 5.2 mozilla-central moz2-win32-slave07 dep unit test on 2008/12/14 22:38:00

TinderboxPrint: browser<br/><em class="testfail">1239/0/5 LEAK</em>

http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1229303626.1229307475.10785.gz
Linux comm-central dep unit test on 2008/12/14 17:13:46

TEST-UNEXPECTED-FAIL | runtests-leaks | leaked 350823 bytes during test execution (threshold set at 8556 bytes)
but no "TinderboxPrint: ... LEAK".
}

Does "SeaMonkey" need to do/port something additional ? (maybe a simple restart ?)
Version: unspecified → Trunk

Comment 12

10 years ago
(In reply to comment #11)
> Does "SeaMonkey" need to do/port something additional ? (maybe a simple restart
> ?)

SeaMonkey needed an update of buildbotcustom for that, I've done that a few days ago and I just spotted a failure that states "5561/2/29 LEAK" on the Linux column, so this is done there as well.
(In reply to comment #11)
> What is the current status ?

Jeff, any reason not to R/V.Fixed this bug ?

(In reply to comment #12)
> SeaMonkey needed an update of buildbotcustom for that, I've done that a few
> days ago and I just spotted a failure that states "5561/2/29 LEAK" on the Linux
> column, so this is done there as well.

Robert, I saw (you did) that too ;-)
(Assignee)

Comment 14

10 years ago
Yeah, I think we're finished here.
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
V.Fixed, per waterfall output(s).
Status: RESOLVED → VERIFIED
Target Milestone: --- → mozilla1.9.1b3
You need to log in before you can comment on or make changes to this bug.