Reland of refactor the buildbot module in webkitpy.

Reland of actor the buildbot module in webkitpy. (patchset #1 id:1 of https://codereview.chromium.org/2165983003/ )

Reason for revert:
Creating a re-land CL to fix the issue encountered in https://build.chromium.org/p/chromium.infra.cron/builders/rebaseline-o-matic/builds/396281 and then re-land.

As of the time when this reland patch is created, I still haven't found the cause of the problem.

Original issue's description:
> Revert of Refactor the buildbot module in webkitpy. (patchset #3 id:40001 of https://codereview.chromium.org/2152663003/ )
>
> Reason for revert:
> On 2016/07/20 at 17:51:31, vadimsh wrote:
> > Probably broke rebaseline-o-matic: https://build.chromium.org/p/chromium.infra.cron/builders/rebaseline-o-matic/builds/396281
>
> Original issue's description:
> > Refactor the buildbot module in webkitpy.
> >
> > This CL refactors BuildBot so that:
> >  - There is now a results_url method in the BuildBot class (moved from AbstractRebaseliningCommand)
> >  - Redundant functions are removed
> >  - The Builder and Build classes are removed
> >  - Buildbot-related functions in webkitpy.common.config.urls are moved into BuildBot
> >  - Unit test and mock version are updated
> >
> > The purpose of this is to simplify the code; this is a follow-up to http://crrev.com/2112133002 which removed some unused functionality.
> >
> > Committed: https://crrev.com/66b942da80d7af5806f50630e55a9800102f43e1
> > Cr-Commit-Position: refs/heads/master@{#406599}
>
> TBR=wkorman@chromium.org,dcampb@google.com,dpranke@chromium.org,vadimsh@chromium.org
> # Skipping CQ checks because original CL landed less than 1 days ago.
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
>
> Committed: https://crrev.com/b7c2c15eac19863207b008eaaa7916b726ef8d6c
> Cr-Commit-Position: refs/heads/master@{#406614}

TBR=wkorman@chromium.org,dcampb@google.com,dpranke@chromium.org,vadimsh@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true

Committed: https://crrev.com/c0fee27d4307cb92cbe13e4d519f7c11b025a90b
Cr-Commit-Position: refs/heads/master@{#406933}

[qyearsley, 2016-07-20 22:06:12]

Created Reland of actor the buildbot module in webkitpy.

[qyearsley, 2016-07-20 22:37:40]

The CQ bit was checked by qyearsley@chromium.org to run a CQ dry run

[qyearsley, 2016-07-20 22:37:49]

https://codereview.chromium.org/2164153002/diff/180001/third_party/WebKit/Too...
File third_party/WebKit/Tools/Scripts/webkitpy/common/net/buildbot.py (right):

https://codereview.chromium.org/2164153002/diff/180001/third_party/WebKit/Too...
third_party/WebKit/Tools/Scripts/webkitpy/common/net/buildbot.py:77: return
self.fetch_layout_test_results(self.accumulated_results_url_base(builder_name))
In the previous submitted version, latest_layout_test_results had been removed
and was made @memoized. This was not how I meant it to be --
latest_layout_test_results is still a useful function, and making it memoized
has the advantage of making it so that the same results aren't downloaded
multiple times.

https://codereview.chromium.org/2164153002/diff/180001/third_party/WebKit/Too...
File third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline.py
(right):

https://codereview.chromium.org/2164153002/diff/180001/third_party/WebKit/Too...
third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline.py:319:
builder_results = self._tool.buildbot.latest_layout_test_results(builder_name)
In the previous submitted version, this was calling
`self._tool.buildbot.accumulated_results_url_base`, which of course returns a
string instead of a LayoutTestResults object.

https://codereview.chromium.org/2164153002/diff/180001/third_party/WebKit/Too...
File
third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline_unittest.py
(right):

https://codereview.chromium.org/2164153002/diff/180001/third_party/WebKit/Too...
third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline_unittest.py:1445:
[{'builder': 'MOCK Win7', 'revision': '9000'}])
Adding a test method to add some test coverage -- although this wouldn't have
caught the error in this particular case since _setup_mock_build_data() sets up
build_data() to be a dict with LayoutTestResults, whereas the bug here was that
the real build_data had strings instead of LayoutTestResults objects.

[commit-bot: I haz the power, 2016-07-20 22:38:07]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-20 22:38:09]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-20 22:38:10]

Dry run: This issue passed the CQ dry run.

[qyearsley, 2016-07-20 23:28:43]

On 2016/07/20 at 22:37:49, qyearsley wrote:
>
https://codereview.chromium.org/2164153002/diff/180001/third_party/WebKit/Too...
> File third_party/WebKit/Tools/Scripts/webkitpy/common/net/buildbot.py (right):
> 
>
https://codereview.chromium.org/2164153002/diff/180001/third_party/WebKit/Too...
> third_party/WebKit/Tools/Scripts/webkitpy/common/net/buildbot.py:77: return
self.fetch_layout_test_results(self.accumulated_results_url_base(builder_name))
> In the previous submitted version, latest_layout_test_results had been removed
and was made @memoized. This was not how I meant it to be --
latest_layout_test_results is still a useful function, and making it memoized
has the advantage of making it so that the same results aren't downloaded
multiple times.
> 
>
https://codereview.chromium.org/2164153002/diff/180001/third_party/WebKit/Too...
> File third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline.py
(right):
> 
>
https://codereview.chromium.org/2164153002/diff/180001/third_party/WebKit/Too...
> third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline.py:319:
builder_results = self._tool.buildbot.latest_layout_test_results(builder_name)
> In the previous submitted version, this was calling
`self._tool.buildbot.accumulated_results_url_base`, which of course returns a
string instead of a LayoutTestResults object.
> 
>
https://codereview.chromium.org/2164153002/diff/180001/third_party/WebKit/Too...
> File
third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline_unittest.py
(right):
> 
>
https://codereview.chromium.org/2164153002/diff/180001/third_party/WebKit/Too...
>
third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline_unittest.py:1445:
[{'builder': 'MOCK Win7', 'revision': '9000'}])
> Adding a test method to add some test coverage -- although this wouldn't have
caught the error in this particular case since _setup_mock_build_data() sets up
build_data() to be a dict with LayoutTestResults, whereas the bug here was that
the real build_data had strings instead of LayoutTestResults objects.

Walter, does this reland look OK?

[qyearsley, 2016-07-20 23:29:11]

On 2016/07/20 at 23:28:43, qyearsley wrote:
> On 2016/07/20 at 22:37:49, qyearsley wrote:
> >
https://codereview.chromium.org/2164153002/diff/180001/third_party/WebKit/Too...
> > File third_party/WebKit/Tools/Scripts/webkitpy/common/net/buildbot.py
(right):
> > 
> >
https://codereview.chromium.org/2164153002/diff/180001/third_party/WebKit/Too...
> > third_party/WebKit/Tools/Scripts/webkitpy/common/net/buildbot.py:77: return
self.fetch_layout_test_results(self.accumulated_results_url_base(builder_name))
> > In the previous submitted version, latest_layout_test_results had been
removed and was made @memoized. This was not how I meant it to be --
latest_layout_test_results is still a useful function, and making it memoized
has the advantage of making it so that the same results aren't downloaded
multiple times.
> > 
> >
https://codereview.chromium.org/2164153002/diff/180001/third_party/WebKit/Too...
> > File third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline.py
(right):
> > 
> >
https://codereview.chromium.org/2164153002/diff/180001/third_party/WebKit/Too...
> > third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline.py:319:
builder_results = self._tool.buildbot.latest_layout_test_results(builder_name)
> > In the previous submitted version, this was calling
`self._tool.buildbot.accumulated_results_url_base`, which of course returns a
string instead of a LayoutTestResults object.
> > 
> >
https://codereview.chromium.org/2164153002/diff/180001/third_party/WebKit/Too...
> > File
third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline_unittest.py
(right):
> > 
> >
https://codereview.chromium.org/2164153002/diff/180001/third_party/WebKit/Too...
> >
third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline_unittest.py:1445:
[{'builder': 'MOCK Win7', 'revision': '9000'}])
> > Adding a test method to add some test coverage -- although this wouldn't
have caught the error in this particular case since _setup_mock_build_data()
sets up build_data() to be a dict with LayoutTestResults, whereas the bug here
was that the real build_data had strings instead of LayoutTestResults objects.
> 
> Walter, does this reland look OK?

By the way, thanks for reviewing :-D

[wkorman, 2016-07-21 18:40:47]

lgtm

Did running 'webkit-patch auto-rebaseline' locally with the previous patch
produce the error we saw on the rebaselineomatic bot?

If so we can at least gain confidence this reland won't break things by running
locally again.

If there's a test we can write to rudimentarily test auto-rebaseline to catch
whatever issue happened this last time, could consider adding, but I am
sensitive as discussed to investing too much time in keeping the old thing
running when the new thing you're working on is almost ready.

[qyearsley, 2016-07-21 21:02:43]

On 2016/07/21 at 18:40:47, wkorman wrote:
> lgtm
> 
> Did running 'webkit-patch auto-rebaseline' locally with the previous patch
produce the error we saw on the rebaselineomatic bot?
> 
> If so we can at least gain confidence this reland won't break things by
running locally again.
> 
> If there's a test we can write to rudimentarily test auto-rebaseline to catch
whatever issue happened this last time, could consider adding, but I am
sensitive as discussed to investing too much time in keeping the old thing
running when the new thing you're working on is almost ready.

Just verified that with the previous patch, running `webkit-patch
auto-rebaseline` reproduced the error. Tried it again with this patch, didn't
get the error.

The error was in build_data(), which I think I want to change or remove in a
subsequent refactoring, so I'll leave it as-is now, and make sure to have
coverage for any code added in the future.

[qyearsley, 2016-07-21 21:02:47]

The CQ bit was checked by qyearsley@chromium.org

[commit-bot: I haz the power, 2016-07-21 21:03:24]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-21 21:04:34]

Committed patchset #2 (id:180001)

[commit-bot: I haz the power, 2016-07-21 21:06:55]

Description was changed from

==========
Reland of actor the buildbot module in webkitpy. (patchset #1 id:1 of
https://codereview.chromium.org/2165983003/ )

Reason for revert:
Creating a re-land CL to fix the issue encountered in
https://build.chromium.org/p/chromium.infra.cron/builders/rebaseline-o-matic/...
and then re-land.

As of the time when this reland patch is created, I still haven't found the
cause of the problem.

Original issue's description:
> Revert of Refactor the buildbot module in webkitpy. (patchset #3 id:40001 of
https://codereview.chromium.org/2152663003/ )
> 
> Reason for revert:
> On 2016/07/20 at 17:51:31, vadimsh wrote:
> > Probably broke rebaseline-o-matic:
https://build.chromium.org/p/chromium.infra.cron/builders/rebaseline-o-matic/...
> 
> Original issue's description:
> > Refactor the buildbot module in webkitpy.
> >
> > This CL refactors BuildBot so that:
> >  - There is now a results_url method in the BuildBot class (moved from
AbstractRebaseliningCommand)
> >  - Redundant functions are removed
> >  - The Builder and Build classes are removed
> >  - Buildbot-related functions in webkitpy.common.config.urls are moved into
BuildBot
> >  - Unit test and mock version are updated
> >
> > The purpose of this is to simplify the code; this is a follow-up to
http://crrev.com/2112133002 which removed some unused functionality.
> >
> > Committed: https://crrev.com/66b942da80d7af5806f50630e55a9800102f43e1
> > Cr-Commit-Position: refs/heads/master@{#406599}
> 
>
TBR=wkorman@chromium.org,dcampb@google.com,dpranke@chromium.org,vadimsh@chrom...
> # Skipping CQ checks because original CL landed less than 1 days ago.
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> 
> Committed: https://crrev.com/b7c2c15eac19863207b008eaaa7916b726ef8d6c
> Cr-Commit-Position: refs/heads/master@{#406614}

TBR=wkorman@chromium.org,dcampb@google.com,dpranke@chromium.org,vadimsh@chrom...
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
==========

to

==========
Reland of actor the buildbot module in webkitpy. (patchset #1 id:1 of
https://codereview.chromium.org/2165983003/ )

Reason for revert:
Creating a re-land CL to fix the issue encountered in
https://build.chromium.org/p/chromium.infra.cron/builders/rebaseline-o-matic/...
and then re-land.

As of the time when this reland patch is created, I still haven't found the
cause of the problem.

Original issue's description:
> Revert of Refactor the buildbot module in webkitpy. (patchset #3 id:40001 of
https://codereview.chromium.org/2152663003/ )
>
> Reason for revert:
> On 2016/07/20 at 17:51:31, vadimsh wrote:
> > Probably broke rebaseline-o-matic:
https://build.chromium.org/p/chromium.infra.cron/builders/rebaseline-o-matic/...
>
> Original issue's description:
> > Refactor the buildbot module in webkitpy.
> >
> > This CL refactors BuildBot so that:
> >  - There is now a results_url method in the BuildBot class (moved from
AbstractRebaseliningCommand)
> >  - Redundant functions are removed
> >  - The Builder and Build classes are removed
> >  - Buildbot-related functions in webkitpy.common.config.urls are moved into
BuildBot
> >  - Unit test and mock version are updated
> >
> > The purpose of this is to simplify the code; this is a follow-up to
http://crrev.com/2112133002 which removed some unused functionality.
> >
> > Committed: https://crrev.com/66b942da80d7af5806f50630e55a9800102f43e1
> > Cr-Commit-Position: refs/heads/master@{#406599}
>
>
TBR=wkorman@chromium.org,dcampb@google.com,dpranke@chromium.org,vadimsh@chrom...
> # Skipping CQ checks because original CL landed less than 1 days ago.
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
>
> Committed: https://crrev.com/b7c2c15eac19863207b008eaaa7916b726ef8d6c
> Cr-Commit-Position: refs/heads/master@{#406614}

TBR=wkorman@chromium.org,dcampb@google.com,dpranke@chromium.org,vadimsh@chrom...
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true

Committed: https://crrev.com/c0fee27d4307cb92cbe13e4d519f7c11b025a90b
Cr-Commit-Position: refs/heads/master@{#406933}
==========

[commit-bot: I haz the power, 2016-07-21 21:06:56]

Patchset 2 (id:??) landed as
https://crrev.com/c0fee27d4307cb92cbe13e4d519f7c11b025a90b
Cr-Commit-Position: refs/heads/master@{#406933}