720985 - Report a failure when a test leaks a global variable

Status: RESOLVED FIXED

(Testing :: Mochitest, defect)

Comment 1

[Dão Gottwald [:dao]]

Attachment: rough patch

this works, but we'll need to whitelist a few expected properties and known failures

Comment 2

[Jesse Ruderman]

O(n²) :(

Comment 3

[Dão Gottwald [:dao]]

(In reply to Jesse Ruderman from comment #2)
> O(n²) :(

This is only testing code. As long as it doesn't cause the test suite to take significantly longer, who cares?

Comment 4

[Dão Gottwald [:dao]]

Attachment: patch

Comment 5

[:Gavin Sharp [email: [e-mail]]]

Why do each of those properties need to be in the whitelist?

Comment 6

[Dão Gottwald [:dao]]

"navigator", "constructor" and "Application" because they seem to enter the keys list lazily. "__SS_tabsToRestore" are more like known real failures "webConsoleCommandController" and wouldn't be needed ideally.

Comment 7

[:Gavin Sharp [email: [e-mail]]]

(In reply to Dão Gottwald [:dao] from comment #3)
> As long as it doesn't cause the test suite to take significantly longer, who cares?

Seems reasonable - did you test whether it has any impact on test suite run time?

Comment 8

[Dão Gottwald [:dao]]

(In reply to Gavin Sharp (use [e-mail] for email) from comment #7)
> Seems reasonable - did you test whether it has any impact on test suite run
> time?

I tested this on try (linux64/macosx64/win32 x debug/opt) and saw no meaningful difference. Sometimes it took a minute less, sometimes more, most of the time the same. If there's a difference I think it adds up to a few seconds at most. Many single tests probably take longer.

(In reply to Dão Gottwald [:dao] from comment #6)
> "__SS_tabsToRestore" are more like known real failures
> "webConsoleCommandController" and wouldn't be needed ideally.

Hm, that sentence fell apart.

"webConsoleCommandController" and "__SS_tabsToRestore" are more like known real failures and wouldn't be needed ideally.

Comment 9

[Dão Gottwald [:dao]]

https://hg.mozilla.org/mozilla-central/rev/ecc06df9348b

Comment 10

[Dão Gottwald [:dao]]

http://hg.mozilla.org/mozilla-central/rev/da45fc5e6b95

Comment 11

[Matt Brubeck (:mbrubeck)]

Attachment: followup to whitelist properties leaked by Fennec browser-chrome tests

Comment 12

[Matt Brubeck (:mbrubeck)]

Followup landed: https://hg.mozilla.org/mozilla-central/rev/e9183168a27d

Filed bug 723953 to fix the Fennec tests for real.