Refactoring: Move _port_skips_test to Port class.

Refactoring: Move _port_skips_test to Port class.

As part of rebaselining, when copying existing baselines, we don't want to
make new platform-specific baseline files for platforms where tests are
skipped. In particular, we don't want to make new baselines for android
for non-smoke-tests now since non-smoke-tests are skipped on android.

There's already logic in the copy-existing-baselines to do this; it uses
the TestExpectations.get_expectations method, which, as far as I understand,
returns expectations based on test expectations files, but not based on
whether the port only runs smoke tests.

Meanwhile, elsewhere in rebaseline.py, there's already logic to check
whether a test is skipped on a particular port including because it's not
a smoke test.

The purpose of this CL is to extract that method out so it could
be re-used. An alternative to doing this would be to change
TestExpectations.get_expectations to also include "SKIP" if
port.default_smoke_test_only() and the test is in the smoke tests file.

(Either way, there should be a centralized way of checking whether
a test runs on a port, and this should be used in both places in rebaseline.py.)

BUG=655196
Committed: https://crrev.com/f437383bbfd2cd52b8cacf0adff4e4fb17293127
Cr-Commit-Position: refs/heads/master@{#428067}

[qyearsley, 2016-10-27 00:47:10]

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

[qyearsley, 2016-10-27 00:47:11]

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

https://codereview.chromium.org/2450383002/diff/1/third_party/WebKit/Tools/Sc...
third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline.py:455: SKIP
not in generic_expectations.get_expectations(test))
It seems like this should just be (SKIP in
full_expectations.get_expectations(test)) -- I'm not sure why we would also want
to check that SKIP is not in the generic expectations file expectations.

[Dirk Pranke, 2016-10-27 00:53:45]

lgtm.

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

https://codereview.chromium.org/2450383002/diff/1/third_party/WebKit/Tools/Sc...
third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline.py:455: SKIP
not in generic_expectations.get_expectations(test))
On 2016/10/27 00:47:11, qyearsley wrote:
> It seems like this should just be (SKIP in
> full_expectations.get_expectations(test)) -- I'm not sure why we would also
want
> to check that SKIP is not in the generic expectations file expectations.

Unfortunately, I don't remember why we did this. Your question seems reasonable;
it might've been left over from a time when things were more complicated.

[qyearsley, 2016-10-27 01:01:41]

On 2016/10/27 at 00:53:45, dpranke wrote:
> lgtm.
> 
>
https://codereview.chromium.org/2450383002/diff/1/third_party/WebKit/Tools/Sc...
> File third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline.py
(left):
> 
>
https://codereview.chromium.org/2450383002/diff/1/third_party/WebKit/Tools/Sc...
> third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline.py:455:
SKIP not in generic_expectations.get_expectations(test))
> On 2016/10/27 00:47:11, qyearsley wrote:
> > It seems like this should just be (SKIP in
> > full_expectations.get_expectations(test)) -- I'm not sure why we would also
want
> > to check that SKIP is not in the generic expectations file expectations.
> 
> Unfortunately, I don't remember why we did this. Your question seems
reasonable; it might've been left over from a time when things were more
complicated.

Before submitting this: What do you think about adding this logic in
TestExpectations.get_expectations instead? (http://crrev.com/2457613002)

The advantage of putting this logic on Port rather than TestExpectations, I
think, is that TestExpectations represents the data from the test expectations
files, so adding in extra expectations that are not in any of the test
expectations files seems strange.

[Dirk Pranke, 2016-10-27 01:25:28]

On 2016/10/27 01:01:41, qyearsley wrote:
> Before submitting this: What do you think about adding this logic in
> TestExpectations.get_expectations instead? (http://crrev.com/2457613002)
> 
> The advantage of putting this logic on Port rather than TestExpectations, I
> think, is that TestExpectations represents the data from the test expectations
> files, so adding in extra expectations that are not in any of the test
> expectations files seems strange.

Right, I don't think I would do that.

[qyearsley, 2016-10-27 16:27:29]

The CQ bit was checked by qyearsley@chromium.org

[commit-bot: I haz the power, 2016-10-27 16:27:53]

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-10-27 17:25:43]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-10-27 17:31:17]

Description was changed from

==========
Refactoring: Move _port_skips_test to Port class.

As part of rebaselining, when copying existing baselines, we don't want to
make new platform-specific baseline files for platforms where tests are
skipped. In particular, we don't want to make new baselines for android
for non-smoke-tests now since non-smoke-tests are skipped on android.

There's already logic in the copy-existing-baselines to do this; it uses
the TestExpectations.get_expectations method, which, as far as I understand,
returns expectations based on test expectations files, but not based on
whether the port only runs smoke tests.

Meanwhile, elsewhere in rebaseline.py, there's already logic to check
whether a test is skipped on a particular port including because it's not
a smoke test.

The purpose of this CL is to extract that method out so it could
be re-used. An alternative to doing this would be to change
TestExpectations.get_expectations to also include "SKIP" if
port.default_smoke_test_only() and the test is in the smoke tests file.

(Either way, there should be a centralized way of checking whether
a test runs on a port, and this should be used in both places in rebaseline.py.)

BUG=655196
==========

to

==========
Refactoring: Move _port_skips_test to Port class.

As part of rebaselining, when copying existing baselines, we don't want to
make new platform-specific baseline files for platforms where tests are
skipped. In particular, we don't want to make new baselines for android
for non-smoke-tests now since non-smoke-tests are skipped on android.

There's already logic in the copy-existing-baselines to do this; it uses
the TestExpectations.get_expectations method, which, as far as I understand,
returns expectations based on test expectations files, but not based on
whether the port only runs smoke tests.

Meanwhile, elsewhere in rebaseline.py, there's already logic to check
whether a test is skipped on a particular port including because it's not
a smoke test.

The purpose of this CL is to extract that method out so it could
be re-used. An alternative to doing this would be to change
TestExpectations.get_expectations to also include "SKIP" if
port.default_smoke_test_only() and the test is in the smoke tests file.

(Either way, there should be a centralized way of checking whether
a test runs on a port, and this should be used in both places in rebaseline.py.)

BUG=655196

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

[commit-bot: I haz the power, 2016-10-27 17:31:18]

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