Read in gtest_exclude files before running ASAN.

Read in gtest_exclude files before running ASAN.

BUG=122219
TEST=none

[Lei Zhang, 2012-04-05 21:53:37]

Test run with locally modified runtest.py here:
http://build.chromium.org/p/chromium.memory/builders/ASAN%20Tests%20%282%29/b...

[Timur Iskhodzhanov, 2012-04-06 07:03:24]

-me
+glider

[Alexander Potapenko, 2012-04-06 07:32:52]

LGTM with nits.

BTW do we need something like this for the regular bots?

https://chromiumcodereview.appspot.com/9949032/diff/1/scripts/slave/runtest.py
File scripts/slave/runtest.py (right):

https://chromiumcodereview.appspot.com/9949032/diff/1/scripts/slave/runtest.p...
scripts/slave/runtest.py:156: if line.startswith('#') or line.startswith('//')
or line.isspace():
I don't mind treating '//' as a comment line, but we've many places where it is
not treated as such. Won't this confuse the users?

https://chromiumcodereview.appspot.com/9949032/diff/1/scripts/slave/runtest.p...
scripts/slave/runtest.py:173: gtest_filter = '-'
How about joining lines 173 and 174?
(and maybe 175?)

[Reid Kleckner, 2012-04-06 15:46:18]

Is this necessary?

You can already change the source to disable the test conditionally on the
ADDRESS_SANITIZER macro:
http://code.google.com/searchframe#OAMlx_jo-ck/src/base/process_util_unittest...

ifdefs are ugly but they also live with the source, which means they're more
likely to get noticed and fixed.

[Alexander Potapenko, 2012-04-06 16:19:20]

On 2012/04/06 15:46:18, Reid Kleckner wrote:
> Is this necessary?
> 
> You can already change the source to disable the test conditionally on the
> ADDRESS_SANITIZER macro:
>
http://code.google.com/searchframe#OAMlx_jo-ck/src/base/process_util_unittest...
> 
> ifdefs are ugly but they also live with the source, which means they're more
> likely to get noticed and fixed.

We currently have the same gtest machinery for the rest of memory bots.
The advantage is that enabling/disabling the tests does not require
recompilation. The disadvantage is that the devs will be required to pass these
gtest_flags manually if they run the tests locally.
After thinking for a while, I think you are right: it's better to have the same
setup for the developers.

[Reid Kleckner (google), 2012-04-06 16:40:43]

On Fri, Apr 6, 2012 at 9:19 AM, <glider@chromium.org> wrote:

> On 2012/04/06 15:46:18, Reid Kleckner wrote:
>
>> Is this necessary?
>>
>
>  You can already change the source to disable the test conditionally on the
>> ADDRESS_SANITIZER macro:
>>
>
> http://code.google.com/**searchframe#OAMlx_jo-ck/src/**
> base/process_util_unittest.cc&**exact_package=chromium&q=**
>
address_sanitizer&l=227<http://code.google.com/searchframe#OAMlx_jo-ck/src/base/process_util_unittest.cc&exact_package=chromium&q=address_sanitizer&l=227>
>
>  ifdefs are ugly but they also live with the source, which means they're
>> more
>> likely to get noticed and fixed.
>>
>
> We currently have the same gtest machinery for the rest of memory bots.
> The advantage is that enabling/disabling the tests does not require
> recompilation. The disadvantage is that the devs will be required to pass
> these
> gtest_flags manually if they run the tests locally.
> After thinking for a while, I think you are right: it's better to have the
> same
> setup for the developers.
>

Good point.

The memory waterfall exclusion mechanism also tends to rot.  Often devs
will write a new test case using the same techniques that make the excluded
test case incompatible with the tool and not update the exclusions, leading
to more redness.

Or they'll delete or rename the test and the exclusions stay behind.
 Consider all the ui_tests jam@ has recently converted to browser_tests, I
bet we still have exclusions for those.

Reid

[Lei Zhang, 2012-04-06 20:53:41]

On 2012/04/06 16:40:43, Reid Kleckner (google) wrote:
> On Fri, Apr 6, 2012 at 9:19 AM, <mailto:glider@chromium.org> wrote:
> 
> > On 2012/04/06 15:46:18, Reid Kleckner wrote:
> >
> >> Is this necessary?
> >>
> >
> >  You can already change the source to disable the test conditionally on the
> >> ADDRESS_SANITIZER macro:
> >>
> >
> > http://code.google.com/**searchframe#OAMlx_jo-ck/src/**
> > base/process_util_unittest.cc&**exact_package=chromium&q=**
> >
>
address_sanitizer&l=227<http://code.google.com/searchframe#OAMlx_jo-ck/src/base/process_util_unittest.cc&exact_package=chromium&q=address_sanitizer&l=227>
> >
> >  ifdefs are ugly but they also live with the source, which means they're
> >> more
> >> likely to get noticed and fixed.
> >>
> >
> > We currently have the same gtest machinery for the rest of memory bots.
> > The advantage is that enabling/disabling the tests does not require
> > recompilation. The disadvantage is that the devs will be required to pass
> > these
> > gtest_flags manually if they run the tests locally.
> > After thinking for a while, I think you are right: it's better to have the
> > same
> > setup for the developers.
> >
> 
> Good point.
> 
> The memory waterfall exclusion mechanism also tends to rot.  Often devs
> will write a new test case using the same techniques that make the excluded
> test case incompatible with the tool and not update the exclusions, leading
> to more redness.
> 
> Or they'll delete or rename the test and the exclusions stay behind.
>  Consider all the ui_tests jam@ has recently converted to browser_tests, I
> bet we still have exclusions for those.
> 
> Reid

In that case, I'll retract this CL and use the ADDRESS_SANITIZER macro instead.