Avoid duplicate error reports / suppressions when UI test reports are...

Avoid duplicate error reports / suppressions when UI test reports are analyzed separately

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=54118

[Timur Iskhodzhanov, 2010-07-28 16:17:18]

Alexander, Lei,

After my recent patch
http://src.chromium.org/viewvc/chrome?view=rev&revision=53583
the new problem appeared.
If two tests have the same error, we will have two similar reports and
suppressions even if they match exactly.
This happens because MemcheckAnalyze object is created for each UI test.

In this CL I re-factor the code a bit so we create only one MemcheckAnalyze for
each chrome_tests.sh run
and call Report() for each UI test.

===NOTE!===
This is a draft patch.
Memcheck part is ready for your review (I think so),
tsan_analyze.py is not yet - I hope to prepare a final version of the patch
tomorrow.
By now - I'd really appreciate your feedback on the output.
Also, feel free to show me the places in the new code where you think I should
add comments (if any).
===NOTE!===

Example of the output:

test B follows test A and has the same error reported by Valgrind.
test B error report is shortened to a hash-only string with the same has as in
the test A report.
This avoids duplication while you can still see how many tests are affected by
that specific error.

============
test A report
-----------------------------------------------------
Suppressions used:
  count name
-----------------------------------------------------
ERROR:root:FAIL! There were 1 errors: 
ERROR:root:Command: ...
Leak_DefinitelyLost
3,658 (32 direct, 3,626 indirect) bytes in 2 blocks are definitely lost in loss
record 8,651 of 8,807
  malloc
(/usr/local/google/valgrind-for-chromium-client/valgrind/scripts/valgrind-memcheck/coregrind/m_replacemalloc/vg_replace_malloc.c:236)
  malloc (/home/timurrrr/ULGC/src/base/process_util_linux.cc:583)
Suppression:
{
   <insert_a_suppression_name_here> hash=#5166929671642706188#
   Memcheck:Leak
   fun:malloc
   fun:malloc
}
============
============
test B report
-----------------------------------------------------
Suppressions used:
  count name
      2 fontconfig_bug_8428
-----------------------------------------------------
ERROR:root:FAIL! There were 1 errors: 
ERROR:root:Error hash #5166929671642706188# (seen before)
============

[Timur Iskhodzhanov, 2010-07-29 08:39:39]

Re-factored the TSan part;
Slightly changed the hash printing - now it's hex and the "seen before" line is
different:
"This error was already printed in some other test, see
'hash=#722E9BE07B77DED0#'"

On 2010/07/28 16:17:18, Timur Iskhodzhanov wrote:
> Alexander, Lei,
> 
> After my recent patch
> http://src.chromium.org/viewvc/chrome?view=rev&revision=53583
> the new problem appeared.
> If two tests have the same error, we will have two similar reports and
> suppressions even if they match exactly.
> This happens because MemcheckAnalyze object is created for each UI test.
> 
> In this CL I re-factor the code a bit so we create only one MemcheckAnalyze
for
> each chrome_tests.sh run
> and call Report() for each UI test.
> 
> ===NOTE!===
> This is a draft patch.
> Memcheck part is ready for your review (I think so),
> tsan_analyze.py is not yet - I hope to prepare a final version of the patch
> tomorrow.
> By now - I'd really appreciate your feedback on the output.
> Also, feel free to show me the places in the new code where you think I should
> add comments (if any).
> ===NOTE!===
> 
> Example of the output:
> 
> test B follows test A and has the same error reported by Valgrind.
> test B error report is shortened to a hash-only string with the same has as in
> the test A report.
> This avoids duplication while you can still see how many tests are affected by
> that specific error.
> 
> ============
> test A report
> -----------------------------------------------------
> Suppressions used:
>   count name
> -----------------------------------------------------
> ERROR:root:FAIL! There were 1 errors: 
> ERROR:root:Command: ...
> Leak_DefinitelyLost
> 3,658 (32 direct, 3,626 indirect) bytes in 2 blocks are definitely lost in
loss
> record 8,651 of 8,807
>   malloc
>
(/usr/local/google/valgrind-for-chromium-client/valgrind/scripts/valgrind-memcheck/coregrind/m_replacemalloc/vg_replace_malloc.c:236)
>   malloc (/home/timurrrr/ULGC/src/base/process_util_linux.cc:583)
> Suppression:
> {
>    <insert_a_suppression_name_here> hash=#5166929671642706188#
>    Memcheck:Leak
>    fun:malloc
>    fun:malloc
> }
> ============
> ============
> test B report
> -----------------------------------------------------
> Suppressions used:
>   count name
>       2 fontconfig_bug_8428
> -----------------------------------------------------
> ERROR:root:FAIL! There were 1 errors: 
> ERROR:root:Error hash #5166929671642706188# (seen before)
> ============

[Alexander Potapenko, 2010-07-29 10:36:38]

LG so far

http://codereview.chromium.org/3056025/diff/14001/15001
File tools/valgrind/memcheck_analyze.py (right):

http://codereview.chromium.org/3056025/diff/14001/15001#newcode354
tools/valgrind/memcheck_analyze.py:354: show_all_leaks: whether to show even
less important leaks
Please describe |use_gdb| here as well.

http://codereview.chromium.org/3056025/diff/14001/15001#newcode500
tools/valgrind/memcheck_analyze.py:500: sys.stdout.flush()
Please add a comment that describes a reason for using flush() here

[Timur Iskhodzhanov, 2010-07-29 10:57:51]

http://codereview.chromium.org/3056025/diff/14001/15001
File tools/valgrind/memcheck_analyze.py (right):

http://codereview.chromium.org/3056025/diff/14001/15001#newcode354
tools/valgrind/memcheck_analyze.py:354: show_all_leaks: whether to show even
less important leaks
On 2010/07/29 10:36:38, Alexander Potapenko wrote:
> Please describe |use_gdb| here as well.

Done.

http://codereview.chromium.org/3056025/diff/14001/15001#newcode500
tools/valgrind/memcheck_analyze.py:500: sys.stdout.flush()
Removed it.
It was used during local testing to flush temporary logging prints
On 2010/07/29 10:36:38, Alexander Potapenko wrote:
> Please add a comment that describes a reason for using flush() here

[Timur Iskhodzhanov, 2010-07-29 11:33:14]

I've commited the CL so we can revert if anything goes wrong before Mt.V office
wakes up

On 2010/07/29 10:57:51, Timur Iskhodzhanov wrote:
> http://codereview.chromium.org/3056025/diff/14001/15001
> File tools/valgrind/memcheck_analyze.py (right):
> 
> http://codereview.chromium.org/3056025/diff/14001/15001#newcode354
> tools/valgrind/memcheck_analyze.py:354: show_all_leaks: whether to show even
> less important leaks
> On 2010/07/29 10:36:38, Alexander Potapenko wrote:
> > Please describe |use_gdb| here as well.
> 
> Done.
> 
> http://codereview.chromium.org/3056025/diff/14001/15001#newcode500
> tools/valgrind/memcheck_analyze.py:500: sys.stdout.flush()
> Removed it.
> It was used during local testing to flush temporary logging prints
> On 2010/07/29 10:36:38, Alexander Potapenko wrote:
> > Please add a comment that describes a reason for using flush() here