[webkit-reviews] review granted: [Bug 86749] add skia test_expectations override file to chromium NRWT : [Attachment 142718] 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 86749: add skia test_expectations override file to chromium NRWT
https://bugs.webkit.org/show_bug.cgi?id=86749

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

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


I'm a little concerned about having the skia overrides live in the chromium
repo and still apply to all builds, rather than just the deps bots; this means
that we can't be sure that all the suppressions necessary for a green roll are
present in just the upstream files; they're no way to ignore the skia
suppressions. The chromium overrides initially applied everywhere, but we ended
up changing that for just this reason.

That said, it's not clear to me that there's a better solution, and in theory
most of the time the skia overrides will be empty, so this won't be an issue.
I'm willing to give this a shot.

> Tools/Scripts/webkitpy/layout_tests/port/chromium.py:323
> +    def expectations_file_contents(self, filetype, filepath):

Nit: I think this is a protected method, right? It should be
_expectations_file_contents (leading underscore).