Bug 38936 – sheriffbot is spammy.

• Component: Tools / Tests
• Status: RESOLVED FIXED

[David Levin]

Today the tree went red.
Sherriffbot decided to blame everyone for the failure except for the right change which was 59171.

sheriffbot: dave_levin, ojan: http://trac.webkit.org/changeset/59168 might have broken Chromium Win Release
[2:25pm] sheriffbot: abarth, japhet: http://trac.webkit.org/changeset/59169 might have broken Chromium Win Release
[2:25pm] sheriffbot: ggaren: http://trac.webkit.org/changeset/59170 might have broken Chromium Win Release
[2:25pm] sheriffbot: tonikitoo: http://trac.webkit.org/changeset/59165 might have broken Chromium Win Release
[2:25pm] sheriffbot: jianli: http://trac.webkit.org/changeset/59166 might have broken Chromium Win Release

A few possible issues: 
1. Perhaps it would be better to do more of a summary.
2. It is odd that it omitted the right change.
3. Does it keep telling people that they may have broken the build if the build was already broken and didn't turn green yet?

[Adam Barth]

The problem is that SheriffBot detected that 59171 broke Qt Linux Release minimal already.  When Chromium Win Release broke, the blamelist included a bunch of other patches.  In an effort not to spam, it didn't repeat the warning for 59171.

One proposal is to not warn about a blamelist that has some intersection with an already blamed list.  That will reduce false positives in cases like this at the cost of missing some warnings about concurrent failures.

[Adam Barth]

Created attachment 55792 [details]
Patch

[Eric Seidel (no email)]

Comment on attachment 55792 [details]
Patch

WebKitTools/Scripts/webkitpy/tool/commands/sheriffbot.py:64
 +          # been that skippign this check causes a lot of spam for builders that
skippign

Makes sense.

I still think our sheriffbot arch is less than optimal.  I would like him eventually to hold in memory and increasingly good picture of what went wrong.  So that one can ask him "what's broken" and he can tell you the exact revision and what it broke.  Right now he just warns as he gets information and doesn't keep around any bigger picture.

Ideally we should cross-reference builds to figure out what actually started when and hold multiple "break" objects or something in memory.  Then we would warn about them every 30m or something until they got fixed, and could explain all the information we know about them over IRC when asked.

[Adam Barth]

Committed r59204: <http://trac.webkit.org/changeset/59204>

[Eric Seidel (no email)]

Committed r59206: <http://trac.webkit.org/changeset/59206>