904322 - [graph server] Do not send regression alerts for known-bad data

Status: RESOLVED FIXED

(Release Engineering :: General, defect)

Description

[Matt Brubeck (:mbrubeck)]

To reduce noise from false positives on the mailing list, we should add an option to analysis.cfg to temporarily suppress email for datasets that are known to be bad/corrupt, e.g. bug 859571.

Comment 1

[Matt Brubeck (:mbrubeck)]

Attachment: part 1: make the test load its config from analysis.cfg.template

This reduces duplication, and helps make sure we are testing what we are actually deploying.

Comment 2

[Matt Brubeck (:mbrubeck)]

Attachment: part 2: suppress email for known bad data

Comment 3

[Chris AtLee [:catlee]]

Comment on attachment 789630 [details] [diff] [review]
part 2: suppress email for known bad data

Review of attachment 789630 [details] [diff] [review]:
-----------------------------------------------------------------

::: server/analysis/analysis.cfg.template
@@ +24,5 @@
>  ignore_percentage_tests = LibXUL Memory during link
>  
> +# Don't send any email alerts for these subjects (comma-separated list of regexps)
> +suppress_email_subjects = Ts.*Paint.* WINNT 6\.(1|2), ; bug 859571
> +    Tp5 Optimized Responsiveness - WINNT 6\.(1|2), ; bug 751975

doesn't the trailing comma here cause the set of regular expressions to be ["Ts.*Paint.* WINNT 6\.(1|2)", "Tp5 Optimized Responsiveness - WINNT 6\.(1|2)", ""]? And then the empty pattern matches all tests.

Comment 4

[Matt Brubeck (:mbrubeck)]

(In reply to Chris AtLee [:catlee] from comment #3)
> > +suppress_email_subjects = Ts.*Paint.* WINNT 6\.(1|2), ; bug 859571
> > +    Tp5 Optimized Responsiveness - WINNT 6\.(1|2), ; bug 751975
> 
> doesn't the trailing comma here cause the set of regular expressions to be
> ["Ts.*Paint.* WINNT 6\.(1|2)", "Tp5 Optimized Responsiveness - WINNT
> 6\.(1|2)", ""]? And then the empty pattern matches all tests.

It looks like it's actually picking up "; bug 751975" even though ";" is supposed to begin an inline comment.  (I think the comment parsing in ConfigParser is a bit wonky.)  I'll remove the trailing comma and just use # comments on a separate line.

Comment 5

[Matt Brubeck (:mbrubeck)]

Attachment: part 2: suppress email for known bad data (v2)

Addresses issue found in review.

Comment 6

[Matt Brubeck (:mbrubeck)]

http://hg.mozilla.org/graphs/rev/ae907a51f5f3
http://hg.mozilla.org/graphs/rev/34f664a03e4d

Comment 7

[Matt Brubeck (:mbrubeck)]

Note: This requires a change to analysis.cfg in production when it is deployed.

Comment 8

[Matt Brubeck (:mbrubeck)]

Attachment: follow-up: fix overly broad regex

Oops!  I used /Ts.*Paint/ because I wanted to match both "Ts, Paint" and "Ts Paint, (MAX|MED) Dirty Profile" -- but this also matches "tscroll-MozAfterPaint".

This adjusts the regex to use /\bTs\b/ so it won't match "ts" within another word, and adds some more test cases.

Comment 9

[Matt Brubeck (:mbrubeck)]

Comment on attachment 796366 [details] [diff] [review]
follow-up: fix overly broad regex

http://hg.mozilla.org/graphs/rev/0b168c847cc2