617441 - stop running trace-malloc tests

Status: RESOLVED FIXED

(Release Engineering :: General, defect, P3)

Description

[[e-mail] (:bhearsum)]

From e-mail:
On Tuesday 2010-12-07 15:32 -0500, Ben Hearsum wrote:
> > On 12/07/10 03:30 PM, Ehsan Akhgari wrote:
>> > >Hi David,
>> > >
>> > >I was pointing out to Ben just now that one of the test suites which is
>> > >network dependent is the leaktest, and I remembered that I can actually
>> > >question the very need of running those tests.  He said you're the best
>> > >person to talk to about this, so here's my question.  Is there any
>> > >actual value in running the leaktest suite?  Are we expecting to catch
>> > >certain types of leaks in this suite which we wouldn't be able to using
>> > >our mochitest framework?
>> > >
>> > >I think if we do want to test whether we leak on this very small subset
>> > >of web pages, we should rewrite the suite to be based on mochitest and
>> > >just load stuff from an httpd.js based local server.
>> > >
>> > >What do you think?
>> > >Ehsan
> > 
> > Just to be crystal clear: we're talking about the thing that gets
> > run inside of the "leak test" builders, launches firefox with
> > --trace-malloc, and cycles through some pages on mozilla.org.
It's the only thing we're testing with --trace-malloc leak testing,
so it's the only thing that we run that catches leaks that don't
involve nsTraceRefcnt-logged objects (except for ones that are big
enough to affect the Tp memory use metrics).  That said, I'm not
sure we look at either set of numbers (trace-malloc leaks or Tp
memory use metrics).

I guess I'd be ok with just dropping it.

-David


Probably should go out to the newsgroups before happening, too.

Comment 1

[Phil Ringnalda (:philor)]

http://groups.google.com/group/mozilla.dev.platform/browse_thread/thread/d259236ed04af1e4#

Comment 2

[Phil Ringnalda (:philor)]

Landed bug 620273, so it's no longer the case that it hits the network. I did offer some other reasons to kill it, in the newsgroup thread, though some of them were just my not understand the ways in which we're running things wrong, which would be cleaned up by bug 620282, and the noisiness and the resulting way we ignore the results may well be fixed by bug 620273, we'll see.

Comment 3

[Chris AtLee [:catlee]]

Are we disabling both 'make leaktest' and 'make leaktest --trace-malloc', or just the tracemalloc tests?

Comment 4

[Chris AtLee [:catlee]]

Attachment: stop running leak tests

Comment 5

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

I'll review this patch on Sunday PT.

Comment 6

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

Comment on attachment 777497 [details] [diff] [review]
stop running leak tests

Looks fairly close, just some leftover references to leakTest* and leakTarget:
process/factory.py
773:                 ausHost=None, nightly=False, leakTest=False, leakTarget=None,
847:        self.leakTest = leakTest
1037:        if self.uploadSymbols or (not self.disableSymbols and (self.packageTests or self.leakTest)):
1908:        self.leakTestReferenceRepo = 'mozilla-central'
1910:            self.leakTestReferenceRepo = 'comm-central'
r+ if those can go away on checkin, and maybe check with pylint or something.

Followup up patch to remove enable_leaktests, run_alive_tests, run_make_alive_tests from buildbot-configs ?

Comment 7

[Chris AtLee [:catlee]]

Attachment: stop running leak tests

removed leakTest and leakTarget from __init__ and in misc.py. There's one bit of logic that depended on leakTest being enabled, but looks like it doesn't affect any of our existing configurations.

Comment 8

[Chris AtLee [:catlee]]

Attachment: remove unused configuration options

this is a noop once the buildbotcustom patch is applied

Comment 9

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

Comment on attachment 779922 [details] [diff] [review]
stop running leak tests

The interdiff looks good.

Comment 10

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

Comment on attachment 779925 [details] [diff] [review]
remove unused configuration options

This misses:
* 1 enable_leaktests in mozilla/b2g_config.py
* 2 enable_leaktests in mozilla/project_branches.py
* 1 leak_target, 9 run_alive_tests and 4 enable_leaktests in mozilla/thunderbird_config.py

A potential spanner in the works is thunderbird_config setting 
  'leak_target': 'mailbloat'

Comment 11

[Chris AtLee [:catlee]]

Attachment: remove unused configuration options

Comment 12

[Mark Banner (:standard8)]

(In reply to Nick Thomas [:nthomas] from comment #10)
> A potential spanner in the works is thunderbird_config setting 
>   'leak_target': 'mailbloat'

Shouldn't be, it was just a specialised form of trace-malloc test relevant to Thunderbird.

Comment 13

[Rail Aliiev [:rail]]

In production