[webkit-reviews] review granted: [Bug 86642] Add setting of additional_expectations option to chromium_unittest.test_overrides_and_builder_names() : [Attachment 142282] Patch

[bugzilla-daemon at webkit.org]

Dirk Pranke <dpranke at chromium.org> has granted Elliot Poger
<epoger at chromium.org>'s request for review:
Bug 86642: Add setting of additional_expectations option to
chromium_unittest.test_overrides_and_builder_names()
https://bugs.webkit.org/show_bug.cgi?id=86642

Attachment 142282: Patch
https://bugs.webkit.org/attachment.cgi?id=142282&action=review

------- Additional Comments from Dirk Pranke <dpranke at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=142282&action=review


looks mostly fine, just a couple of nits.

> Tools/Scripts/webkitpy/layout_tests/port/chromium_unittest.py:296
> +	   filesystem.files[chromium_overrides_path] = CHROMIUM_OVERRIDES

please change this to filesystem.write_text_file(chromium_overrides_path,
CHROMIUM_OVERRIDES); originally I used to allow settings .files directly, but
that turns out to be fragile.

> Tools/Scripts/webkitpy/layout_tests/port/chromium_unittest.py:300
> +	   filesystem.files[additional_expectations_path] =
ADDITIONAL_EXPECTATIONS

same thing. Also, you probably want to use an absolute path here.