Refactoring: Replace test_prefix_list variables with TestBaselineSet objects.

Refactoring: Replace test_prefix_list variables with TestBaselineSet objects.

Purpose: I think that this improves the readability of the code, since
it replaces dicts with a specific format that are being passed around
with instances of a class, and that class has documentation and tests.

Also, this should make it easier to fix http://crbug.com/673966; I'm
thinking that TestBaselineSet could be expanded to specify port names
separately from build names, so that rebaseline_cl could specify for
example that it wants to get baselines for some test from some Build,
but that the baseline location should correspond to another port's
baseline location.

This is a refactoring CL, so behavior should not change. In this CL:
 - Adds a class called TestBaselineSet (I'm open to name suggestions...)
   which represents some combinations of tests and platforms to rebaseline.
 - Adds a test for that class.
 - Updates other tests.
 - Removes _log_test_prefix_list; this is replaced with __str__ on the
   new class.
 - Refactors iteration through the collection of tests/builds to rebaseline.
 - Removes redundant method test_execute_with_issue_number_from_branch
   in rebaseline_cl_unittest.

BUG=702819
Review-Url: https://codereview.chromium.org/2765863003
Cr-Commit-Position: refs/heads/master@{#458592}
Committed: https://chromium.googlesource.com/chromium/src/+/09067d4bab0caeb8e1409148c9d6beabf762fd39

[qyearsley, 2017-03-21 18:18:56]

Description was changed from

==========
Replace test_prefix_list variables with TestBaselineSet objects.

Purpose: I think that this improves the readability of the code, since
it replaces dicts with a specific format that are being passed around
with instances of a class, and that class has documentation and tests.

Also, this should make it easier to fix http://crbug.com/673966; I'm
thinking that TestBaselineSet could be expanded to specify port names
separately from build names, so that rebaseline_cl could specify for
example that it wants to get baselines for some test from some Build,
but that the baseline location should correspond to another port's
baseline location.

This is a refactoring CL, so behavior should not change. In this CL:
 - Adds a class called TestBaselineSet (I'm open to name suggestions...)
   which represents some combinations of tests and platforms to rebaseline.
 - Adds a test for that class.
 - Updates other tests.
 - Removes _log_test_prefix_list; this is replaced with __str__ on the
   new class.
 - Refactors iteration through the collection of tests/builds to rebaseline.
 - Removes redundant method test_execute_with_issue_number_from_branch
   in rebaseline_cl_unittest.

BUG=702819
==========

to

==========
Refactoring: Replace test_prefix_list variables with TestBaselineSet objects.

Purpose: I think that this improves the readability of the code, since
it replaces dicts with a specific format that are being passed around
with instances of a class, and that class has documentation and tests.

Also, this should make it easier to fix http://crbug.com/673966; I'm
thinking that TestBaselineSet could be expanded to specify port names
separately from build names, so that rebaseline_cl could specify for
example that it wants to get baselines for some test from some Build,
but that the baseline location should correspond to another port's
baseline location.

This is a refactoring CL, so behavior should not change. In this CL:
 - Adds a class called TestBaselineSet (I'm open to name suggestions...)
   which represents some combinations of tests and platforms to rebaseline.
 - Adds a test for that class.
 - Updates other tests.
 - Removes _log_test_prefix_list; this is replaced with __str__ on the
   new class.
 - Refactors iteration through the collection of tests/builds to rebaseline.
 - Removes redundant method test_execute_with_issue_number_from_branch
   in rebaseline_cl_unittest.

BUG=702819
==========

[qyearsley, 2017-03-21 18:18:56]

qyearsley@chromium.org changed reviewers:
+ wkorman@chromium.org

[qyearsley, 2017-03-21 18:20:54]

Hi Walter, if you have time could you take a look at this?

This was a refactoring change that I wanted to do before (and I had left TODOs
in rebaseline.py), and now I think it may make it easier to add the
functionality of filling in results from other builders after this.

[qyearsley, 2017-03-21 18:21:01]

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

[commit-bot: I haz the power, 2017-03-21 18:21:46]

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

[wkorman, 2017-03-21 19:02:14]

lgtm

Sweet, name seems good to me, only a couple minor comments.

https://codereview.chromium.org/2765863003/diff/1/third_party/WebKit/Tools/Sc...
File third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline.py
(right):

https://codereview.chromium.org/2765863003/diff/1/third_party/WebKit/Tools/Sc...
third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline.py:382: A
list of builders that we may fetch from.
Document that it's returned sorted? Presuming that's important, which it seems
like it is since it looks like it's added in this change

https://codereview.chromium.org/2765863003/diff/1/third_party/WebKit/Tools/Sc...
third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline.py:535: a set
of tests/platform combinations to rebaseline.
TestRebaselineSet -> TestBaselineSet

[commit-bot: I haz the power, 2017-03-21 20:15:45]

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

[commit-bot: I haz the power, 2017-03-21 20:15:46]

Dry run: This issue passed the CQ dry run.

[qyearsley, 2017-03-21 21:47:22]

https://codereview.chromium.org/2765863003/diff/1/third_party/WebKit/Tools/Sc...
File third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline.py
(right):

https://codereview.chromium.org/2765863003/diff/1/third_party/WebKit/Tools/Sc...
third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline.py:382: A
list of builders that we may fetch from.
On 2017/03/21 at 19:02:13, wkorman wrote:
> Document that it's returned sorted? Presuming that's important, which it seems
like it is since it looks like it's added in this change

It's not really important that it was sorted; the reason for changing that was
so that the return order would be consistent for the unit test. Since order
isn't important here, I just changed this to return a set instead.

https://codereview.chromium.org/2765863003/diff/1/third_party/WebKit/Tools/Sc...
third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline.py:535: a set
of tests/platform combinations to rebaseline.
On 2017/03/21 at 19:02:13, wkorman wrote:
> TestRebaselineSet -> TestBaselineSet

Fixed

[qyearsley, 2017-03-21 21:47:28]

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

[commit-bot: I haz the power, 2017-03-21 21:48:07]

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

[qyearsley, 2017-03-21 22:23:46]

The CQ bit was unchecked by qyearsley@chromium.org

[qyearsley, 2017-03-21 22:23:47]

The CQ bit was checked by qyearsley@chromium.org

[qyearsley, 2017-03-21 22:23:48]

The patchset sent to the CQ was uploaded after l-g-t-m from wkorman@chromium.org
Link to the patchset: https://codereview.chromium.org/2765863003/#ps20001
(title: "fix typo, return set from _builders_to_fetch_from")

[commit-bot: I haz the power, 2017-03-21 22:24:14]

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, 2017-03-21 22:53:49]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1490135027969880,
"parent_rev": "8aa048d906db40cb8067e6e2d9393aca52f582d0", "commit_rev":
"09067d4bab0caeb8e1409148c9d6beabf762fd39"}

[commit-bot: I haz the power, 2017-03-21 22:54:34]

Description was changed from

==========
Refactoring: Replace test_prefix_list variables with TestBaselineSet objects.

Purpose: I think that this improves the readability of the code, since
it replaces dicts with a specific format that are being passed around
with instances of a class, and that class has documentation and tests.

Also, this should make it easier to fix http://crbug.com/673966; I'm
thinking that TestBaselineSet could be expanded to specify port names
separately from build names, so that rebaseline_cl could specify for
example that it wants to get baselines for some test from some Build,
but that the baseline location should correspond to another port's
baseline location.

This is a refactoring CL, so behavior should not change. In this CL:
 - Adds a class called TestBaselineSet (I'm open to name suggestions...)
   which represents some combinations of tests and platforms to rebaseline.
 - Adds a test for that class.
 - Updates other tests.
 - Removes _log_test_prefix_list; this is replaced with __str__ on the
   new class.
 - Refactors iteration through the collection of tests/builds to rebaseline.
 - Removes redundant method test_execute_with_issue_number_from_branch
   in rebaseline_cl_unittest.

BUG=702819
==========

to

==========
Refactoring: Replace test_prefix_list variables with TestBaselineSet objects.

Purpose: I think that this improves the readability of the code, since
it replaces dicts with a specific format that are being passed around
with instances of a class, and that class has documentation and tests.

Also, this should make it easier to fix http://crbug.com/673966; I'm
thinking that TestBaselineSet could be expanded to specify port names
separately from build names, so that rebaseline_cl could specify for
example that it wants to get baselines for some test from some Build,
but that the baseline location should correspond to another port's
baseline location.

This is a refactoring CL, so behavior should not change. In this CL:
 - Adds a class called TestBaselineSet (I'm open to name suggestions...)
   which represents some combinations of tests and platforms to rebaseline.
 - Adds a test for that class.
 - Updates other tests.
 - Removes _log_test_prefix_list; this is replaced with __str__ on the
   new class.
 - Refactors iteration through the collection of tests/builds to rebaseline.
 - Removes redundant method test_execute_with_issue_number_from_branch
   in rebaseline_cl_unittest.

BUG=702819

Review-Url: https://codereview.chromium.org/2765863003
Cr-Commit-Position: refs/heads/master@{#458592}
Committed:
https://chromium.googlesource.com/chromium/src/+/09067d4bab0caeb8e1409148c9d6...
==========

[commit-bot: I haz the power, 2017-03-21 22:54:35]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/09067d4bab0caeb8e1409148c9d6...