Bug 46703 – Move more SheriffBot smarts into FailureMap

• Component: New Bugs
• Status: RESOLVED FIXED

[Adam Barth]

Move more SheriffBot smarts into FailureMap

[Adam Barth]

Created attachment 69030 [details]
Work in progress

[Adam Barth]

Created attachment 69297 [details]
Patch

[Eric Seidel (no email)]

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. :(

[Adam Barth]

Committed r68740: <http://trac.webkit.org/changeset/68740>