Initialize run_webkit_tests's image-first-tests flag from file

Initialize run_webkit_tests's image-first-tests flag from file

This patch changes run_webkit_tests.py to read the default value for
image-first-tests flag from
third_party/WebKit/LayoutTests/ImageFirstTests

BUG=703899
Review-Url: https://codereview.chromium.org/2772373003
Cr-Commit-Position: refs/heads/master@{#460980}
Committed: https://chromium.googlesource.com/chromium/src/+/899415b21db5d6befa5fdf7f38b256e346dcb465

[Gleb Lanbin, 2017-03-27 17:55:39]

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

[Gleb Lanbin, 2017-03-27 17:57:00]

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

[qyearsley, 2017-03-30 17:26:12]

Sorry for the delay, this CL slipped by me for the last couple days!

https://codereview.chromium.org/2772373003/diff/1/third_party/WebKit/LayoutTe...
File third_party/WebKit/LayoutTests/OWNERS (right):

https://codereview.chromium.org/2772373003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/OWNERS:7: per-file
ImageFirstTests=file://third_party/WebKit/Source/core/layout/OWNERS
First time I've seen this notation, didn't know you could do this :-)

https://codereview.chromium.org/2772373003/diff/1/third_party/WebKit/Tools/Sc...
File third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py
(right):

https://codereview.chromium.org/2772373003/diff/1/third_party/WebKit/Tools/Sc...
third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py:560:
options.image_first_tests.extend(line for line in
open(image_first_tests_path).read().strip().split('\n') if line)
In general, most of the code in webkitpy interacts with the filesystem
(including reading files) via a FileSystem object so that it can be replaced by
MockFileSystem in unit tests.

So for example, here would want to do something like:
    contents = port.host.filesystem.read_text_file(image_first_tests_path)
    options.image_first_tests.extend(line for line in contents.splitlines(False)
if line)

Note, if there is extra whitespace at the beginning and end of lines (e.g.
"one\n  two \nthree\n") then it'll be kept, but I assume that's OK. If it's not,
then we could change it to:
options.image_first_tests.extend(line.strip() for line in lines if line.strip())

I think it's a good idea to add a simple test for this in
run_webkit_tests_unittest.py.

[Gleb Lanbin, 2017-03-30 23:45:39]

PTAL

https://codereview.chromium.org/2772373003/diff/1/third_party/WebKit/LayoutTe...
File third_party/WebKit/LayoutTests/OWNERS (right):

https://codereview.chromium.org/2772373003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/OWNERS:7: per-file
ImageFirstTests=file://third_party/WebKit/Source/core/layout/OWNERS
On 2017/03/30 17:26:12, qyearsley wrote:
> First time I've seen this notation, didn't know you could do this :-)

Acknowledged. :)

https://codereview.chromium.org/2772373003/diff/1/third_party/WebKit/Tools/Sc...
File third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py
(right):

https://codereview.chromium.org/2772373003/diff/1/third_party/WebKit/Tools/Sc...
third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py:560:
options.image_first_tests.extend(line for line in
open(image_first_tests_path).read().strip().split('\n') if line)
On 2017/03/30 17:26:12, qyearsley wrote:
> In general, most of the code in webkitpy interacts with the filesystem
> (including reading files) via a FileSystem object so that it can be replaced
by
> MockFileSystem in unit tests.
> 
> So for example, here would want to do something like:
>     contents = port.host.filesystem.read_text_file(image_first_tests_path)
>     options.image_first_tests.extend(line for line in
contents.splitlines(False)
> if line)
> 
> Note, if there is extra whitespace at the beginning and end of lines (e.g.
> "one\n  two \nthree\n") then it'll be kept, but I assume that's OK. If it's
not,
> then we could change it to:
> options.image_first_tests.extend(line.strip() for line in lines if
line.strip())
> 
> I think it's a good idea to add a simple test for this in
> run_webkit_tests_unittest.py.

Done.

[Gleb Lanbin, 2017-03-30 23:50:43]

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

[commit-bot: I haz the power, 2017-03-30 23:51:30]

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-30 23:54:47]

lgtm

[Dirk Pranke, 2017-03-31 00:09:25]

lgtm

[commit-bot: I haz the power, 2017-03-31 01:07:05]

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

[commit-bot: I haz the power, 2017-03-31 01:07:06]

Dry run: This issue passed the CQ dry run.

[Gleb Lanbin, 2017-03-31 01:10:41]

The CQ bit was checked by glebl@chromium.org

[commit-bot: I haz the power, 2017-03-31 01:11:44]

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-31 01:31:28]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1490922641795340,
"parent_rev": "b0c09591d4f59f728515a628f25e78b19b78c09c", "commit_rev":
"899415b21db5d6befa5fdf7f38b256e346dcb465"}

[commit-bot: I haz the power, 2017-03-31 01:46:50]

Description was changed from

==========
Initialize run_webkit_tests's image-first-tests flag from file

This patch changes run_webkit_tests.py to read the default value for
image-first-tests flag from
third_party/WebKit/LayoutTests/ImageFirstTests

BUG=703899
==========

to

==========
Initialize run_webkit_tests's image-first-tests flag from file

This patch changes run_webkit_tests.py to read the default value for
image-first-tests flag from
third_party/WebKit/LayoutTests/ImageFirstTests

BUG=703899

Review-Url: https://codereview.chromium.org/2772373003
Cr-Commit-Position: refs/heads/master@{#460980}
Committed:
https://chromium.googlesource.com/chromium/src/+/899415b21db5d6befa5fdf7f38b2...
==========

[commit-bot: I haz the power, 2017-03-31 01:46:52]

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