[webkit-reviews] review denied: [Bug 90400] Make the skia_test_expectations.txt file optional. : [Attachment 150471] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Jul 2 14:36:03 PDT 2012


Dirk Pranke <dpranke at chromium.org> has denied  review:
Bug 90400: Make the skia_test_expectations.txt file optional.
https://bugs.webkit.org/show_bug.cgi?id=90400

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

------- Additional Comments from Dirk Pranke <dpranke at chromium.org>
So, I'm conflicted about this change ... r-/cq-'ing it for a chance at
discussion.

The skia test_expectations file was never optional. The chromium downstream
file is optional. However, the skia file is used by both the upstream and
downstream bots, and corresponds to the version of skia that both bots build
against.

In other words, this file always exists and is used on the bots, and thus, it
would be an error if the file was missing.

Therefore, if you are trying to figure out what the expectations for a test
are, and you don't have this file in your checkout, there's a chance you will
form the wrong expectations.

This means that if you run rebaseline-expectations, you better have this file
in your checkout. If rebaseline-test is also trying to update the expectations,
it needs the skia file as well, and I think that this implies that you need
this even if you're running w/ garden-o-matic.

On the other hand, it's obviously annoying to have to do 'update-webkit
--chromium' just for this one file, especially when this file is supposed to be
empty most of the time.

So, I'm not sure what the right thing to do here is.


More information about the webkit-reviews mailing list