Closed Bug 726514 Opened 12 years ago Closed 12 years ago

Blacklist test_process_error.xul in leak analysis


(Tree Management Graveyard :: TBPL, defect)

Not set


(Not tracked)



(Reporter: philor, Assigned: ewong)



(Whiteboard: [sheriff-want])


(1 file, 2 obsolete files)

Pretty much every mochitest-other leak analysis claims "chrome://mochitests/content/chrome/dom/ipc/tests/test_process_error.xul leaked 1 DOMWINDOW(s)" even when the leak was in some other sub-suite. It's never true, and it only leads to confusion and invalid bugs. We should just never output that line.
An example:
Rev3 WINNT 6.1 mozilla-central debug test mochitest-other on 2012-02-17 12:28:06 PST for push eb85fbbeb6d9
TEST-UNEXPECTED-FAIL | automationutils.processLeakLog() | leaked 448 bytes during test execution
TEST-UNEXPECTED-FAIL | automationutils.processLeakLog() | leaked 1 instance of CondVar with size 16 bytes
TEST-UNEXPECTED-FAIL | automationutils.processLeakLog() | leaked 1 instance of Mutex with size 12 bytes
TEST-UNEXPECTED-FAIL | automationutils.processLeakLog() | leaked 1 instance of nsCacheEntryHashTable with size 36 bytes
TEST-UNEXPECTED-FAIL | automationutils.processLeakLog() | leaked 1 instance of nsCacheService with size 148 bytes
TEST-UNEXPECTED-FAIL | automationutils.processLeakLog() | leaked 2 instances of nsDiskCacheBinding with size 52 bytes each (104 bytes total)
mochitest-browser-chrome: 26350/0/91 LEAK
chrome://mochitests/content/chrome/dom/ipc/tests/test_process_error.xul leaked 1 DOMWINDOW(s)
Severity: normal → major
Severity: major → normal
Whiteboard: [sheriff-want]
Assignee: nobody → ewong
Attachment #664837 - Flags: review?(bmo)
philor, is the test_process_error.xul 'leak' an actual leak but caused by something else, or not actually a leak at all?

ie: if the former we should just s/test_process_error.xul/(unknown)/ like we do for "Shutdown"; or if the latter, we need to do as in the attached patch, but presumably handle the result == "" case too (if so, what string would you suggest for that?)

Lastly, bug 538462 seems to have been WFM for ages, so do you think we can remove the special casing for test_unknownContentType_dialog_layout.xul?

Cheers :-)
It's not a leak at all, it just does something tricky so there's no --DOMWINDOW to see. The typical case is that we have a browser-chrome leak, mochitest-chrome didn't leak at all, and we show it as being the 2 domwindows that were leaked despite it being in a non-leaking suite.

I'd rather not remove the test_unknownContentType_dialog_layout.xul thing unless we replace it, since that's a reminder of what the page is supposed to show. We could stick in hints for bug 730746 and bug 787312 instead - neither one is all that frequent, but they're more frequent than 538462 :)
Comment on attachment 664837 [details] [diff] [review]
Do not print out test_process_error.xul in leak analysis. (v1)

Really sorry for the delay, last week was the end of the quarter so had a bunch of stuff get in the way.

AIUI, we need to cover the case where this was the only leak showing, but we've now hidden it & don't display anything. We should fall back to the "No DOMWINDOWs leaked!" case.

Also, if possible, would you mind replacing the test_unknownContentType_dialog_layout.xul case with the ones suggested by philor in comment 4.
Attachment #664837 - Flags: review?(bmo)
Attachment #664837 - Attachment is obsolete: true
Attachment #669954 - Flags: review?(bmo)
Comment on attachment 669954 [details] [diff] [review]
Blacklist test_process_error.xul in leak analysis. (v2)

Looks good, the only thing needing changing is that the |$result = "No DOMWINDOWs leaked!";| lost its <div>

With that fixed, I'll be happy to r+ :-)
Attachment #669954 - Flags: review?(bmo)
Attachment #669954 - Attachment is obsolete: true
Attachment #672212 - Flags: review?(bmo)
I spotted one last change:
> if ($results == '')
needs to be:
> if ($result == '')

But I fixed that locally & testing with Vagrant looked good, so landed as:

Thank you for the patch! :-)
Attachment #672212 - Flags: review?(bmo) → review+
Depends on: 804021
Closed: 12 years ago
Resolution: --- → FIXED
Product: Webtools → Tree Management
Product: Tree Management → Tree Management Graveyard
You need to log in before you can comment on or make changes to this bug.