Bug 46703 - Move more SheriffBot smarts into FailureMap
Summary: Move more SheriffBot smarts into FailureMap
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Adam Barth
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-09-28 01:13 PDT by Adam Barth
Modified: 2010-09-29 19:54 PDT (History)
1 user (show)

See Also:

Attachments
Work in progress (10.56 KB, patch)
2010-09-28 01:15 PDT, Adam Barth 		no flags Details | Formatted Diff | Diff
Patch (16.13 KB, patch)
2010-09-29 19:35 PDT, Adam Barth 		eric: review+
eric: commit-queue-
Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Adam Barth 2010-09-28 01:13:14 PDT 
Move more SheriffBot smarts into FailureMap
Comment 1 Adam Barth 2010-09-28 01:15:31 PDT 
Created attachment 69030 [details]
Work in progress
Comment 2 Adam Barth 2010-09-29 19:35:39 PDT 
Created attachment 69297 [details]
Patch
Comment 3 Eric Seidel (no email) 2010-09-29 19:42:49 PDT 
Comment on attachment 69297 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=69297&action=review

In general looks OK.

> WebKitTools/Scripts/webkitpy/common/net/failuremap.py:41
> +        return self._failures == []

return not self._failures maybe?

That will make None empty too.

> WebKitTools/Scripts/webkitpy/common/net/failuremap.py:45
> +        return sorted(set(sum([failure_info['regression_window'].revisions()
> +                               for failure_info in self._failures], [])))

Local variables seems useful here.

> WebKitTools/Scripts/webkitpy/common/net/regressionwindow.py:52
> +        if self._revisions:
> +            return self._revisions
> +        self._revisions = range(self._failing_build.revision(), self._build_before_failure.revision(), -1)
> +        self._revisions.reverse()
> +        return self._revisions

I would have reversed this if.

> WebKitTools/Scripts/webkitpy/tool/commands/sheriffbot.py:61
> +    def _is_old_failure(self):
> +        return self._tool.status_server.svn_revision(svn_revision)

This can't work.  Seems we're missing a unit test?

> WebKitTools/Scripts/webkitpy/tool/commands/sheriffbot.py:67
> +        # FIXME: We need to figure out how to provoke_flaky_builders.

Please file a bug.  If I had known this was broken, I could have fixed it long ago. :(
Comment 4 Adam Barth 2010-09-29 19:54:00 PDT 
Committed r68740: <http://trac.webkit.org/changeset/68740>