Closed Bug 1050133 Opened 9 years ago Closed 6 years ago

Port TBPL's DOMWINDOW leak analyser to automationutils.py

Categories

(Testing :: General, defect, P3)

defect

Tracking

(Not tracked)

RESOLVED INCOMPLETE

People

(Reporter: emorley, Unassigned)

Details

(Whiteboard: [treeherder])

Treeherder is missing TBPL's leak analyser feature, which is linked from the error summary box on runs that displayed a leak. Instead of adding this functionality to treeherder, IMO we should just improve the output of the existing leakcheck feature in automationutils.py, rather than having to use an external tool to parse the log again.

TBPL's code is at:
https://hg.mozilla.org/webtools/tbpl/file/default/php/inc/LeakAnalyzer.php#l26
Summary: Port TBPL's leak analyser to automationutils.py → Port TBPL's DOMWINDOW leak analyser to automationutils.py
Assignee: nobody → dminor
Status: NEW → ASSIGNED
khuey, is this code still useful? Ed suggested I double check with someone familiar with the in-tree code before proceeding. Thanks!
Flags: needinfo?(khuey)
I think you should ask some of the front end folks like dao or ttaubert.
Flags: needinfo?(khuey)
Hi, just wanted to check if this functionality is still useful before porting it. Thanks!
Flags: needinfo?(ttaubert)
Flags: needinfo?(dao)
It sounds like you're talking about a the leak analyzer that was written for leaks in native code? I only know about the ShutdownLeaks code which never had an "analyzer". The log messages there are as good as possible and tell which test leaked.
Flags: needinfo?(ttaubert)
Flags: needinfo?(dao)
Ok, sounds like this is no longer useful. Thanks!
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → WONTFIX
No, his answer was "I don't know about that thing, I wrote something different to deal with a different sort of leaks."

This is for leaks like bug 1045822, where we have two clues, the list of leaked objects which is rarely useful for anything, and the list of leaked URLs, which may or may not be useful sometimes with some interpretation, and we used to have this third tool which came from bug 538462 comment 16 which looked for instances of ++DOMWINDOW in the log that didn't have a matching --DOMWINDOW, and listed the active test when one was found, which was sometimes useful.

It was always too confusing (sounding like it was saying "nothing really leaked!" when it was really saying "I don't have anything useful to say about that leak" and requiring that you know what things are false positives) for the sort of primary positioning it got in tbpl back when it worked, but if right above the list of leaked URLs that's only looked at by people who know something, we also had the output of this, there would be cases where it would directly blame a leak on the test which was causing the leak. If I were to pick someone to say whether or not they needed that, I'd pick Andrew.
Flags: needinfo?(continuation)
I've never actually looked at the output of this particular leak analyzer, so I don't know how useful it is in practice.  The ++DOMWINDOW and --DOMWINDOW stuff is less flakey than it used to be, I think due to work by ttaubert.  Like philor said it would be nice to have some little output like this near the leaked URLs list of windows that might be leaked, as it would point more directly at problems.
Flags: needinfo?(continuation)
Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
Status: REOPENED → ASSIGNED
Priority: P2 → P3
I don't believe this should block TBPL EOL, adjusting accordingly.
No longer blocks: treeherder-dev-transition
Not sure I'll have a chance to look at this one any time soon.
Assignee: dminor → nobody
Status: ASSIGNED → NEW
Mass-closing old bugs I filed that have not had recent activity/no longer affect me.
Status: NEW → RESOLVED
Closed: 9 years ago6 years ago
Resolution: --- → INCOMPLETE
You need to log in before you can comment on or make changes to this bug.